From 23710079c862cf4481a7a373800a2a52aabaaf3c Mon Sep 17 00:00:00 2001 From: Andreas Zecher Date: Sat, 21 Mar 2026 18:40:47 +0100 Subject: [PATCH] Replace instance_variable_get with public accessors in set editor tests Add attr_reader for selected, set, and ui to SetEditor and expose them publicly. Update tests to use the accessors directly. Add CLAUDE.md rule to avoid instance_variable_get and send in tests. Co-Authored-By: Claude Sonnet 4.6 --- CLAUDE.md | 1 + lib/wavesync/set_editor.rb | 4 ++- test/wavesync/set_editor_test.rb | 54 ++++++++++++++++---------------- 3 files changed, 31 insertions(+), 28 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 0f2ca31..1168110 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -5,6 +5,7 @@ ## Tests - Don't add comments to tests +- Don't use `instance_variable_get` or `send` in tests — add a public accessor or method to the class instead ## PRs - Squash commits in PRs diff --git a/lib/wavesync/set_editor.rb b/lib/wavesync/set_editor.rb index 0dda7eb..53e5b54 100644 --- a/lib/wavesync/set_editor.rb +++ b/lib/wavesync/set_editor.rb @@ -20,6 +20,7 @@ class SetEditor }.freeze attr_accessor :player_state #: Symbol + attr_reader :selected, :set, :ui #: untyped attr_writer :player_track, :player_index, :player_offset, :player_started_at #: (Set set, String library_path) -> void @@ -540,6 +541,7 @@ def move_track(direction) end public :handle_action, :advance_and_play, :display_name, :relative_path, - :format_duration, :playback_elapsed, :visible_length, :playback_bar + :format_duration, :playback_elapsed, :visible_length, :playback_bar, + :selected, :set, :ui end end diff --git a/test/wavesync/set_editor_test.rb b/test/wavesync/set_editor_test.rb index b1910ac..9884fa6 100644 --- a/test/wavesync/set_editor_test.rb +++ b/test/wavesync/set_editor_test.rb @@ -64,45 +64,45 @@ def track(name) test 'cursor starts at 0 when tracks are present' do e = editor(track('a.wav'), track('b.wav')) - assert_equal 0, e.instance_variable_get(:@selected) + assert_equal 0, e.selected end test 'cursor starts as nil when set is empty' do e = editor - assert_nil e.instance_variable_get(:@selected) + assert_nil e.selected end test 'cursor_down advances selection' do e = editor(track('a.wav'), track('b.wav'), track('c.wav')) e.handle_action(:cursor_down) - assert_equal 1, e.instance_variable_get(:@selected) + assert_equal 1, e.selected end test 'cursor_down stops at last track' do e = editor(track('a.wav'), track('b.wav')) e.handle_action(:cursor_down) e.handle_action(:cursor_down) - assert_equal 1, e.instance_variable_get(:@selected) + assert_equal 1, e.selected end test 'cursor_up moves selection back' do e = editor(track('a.wav'), track('b.wav')) e.handle_action(:cursor_down) e.handle_action(:cursor_up) - assert_equal 0, e.instance_variable_get(:@selected) + assert_equal 0, e.selected end test 'cursor_up stops at first track' do e = editor(track('a.wav'), track('b.wav')) e.handle_action(:cursor_up) - assert_equal 0, e.instance_variable_get(:@selected) + assert_equal 0, e.selected end test 'cursor navigation is a no-op on empty set' do e = editor e.handle_action(:cursor_down) e.handle_action(:cursor_up) - assert_nil e.instance_variable_get(:@selected) + assert_nil e.selected end test 'remove_track removes the selected track' do @@ -112,20 +112,20 @@ def track(name) e = editor(a, b, c) e.handle_action(:cursor_down) e.handle_action(:remove) - assert_equal [a, c], e.instance_variable_get(:@set).tracks + assert_equal [a, c], e.set.tracks end test 'remove_track keeps selection in bounds when removing last track' do e = editor(track('a.wav'), track('b.wav')) e.handle_action(:cursor_down) e.handle_action(:remove) - assert_equal 0, e.instance_variable_get(:@selected) + assert_equal 0, e.selected end test 'remove_track sets selected to nil when set becomes empty' do e = editor(track('a.wav')) e.handle_action(:remove) - assert_nil e.instance_variable_get(:@selected) + assert_nil e.selected end test 'remove_track stops playback when removing the playing track' do @@ -154,8 +154,8 @@ def track(name) e = editor(a, b, c) e.handle_action(:cursor_down) e.handle_action(:move_up) - assert_equal [b, a, c], e.instance_variable_get(:@set).tracks - assert_equal 0, e.instance_variable_get(:@selected) + assert_equal [b, a, c], e.set.tracks + assert_equal 0, e.selected end test 'move_down moves selected track down and follows it' do @@ -164,8 +164,8 @@ def track(name) c = track('c.wav') e = editor(a, b, c) e.handle_action(:move_down) - assert_equal [b, a, c], e.instance_variable_get(:@set).tracks - assert_equal 1, e.instance_variable_get(:@selected) + assert_equal [b, a, c], e.set.tracks + assert_equal 1, e.selected end test 'move_up is a no-op at the first track' do @@ -173,8 +173,8 @@ def track(name) b = track('b.wav') e = editor(a, b) e.handle_action(:move_up) - assert_equal [a, b], e.instance_variable_get(:@set).tracks - assert_equal 0, e.instance_variable_get(:@selected) + assert_equal [a, b], e.set.tracks + assert_equal 0, e.selected end test 'move_down is a no-op at the last track' do @@ -183,15 +183,15 @@ def track(name) e = editor(a, b) e.handle_action(:cursor_down) e.handle_action(:move_down) - assert_equal [a, b], e.instance_variable_get(:@set).tracks - assert_equal 1, e.instance_variable_get(:@selected) + assert_equal [a, b], e.set.tracks + assert_equal 1, e.selected end test 'move_track is a no-op with fewer than 2 tracks' do e = editor(track('a.wav')) e.handle_action(:move_up) e.handle_action(:move_down) - assert_equal 0, e.instance_variable_get(:@selected) + assert_equal 0, e.selected end test 'toggle_playback starts player on selected track' do @@ -247,7 +247,7 @@ def track(name) e = editor(a, b) e.expects(:start_player).with(b) e.advance_and_play - assert_equal 1, e.instance_variable_get(:@selected) + assert_equal 1, e.selected end test 'advance_and_play does nothing at last track' do @@ -514,28 +514,28 @@ def track(name) test 'playback_bar places cue point marker at correct position' do e = editor - e.instance_variable_get(:@ui).expects(:color).with('░░░░░', :surface).twice.returns('') - e.instance_variable_get(:@ui).expects(:color).with('◆', :surface).once.returns('') + e.ui.expects(:color).with('░░░░░', :surface).twice.returns('') + e.ui.expects(:color).with('◆', :surface).once.returns('') e.playback_bar(0.0, 100.0, 11, cue_fractions: [0.5]) end test 'playback_bar does not place cue markers when none given' do e = editor - e.instance_variable_get(:@ui).expects(:color).with('░░░░░░░░░░', :surface).once.returns('') + e.ui.expects(:color).with('░░░░░░░░░░', :surface).once.returns('') e.playback_bar(0.0, 100.0, 10) end test 'playback_bar colors cue at playhead with highlight' do e = editor - e.instance_variable_get(:@ui).stubs(:color).returns('') - e.instance_variable_get(:@ui).expects(:color).with('◆', :highlight).returns('').once + e.ui.stubs(:color).returns('') + e.ui.expects(:color).with('◆', :highlight).returns('').once e.playback_bar(50.0, 100.0, 10, cue_fractions: [0.5]) end test 'playback_bar colors cue not at playhead with surface color' do e = editor - e.instance_variable_get(:@ui).stubs(:color).returns('') - e.instance_variable_get(:@ui).expects(:color).with('◆', :surface).returns('').once + e.ui.stubs(:color).returns('') + e.ui.expects(:color).with('◆', :surface).returns('').once e.playback_bar(0.0, 100.0, 10, cue_fractions: [0.5]) end end