diff --git a/lib/cli/ui/spinner/spin_group.rb b/lib/cli/ui/spinner/spin_group.rb index ab459921..898027d1 100644 --- a/lib/cli/ui/spinner/spin_group.rb +++ b/lib/cli/ui/spinner/spin_group.rb @@ -100,7 +100,7 @@ class Task # * +title+ - Title of the task # * +block+ - Block for the task, will be provided with an instance of the spinner # - #: (String title, final_glyph: ^(bool success) -> (Glyph | String), merged_output: bool, duplicate_output_to: IO, work_queue: WorkQueue) { (Task task) -> untyped } -> void + #: (String title, final_glyph: ^(bool success) -> (Glyph | String), merged_output: bool, duplicate_output_to: io_like?, work_queue: WorkQueue) { (Task task) -> untyped } -> void def initialize(title, final_glyph:, merged_output:, duplicate_output_to:, work_queue:, &block) @title = title @final_glyph = final_glyph @@ -300,12 +300,12 @@ def inset_width # spin_group.add('Title') { |spinner| sleep 1.0 } # spin_group.wait # - #: (String title, ?final_glyph: ^(bool success) -> (Glyph | String), ?merged_output: bool, ?duplicate_output_to: IO) { (Task task) -> void } -> void + #: (String title, ?final_glyph: ^(bool success) -> (Glyph | String), ?merged_output: bool, ?duplicate_output_to: io_like?) { (Task task) -> void } -> void def add( title, final_glyph: DEFAULT_FINAL_GLYPH, merged_output: false, - duplicate_output_to: File.new(File::NULL, 'w'), + duplicate_output_to: nil, &block ) @m.synchronize do diff --git a/lib/cli/ui/stdout_router.rb b/lib/cli/ui/stdout_router.rb index 20f8144b..97677d1d 100644 --- a/lib/cli/ui/stdout_router.rb +++ b/lib/cli/ui/stdout_router.rb @@ -184,11 +184,11 @@ def outermost_uncaptured? end end - #: (?with_frame_inset: bool, ?merged_output: bool, ?duplicate_output_to: IO) { -> void } -> void + #: (?with_frame_inset: bool, ?merged_output: bool, ?duplicate_output_to: io_like?) { -> void } -> void def initialize( with_frame_inset: true, merged_output: false, - duplicate_output_to: File.open(File::NULL, 'w'), + duplicate_output_to: nil, &block ) @with_frame_inset = with_frame_inset @@ -225,7 +225,11 @@ def run case stream when :stdout @out.write(data) - @duplicate_output_to.write(data) + begin + @duplicate_output_to&.write(data) + rescue IOError + # Ignore + end when :stderr @err.write(data) else raise diff --git a/test/cli/ui/spinner/spin_group_test.rb b/test/cli/ui/spinner/spin_group_test.rb index 4f262568..411b04d1 100644 --- a/test/cli/ui/spinner/spin_group_test.rb +++ b/test/cli/ui/spinner/spin_group_test.rb @@ -36,6 +36,50 @@ def test_spin_group_auto_debrief_false assert_equal('', err) end + def test_spin_group_tasks_open_no_descriptors + skip('/dev/fd unavailable') unless open_fds + + capture_io do + CLI::UI::StdoutRouter.ensure_activated + + warmup = SpinGroup.new(auto_debrief: false) + warmup.add('warmup') { true } + assert(warmup.wait) + + before = open_fds + release = Queue.new + sg = SpinGroup.new(auto_debrief: false, max_concurrent: 1) + 20.times { |i| sg.add("task #{i}") { release.pop } } + + while_running = open_fds + 20.times { release.push(true) } + assert(sg.wait) + after = open_fds + + assert_empty(while_running - before) # pending tasks hold no handles + assert_empty(after - before) + end + end + + def test_spin_group_task_duplicates_output_to_stream + dup = StringIO.new + + capture_io do + CLI::UI::StdoutRouter.ensure_activated + + sg = SpinGroup.new(auto_debrief: false) + sg.add('task', duplicate_output_to: dup) do + print('task output') + true + end + + assert(sg.wait) + end + + assert_equal('task output', dup.string) + refute_predicate(dup, :closed?) # the task doesn't own the handle + end + def test_spin_group_success_debrief capture_io do CLI::UI::StdoutRouter.ensure_activated diff --git a/test/cli/ui/stdout_router_test.rb b/test/cli/ui/stdout_router_test.rb index 23be5aa5..e73e88a2 100644 --- a/test/cli/ui/stdout_router_test.rb +++ b/test/cli/ui/stdout_router_test.rb @@ -24,6 +24,59 @@ def test_current_id end end + def test_capture_opens_no_descriptors + skip('/dev/fd unavailable') unless open_fds + + StdoutRouter::Capture.new {} # warm up any lazily-required files + before = open_fds + captures = Array.new(10) { StdoutRouter::Capture.new {} } + + assert_empty(open_fds - before) + refute_empty(captures) # keep them reachable so nothing is finalized early + end + + def test_capture_duplicate_output_to + capture_io do + StdoutRouter.with_enabled do + dup = StringIO.new + cap = StdoutRouter::Capture.new(duplicate_output_to: dup) { print('hello') } + cap.run + assert_equal('hello', cap.stdout) + assert_equal('hello', dup.string) + refute_predicate(dup, :closed?) # the capture doesn't own the handle + end + end + end + + def test_capture_duplicate_output_to_receives_merged_stderr + capture_io do + StdoutRouter.with_enabled do + dup = StringIO.new + cap = StdoutRouter::Capture.new(merged_output: true, duplicate_output_to: dup) do + $stderr.print('oops') + end + cap.run + assert_equal('oops', dup.string) + end + end + end + + def test_capture_ignores_closed_duplicate_output + capture_io do + StdoutRouter.with_enabled do + dup = StringIO.new + cap = StdoutRouter::Capture.new(duplicate_output_to: dup) do + dup.close + print('hello') + end + + cap.run + + assert_equal('hello', cap.stdout) + end + end + end + def test_frame_can_autoload_after_router_is_enabled script = <<~RUBY require 'stringio' diff --git a/test/test_helper.rb b/test/test_helper.rb index e3cdb17c..5695a192 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -56,6 +56,16 @@ def with_os_mock_and_reload(os, class_names = [], files = []) reset.call end +# The file descriptors this process currently holds open. Both macOS and Linux expose +# them as /dev/fd; returns nil elsewhere so callers can skip. Compare sets and assert +# only on additions: unrelated handles left by earlier tests get finalized at arbitrary +# points, which changes the count without anything having leaked. +def open_fds + Dir.children('/dev/fd') +rescue SystemCallError + nil +end + require 'fileutils' require 'tmpdir' require 'tempfile'