From 3671eb5c1b6853adf0b9c0ff393c04785c949397 Mon Sep 17 00:00:00 2001 From: Jonathan del Strother Date: Thu, 19 Sep 2024 16:41:28 +0100 Subject: [PATCH 1/2] Fix output.to_stdout_from_any_process when $stdout has been reassigned When running under rspec bisect runner, $stdout & $stderr are reassigned to a StringIO, which would introduce spec errors. ("can't convert Tempfile into StringIO", when trying to reopen the stream in CaptureStreamToTempfile) Since CaptureStreamToTempfile reopens the stream rather than reassigning it, I _think_ it makes sense to use STDOUT rather than whatever the current value of $stdout is. --- lib/rspec/matchers/built_in/output.rb | 4 ++-- spec/rspec/matchers/built_in/output_spec.rb | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/rspec/matchers/built_in/output.rb b/lib/rspec/matchers/built_in/output.rb index 888ccaee2..767915839 100644 --- a/lib/rspec/matchers/built_in/output.rb +++ b/lib/rspec/matchers/built_in/output.rb @@ -46,7 +46,7 @@ def to_stderr # Works when subprocesses print to stdout as well. # This is significantly (~30x) slower than `to_stdout` def to_stdout_from_any_process - @stream_capturer = CaptureStreamToTempfile.new("stdout", $stdout) + @stream_capturer = CaptureStreamToTempfile.new("stdout", STDOUT) # rubocop:disable Style/GlobalStdStream self end @@ -55,7 +55,7 @@ def to_stdout_from_any_process # Works when subprocesses print to stderr as well. # This is significantly (~30x) slower than `to_stderr` def to_stderr_from_any_process - @stream_capturer = CaptureStreamToTempfile.new("stderr", $stderr) + @stream_capturer = CaptureStreamToTempfile.new("stderr", STDERR) # rubocop:disable Style/GlobalStdStream self end diff --git a/spec/rspec/matchers/built_in/output_spec.rb b/spec/rspec/matchers/built_in/output_spec.rb index 3873013d1..0440f867a 100644 --- a/spec/rspec/matchers/built_in/output_spec.rb +++ b/spec/rspec/matchers/built_in/output_spec.rb @@ -169,6 +169,16 @@ def print_to_stream(msg) end end } + + it "passes even if $stderr has previously been set to a StringIO" do + original_stderr = $stderr + begin + $stderr = StringIO.new + expect { print_to_stream("foo") }.to output("foo").to_stderr_from_any_process + ensure + $stderr = original_stderr + end + end end RSpec.describe "output.to_stdout_from_any_process matcher" do @@ -181,6 +191,16 @@ def print_to_stream(msg) end end } + + it "passes even if $stdout has previously been set to a StringIO" do + original_stdout = $stdout + begin + $stdout = StringIO.new + expect { print_to_stream("foo") }.to output("foo").to_stdout_from_any_process + ensure + $stdout = original_stdout + end + end end RSpec.describe "output (without `to_stdout` or `to_stderr`)" do From 392050b46ac00703d46e0bbadd8519785cfab29d Mon Sep 17 00:00:00 2001 From: Jonathan del Strother Date: Fri, 20 Sep 2024 16:24:49 +0100 Subject: [PATCH 2/2] Reassign $stdout/$stderr in CaptureStreamToTempfile Otherwise it doesn't work when $stderr has previously been reassigned, like in the rspec-core test suite --- lib/rspec/matchers/built_in/output.rb | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/rspec/matchers/built_in/output.rb b/lib/rspec/matchers/built_in/output.rb index 767915839..97c00c9e5 100644 --- a/lib/rspec/matchers/built_in/output.rb +++ b/lib/rspec/matchers/built_in/output.rb @@ -46,7 +46,7 @@ def to_stderr # Works when subprocesses print to stdout as well. # This is significantly (~30x) slower than `to_stdout` def to_stdout_from_any_process - @stream_capturer = CaptureStreamToTempfile.new("stdout", STDOUT) # rubocop:disable Style/GlobalStdStream + @stream_capturer = CaptureStreamToTempfile.new("stdout") self end @@ -55,7 +55,7 @@ def to_stdout_from_any_process # Works when subprocesses print to stderr as well. # This is significantly (~30x) slower than `to_stderr` def to_stderr_from_any_process - @stream_capturer = CaptureStreamToTempfile.new("stderr", STDERR) # rubocop:disable Style/GlobalStdStream + @stream_capturer = CaptureStreamToTempfile.new("stderr") self end @@ -213,7 +213,7 @@ def capture(block) end # @private - class CaptureStreamToTempfile < Struct.new(:name, :stream) + class CaptureStreamToTempfile < Struct.new(:name) def capture(block) # We delay loading tempfile until it is actually needed because # we want to minimize stdlibs loaded so that users who use a @@ -223,19 +223,29 @@ def capture(block) # thread, fileutils, etc), so it's worth delaying it until this point. require 'tempfile' - original_stream = stream.clone + stream = name == 'stdout' ? STDOUT : STDERR # rubocop:ignore Style/GlobalStdStream captured_stream = Tempfile.new(name) begin + if name == 'stdout' + $stdout = captured_stream + else + $stderr = captured_stream + end captured_stream.sync = true stream.reopen(captured_stream) block.call captured_stream.rewind captured_stream.read ensure - stream.reopen(original_stream) + stream.reopen(stream) captured_stream.close captured_stream.unlink + if name == 'stdout' + $stdout = stream + else + $stderr = stream + end end end end