From 5f08116689ba77e68b0876abc3b393d26c3da64f Mon Sep 17 00:00:00 2001 From: Masterisk-F Date: Sat, 27 Jun 2026 22:09:19 +0900 Subject: [PATCH 1/3] Fix disc rescan not updating after swapping discs ScanDiscCdparanoia#scan() and ScanDiscCdinfo#scan() both had an early return guard ('return true if @status == 'ok'') that prevented re-scanning after the first successful scan. This meant swapping discs and selecting 'Scan drive for audio disc' would still show the old disc's data. - Call restoreStatus() at the start of ScanDiscCdparanoia#scan() to reset @status and @error before each scan. - Set @status = nil at the start of ScanDiscCdinfo#scan() for the same purpose. - Fix existing scanDiscCdparanoia_spec.rb mock argument matching (missing stopPatterns parameter). - Add rescan-behavior tests for both scanners. --- lib/rubyripper/disc/scanDiscCdinfo.rb | 2 +- lib/rubyripper/disc/scanDiscCdparanoia.rb | 2 +- spec/disc/scanDiscCdinfo_spec.rb | 18 ++++++++++++++++++ spec/disc/scanDiscCdparanoia_spec.rb | 22 +++++++++++++++++++++- 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/lib/rubyripper/disc/scanDiscCdinfo.rb b/lib/rubyripper/disc/scanDiscCdinfo.rb index 2e82dc1..d890a64 100644 --- a/lib/rubyripper/disc/scanDiscCdinfo.rb +++ b/lib/rubyripper/disc/scanDiscCdinfo.rb @@ -46,7 +46,7 @@ def initialize(execute=nil, prefs=nil) # scan the contents of the disc def scan - return true if @status == 'ok' + @status = nil query = @exec.launch("cd-info -C #{@prefs.cdrom} -A --no-cddb") if isValidQuery(query) diff --git a/lib/rubyripper/disc/scanDiscCdparanoia.rb b/lib/rubyripper/disc/scanDiscCdparanoia.rb index aeddd44..6454a3c 100644 --- a/lib/rubyripper/disc/scanDiscCdparanoia.rb +++ b/lib/rubyripper/disc/scanDiscCdparanoia.rb @@ -47,7 +47,7 @@ def initialize(execute=nil, permissionDrive=nil, prefs=nil, out=nil) # scan the disc for input and return the object def scan - return true if @status == 'ok' + restoreStatus() if @perm.problems?(@prefs.cdrom) updateStatus(@perm.error) else diff --git a/spec/disc/scanDiscCdinfo_spec.rb b/spec/disc/scanDiscCdinfo_spec.rb index 6b50f83..2f7cde7 100644 --- a/spec/disc/scanDiscCdinfo_spec.rb +++ b/spec/disc/scanDiscCdinfo_spec.rb @@ -153,4 +153,22 @@ def setStandardQueryReply expect(scan.tracks).to eq(1) end end + + context "When scanning again after a successful scan" do + it "should re-execute the launch command on second scan (not reuse cached status)" do + # 1回目のスキャン + expect(exec).to receive(:launch).with('cd-info -C /dev/cdrom -A --no-cddb').and_return( + ["170: 43:33:30 195855 leadout"]) + scan.scan() + expect(scan.status).to eq('ok') + + # 2回目のスキャンでも launch が呼ばれること、かつ異なる結果が反映されること + expect(exec).to receive(:launch).with('cd-info -C /dev/cdrom -A --no-cddb').and_return( + ["170: 50:00:00 225000 leadout"]) + scan.scan() + expect(scan.status).to eq('ok') + # toSectors("50:00:00") - OFFSET_CDINFO(150) = 225000 - 150 = 224850 + expect(scan.totalSectors).to eq(224850) + end + end end diff --git a/spec/disc/scanDiscCdparanoia_spec.rb b/spec/disc/scanDiscCdparanoia_spec.rb index cca6d82..7206cec 100644 --- a/spec/disc/scanDiscCdparanoia_spec.rb +++ b/spec/disc/scanDiscCdparanoia_spec.rb @@ -22,7 +22,7 @@ def setQueryReply(reply, command=nil) allow(prefs).to receive('testdisc').and_return false command ||= 'cd-paranoia -d /dev/cdrom -vQ' - allow(exec).to receive(:launch).with(command).and_return reply + allow(exec).to receive(:launch).with(command, false, nil, any_args).and_return reply end let(:exec) {double('Execute').as_null_object} @@ -223,4 +223,24 @@ def setQueryReply(reply, command=nil) expect(disc.totalSectors).to eq(162919) end end + + context "When scanning again after a successful scan" do + before(:each) do + @cdparanoia ||= File.read('spec/disc/data/cdparanoia').split("\n") + allow(perm).to receive(:problems?).and_return(false) + allow(perm).to receive(:problemsSCSI?).and_return(false) + allow(prefs).to receive(:cdrom).and_return('/dev/cdrom') + end + + it "should re-execute the launch command on second scan (not reuse cached status)" do + setQueryReply(@cdparanoia) + disc.scan() + expect(disc.status).to eq('ok') + + # 2回目の scan でも launch が呼ばれるべき（@status == 'ok' で早期リターンしない） + expect(exec).to receive(:launch).with('cd-paranoia -d /dev/cdrom -vQ', false, nil, any_args).and_return(@cdparanoia) + disc.scan() + expect(disc.status).to eq('ok') + end + end end From 58377a6b724d481c13c990b4de3c40b08fc0b460 Mon Sep 17 00:00:00 2001 From: Masterisk-F Date: Sun, 28 Jun 2026 02:17:42 +0900 Subject: [PATCH 2/3] refactor: recreate scanner instances on rescan instead of manually resetting state --- lib/rubyripper/disc/disc.rb | 9 +++++++++ lib/rubyripper/disc/scanDiscCdcontrol.rb | 1 - lib/rubyripper/disc/scanDiscCdinfo.rb | 1 - lib/rubyripper/disc/scanDiscCdparanoia.rb | 1 - spec/disc/disc_spec.rb | 19 +++++++++++++++++++ spec/disc/scanDiscCdinfo_spec.rb | 16 ---------------- spec/disc/scanDiscCdparanoia_spec.rb | 18 ------------------ 7 files changed, 28 insertions(+), 37 deletions(-) diff --git a/lib/rubyripper/disc/disc.rb b/lib/rubyripper/disc/disc.rb index 216aa74..6e8f5d0 100644 --- a/lib/rubyripper/disc/disc.rb +++ b/lib/rubyripper/disc/disc.rb @@ -26,6 +26,9 @@ class Disc attr_reader :metadata, :cdrdao, :musicbrainz_failed def initialize(cdpar=nil, freedb=nil, musicbrainz=nil, deps=nil, prefs=nil) + # @has_mock flag is a workaround for RSpec tests that inject a mock instance. + # It ensures the mock isn't overwritten by a new instance during a rescan. + @has_mock = !cdpar.nil? @cdparanoia = cdpar ? cdpar : ScanDiscCdparanoia.new() @calcFreedbID = freedb ? freedb : CalcFreedbID.new(self) @calcMusicbrainzID = musicbrainz ? musicbrainz : CalcMusicbrainzID.new(self) @@ -35,6 +38,12 @@ def initialize(cdpar=nil, freedb=nil, musicbrainz=nil, deps=nil, prefs=nil) # scan the disc for a drive def scan(metadata=nil) + # Recreate the scanner instance to clear any previous state, unless a mock was injected for testing + @cdparanoia = ScanDiscCdparanoia.new() unless @has_mock + @scanner = nil + @cdrdao = nil + @cuesheet = nil + @cdparanoia.scan() if @cdparanoia.status == 'ok' setMetadata(metadata) diff --git a/lib/rubyripper/disc/scanDiscCdcontrol.rb b/lib/rubyripper/disc/scanDiscCdcontrol.rb index fbdf5d5..a2c349f 100644 --- a/lib/rubyripper/disc/scanDiscCdcontrol.rb +++ b/lib/rubyripper/disc/scanDiscCdcontrol.rb @@ -43,7 +43,6 @@ def initialize(execute=nil, prefs=nil) # scan the contents of the disc def scan - return true if @status == 'ok' query = @exec.launch("cdcontrol -f #{@prefs.cdrom} info") if isValidQuery(query) diff --git a/lib/rubyripper/disc/scanDiscCdinfo.rb b/lib/rubyripper/disc/scanDiscCdinfo.rb index d890a64..80bcf25 100644 --- a/lib/rubyripper/disc/scanDiscCdinfo.rb +++ b/lib/rubyripper/disc/scanDiscCdinfo.rb @@ -46,7 +46,6 @@ def initialize(execute=nil, prefs=nil) # scan the contents of the disc def scan - @status = nil query = @exec.launch("cd-info -C #{@prefs.cdrom} -A --no-cddb") if isValidQuery(query) diff --git a/lib/rubyripper/disc/scanDiscCdparanoia.rb b/lib/rubyripper/disc/scanDiscCdparanoia.rb index 6454a3c..7a0e928 100644 --- a/lib/rubyripper/disc/scanDiscCdparanoia.rb +++ b/lib/rubyripper/disc/scanDiscCdparanoia.rb @@ -47,7 +47,6 @@ def initialize(execute=nil, permissionDrive=nil, prefs=nil, out=nil) # scan the disc for input and return the object def scan - restoreStatus() if @perm.problems?(@prefs.cdrom) updateStatus(@perm.error) else diff --git a/spec/disc/disc_spec.rb b/spec/disc/disc_spec.rb index c98b88d..2869ca5 100644 --- a/spec/disc/disc_spec.rb +++ b/spec/disc/disc_spec.rb @@ -122,4 +122,23 @@ expect(disc.musicbrainz_failed).to eq(false) end end + context "When scanning again" do + it "should recreate scanner instances on second scan" do + cdpar_instance_0 = double('ScanDiscCdparanoia').as_null_object + cdpar_instance_1 = double('ScanDiscCdparanoia').as_null_object + cdpar_instance_2 = double('ScanDiscCdparanoia').as_null_object + + expect(ScanDiscCdparanoia).to receive(:new).and_return(cdpar_instance_0, cdpar_instance_1, cdpar_instance_2) + + disc_without_mocks = Disc.new(nil, freedb, musicbrainz, deps, prefs) + + expect(cdpar_instance_1).to receive(:scan).once + expect(cdpar_instance_1).to receive(:status).and_return(false) + disc_without_mocks.scan(nil) + + expect(cdpar_instance_2).to receive(:scan).once + expect(cdpar_instance_2).to receive(:status).and_return(false) + disc_without_mocks.scan(nil) + end + end end diff --git a/spec/disc/scanDiscCdinfo_spec.rb b/spec/disc/scanDiscCdinfo_spec.rb index 2f7cde7..fe7a8d0 100644 --- a/spec/disc/scanDiscCdinfo_spec.rb +++ b/spec/disc/scanDiscCdinfo_spec.rb @@ -154,21 +154,5 @@ def setStandardQueryReply end end - context "When scanning again after a successful scan" do - it "should re-execute the launch command on second scan (not reuse cached status)" do - # 1回目のスキャン - expect(exec).to receive(:launch).with('cd-info -C /dev/cdrom -A --no-cddb').and_return( - ["170: 43:33:30 195855 leadout"]) - scan.scan() - expect(scan.status).to eq('ok') - # 2回目のスキャンでも launch が呼ばれること、かつ異なる結果が反映されること - expect(exec).to receive(:launch).with('cd-info -C /dev/cdrom -A --no-cddb').and_return( - ["170: 50:00:00 225000 leadout"]) - scan.scan() - expect(scan.status).to eq('ok') - # toSectors("50:00:00") - OFFSET_CDINFO(150) = 225000 - 150 = 224850 - expect(scan.totalSectors).to eq(224850) - end - end end diff --git a/spec/disc/scanDiscCdparanoia_spec.rb b/spec/disc/scanDiscCdparanoia_spec.rb index 7206cec..52e683e 100644 --- a/spec/disc/scanDiscCdparanoia_spec.rb +++ b/spec/disc/scanDiscCdparanoia_spec.rb @@ -224,23 +224,5 @@ def setQueryReply(reply, command=nil) end end - context "When scanning again after a successful scan" do - before(:each) do - @cdparanoia ||= File.read('spec/disc/data/cdparanoia').split("\n") - allow(perm).to receive(:problems?).and_return(false) - allow(perm).to receive(:problemsSCSI?).and_return(false) - allow(prefs).to receive(:cdrom).and_return('/dev/cdrom') - end - it "should re-execute the launch command on second scan (not reuse cached status)" do - setQueryReply(@cdparanoia) - disc.scan() - expect(disc.status).to eq('ok') - - # 2回目の scan でも launch が呼ばれるべき（@status == 'ok' で早期リターンしない） - expect(exec).to receive(:launch).with('cd-paranoia -d /dev/cdrom -vQ', false, nil, any_args).and_return(@cdparanoia) - disc.scan() - expect(disc.status).to eq('ok') - end - end end From de14419670da3006f5960bbfe05760b2ebb39ca3 Mon Sep 17 00:00:00 2001 From: Masterisk-F Date: Sun, 28 Jun 2026 17:22:58 +0900 Subject: [PATCH 3/3] Revert scanner-side changes from PR #13 and fix via CliDisc#refreshDisc Disc.new() instead PR #13's original approach modified 7 files to add scanner instance re-creation and @has_mock workaround inside Disc#scan. This left multiple stale-state issues (CalcFreedbID/CalcMusicbrainzID caches, @dataTracks accumulation, @firstAudioTrack freeze) that required per-variable resets. A simpler approach: make CliDisc#refreshDisc recreate the Disc instance on every rescan, exactly like GtkDisc and TUIDisc already do. This guarantees all sub-objects (Scanner, CalcFreedbID, CalcMusicbrainzID) start fresh without per-variable reset patches. Changes: - cliDisc.rb: refreshDisc now does @cd = Disc.new(); @cd.scan() - scanDiscCdparanoia.rb: restore 'return true if @status == 'ok'' guard - scanDiscCdinfo.rb: restore 'return true if @status == 'ok'' guard - scanDiscCdcontrol.rb: restore 'return true if @status == 'ok'' guard - disc.rb: revert @has_mock, @cdparanoia re-creation, @scanner/etc=nil - spec/cli/cliDisc_spec.rb: new test verifying Disc.new() on refresh - spec/disc/disc_spec.rb: revert 'When scanning again' test block - spec/disc/scanDiscCdinfo_spec.rb: revert trailing blank lines - spec/disc/scanDiscCdparanoia_spec.rb: revert trailing blank lines (any_args fix from PR #13 preserved as pre-existing bugfix) --- lib/rubyripper/cli/cliDisc.rb | 7 +++- lib/rubyripper/disc/disc.rb | 9 ----- lib/rubyripper/disc/scanDiscCdcontrol.rb | 1 + lib/rubyripper/disc/scanDiscCdinfo.rb | 1 + lib/rubyripper/disc/scanDiscCdparanoia.rb | 1 + spec/cli/cliDisc_spec.rb | 46 +++++++++++++++++++++++ spec/disc/disc_spec.rb | 19 ---------- spec/disc/scanDiscCdinfo_spec.rb | 2 - spec/disc/scanDiscCdparanoia_spec.rb | 2 - 9 files changed, 54 insertions(+), 34 deletions(-) create mode 100644 spec/cli/cliDisc_spec.rb diff --git a/lib/rubyripper/cli/cliDisc.rb b/lib/rubyripper/cli/cliDisc.rb index bb96f40..42eb465 100644 --- a/lib/rubyripper/cli/cliDisc.rb +++ b/lib/rubyripper/cli/cliDisc.rb @@ -57,8 +57,11 @@ def tracks private def discReady? ; @cd.status == 'ok' ; end - # read the disc contents - def refreshDisc ; @cd.scan() ; end + # read the disc contents. Recreate Disc to get fresh scanner & calculator state. + def refreshDisc + @cd = Disc.new() + @cd.scan() + end # show the contents of the audio disc def showDisc diff --git a/lib/rubyripper/disc/disc.rb b/lib/rubyripper/disc/disc.rb index 6e8f5d0..216aa74 100644 --- a/lib/rubyripper/disc/disc.rb +++ b/lib/rubyripper/disc/disc.rb @@ -26,9 +26,6 @@ class Disc attr_reader :metadata, :cdrdao, :musicbrainz_failed def initialize(cdpar=nil, freedb=nil, musicbrainz=nil, deps=nil, prefs=nil) - # @has_mock flag is a workaround for RSpec tests that inject a mock instance. - # It ensures the mock isn't overwritten by a new instance during a rescan. - @has_mock = !cdpar.nil? @cdparanoia = cdpar ? cdpar : ScanDiscCdparanoia.new() @calcFreedbID = freedb ? freedb : CalcFreedbID.new(self) @calcMusicbrainzID = musicbrainz ? musicbrainz : CalcMusicbrainzID.new(self) @@ -38,12 +35,6 @@ def initialize(cdpar=nil, freedb=nil, musicbrainz=nil, deps=nil, prefs=nil) # scan the disc for a drive def scan(metadata=nil) - # Recreate the scanner instance to clear any previous state, unless a mock was injected for testing - @cdparanoia = ScanDiscCdparanoia.new() unless @has_mock - @scanner = nil - @cdrdao = nil - @cuesheet = nil - @cdparanoia.scan() if @cdparanoia.status == 'ok' setMetadata(metadata) diff --git a/lib/rubyripper/disc/scanDiscCdcontrol.rb b/lib/rubyripper/disc/scanDiscCdcontrol.rb index a2c349f..fbdf5d5 100644 --- a/lib/rubyripper/disc/scanDiscCdcontrol.rb +++ b/lib/rubyripper/disc/scanDiscCdcontrol.rb @@ -43,6 +43,7 @@ def initialize(execute=nil, prefs=nil) # scan the contents of the disc def scan + return true if @status == 'ok' query = @exec.launch("cdcontrol -f #{@prefs.cdrom} info") if isValidQuery(query) diff --git a/lib/rubyripper/disc/scanDiscCdinfo.rb b/lib/rubyripper/disc/scanDiscCdinfo.rb index 80bcf25..2e82dc1 100644 --- a/lib/rubyripper/disc/scanDiscCdinfo.rb +++ b/lib/rubyripper/disc/scanDiscCdinfo.rb @@ -46,6 +46,7 @@ def initialize(execute=nil, prefs=nil) # scan the contents of the disc def scan + return true if @status == 'ok' query = @exec.launch("cd-info -C #{@prefs.cdrom} -A --no-cddb") if isValidQuery(query) diff --git a/lib/rubyripper/disc/scanDiscCdparanoia.rb b/lib/rubyripper/disc/scanDiscCdparanoia.rb index 7a0e928..aeddd44 100644 --- a/lib/rubyripper/disc/scanDiscCdparanoia.rb +++ b/lib/rubyripper/disc/scanDiscCdparanoia.rb @@ -47,6 +47,7 @@ def initialize(execute=nil, permissionDrive=nil, prefs=nil, out=nil) # scan the disc for input and return the object def scan + return true if @status == 'ok' if @perm.problems?(@prefs.cdrom) updateStatus(@perm.error) else diff --git a/spec/cli/cliDisc_spec.rb b/spec/cli/cliDisc_spec.rb new file mode 100644 index 0000000..2459354 --- /dev/null +++ b/spec/cli/cliDisc_spec.rb @@ -0,0 +1,46 @@ +#!/usr/bin/env ruby +# RubyRipperRemix - A secure ripper for Linux/BSD/OSX +# Copyright (C) 2026 Masterisk-F +# +# This file is part of RubyRipperRemix. RubyRipperRemix is free software: you can +# redistribute it and/or modify it under the terms of the GNU General +# Public License as published by the Free Software Foundation, either +# version 3 of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see + +require 'rubyripper/cli/cliDisc' + +describe CliDisc do + let(:out) {StringIO.new} + let(:int) {double('CliGetInt').as_null_object} + let(:bool) {double('CliGetBool').as_null_object} + let(:string) {double('CliGetString').as_null_object} + let(:prefs) {double('Preferences::Main').as_null_object} + + def setup_cli_disc + CliDisc.new(out, int, bool, string, prefs) + end + + context "When refreshing the disc" do + it "should create a new Disc instance on refresh" do + first_disc = double('Disc').as_null_object + second_disc = double('Disc').as_null_object + + expect(Disc).to receive(:new).and_return(first_disc, second_disc) + + cli_disc = setup_cli_disc + expect(cli_disc.cd).to eq(first_disc) + + expect(second_disc).to receive(:scan) + cli_disc.send(:refreshDisc) + expect(cli_disc.cd).to eq(second_disc) + end + end +end diff --git a/spec/disc/disc_spec.rb b/spec/disc/disc_spec.rb index 2869ca5..c98b88d 100644 --- a/spec/disc/disc_spec.rb +++ b/spec/disc/disc_spec.rb @@ -122,23 +122,4 @@ expect(disc.musicbrainz_failed).to eq(false) end end - context "When scanning again" do - it "should recreate scanner instances on second scan" do - cdpar_instance_0 = double('ScanDiscCdparanoia').as_null_object - cdpar_instance_1 = double('ScanDiscCdparanoia').as_null_object - cdpar_instance_2 = double('ScanDiscCdparanoia').as_null_object - - expect(ScanDiscCdparanoia).to receive(:new).and_return(cdpar_instance_0, cdpar_instance_1, cdpar_instance_2) - - disc_without_mocks = Disc.new(nil, freedb, musicbrainz, deps, prefs) - - expect(cdpar_instance_1).to receive(:scan).once - expect(cdpar_instance_1).to receive(:status).and_return(false) - disc_without_mocks.scan(nil) - - expect(cdpar_instance_2).to receive(:scan).once - expect(cdpar_instance_2).to receive(:status).and_return(false) - disc_without_mocks.scan(nil) - end - end end diff --git a/spec/disc/scanDiscCdinfo_spec.rb b/spec/disc/scanDiscCdinfo_spec.rb index fe7a8d0..6b50f83 100644 --- a/spec/disc/scanDiscCdinfo_spec.rb +++ b/spec/disc/scanDiscCdinfo_spec.rb @@ -153,6 +153,4 @@ def setStandardQueryReply expect(scan.tracks).to eq(1) end end - - end diff --git a/spec/disc/scanDiscCdparanoia_spec.rb b/spec/disc/scanDiscCdparanoia_spec.rb index 52e683e..ab0d647 100644 --- a/spec/disc/scanDiscCdparanoia_spec.rb +++ b/spec/disc/scanDiscCdparanoia_spec.rb @@ -223,6 +223,4 @@ def setQueryReply(reply, command=nil) expect(disc.totalSectors).to eq(162919) end end - - end