From cda1c904d257ec4c48ebedec2d7023eb6a653c24 Mon Sep 17 00:00:00 2001 From: Eric Mueller Date: Sat, 18 May 2024 23:24:24 -0400 Subject: [PATCH 1/6] Add tests for compound matching on Output matchers This is the issue reported in #1391 - expecting like expect { puts "foobar" } .to output.to_stdout(/foo/) .and output.to_stdout(/bar/) fails, because the two matchers get nested, and inner matcher catches the stream write, hiding it from the outer one. --- spec/rspec/matchers/built_in/output_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spec/rspec/matchers/built_in/output_spec.rb b/spec/rspec/matchers/built_in/output_spec.rb index 3873013d1..9035aa095 100644 --- a/spec/rspec/matchers/built_in/output_spec.rb +++ b/spec/rspec/matchers/built_in/output_spec.rb @@ -139,6 +139,12 @@ def invalid_block }.to fail_including("expected block to not output a string starting with \"f\" to #{stream_name}, but output \"foo\"\nDiff") end end + + context "expect { ... }.to output(matcher1).#{matcher_method}.and output(matcher2).#{matcher_method}" do + it "passes if the block outputs lines to #{stream_name} matching both matchers" do + expect { print_to_stream "foo_bar" }.to matcher(/foo/).and matcher(/bar/) + end + end end module RSpec From 1bc9e69ee3faaca40622386d3f35c6d6a78e0269 Mon Sep 17 00:00:00 2001 From: Eric Mueller Date: Sat, 18 May 2024 23:26:36 -0400 Subject: [PATCH 2/6] Pass output along, if not in the outer matcher For the simple matchers, the solution is straightforward (and @pirj supplied it in the issue thread). Pass the output along to the original stream after storing it, unless the original stream is the _real_ original stream. --- lib/rspec/matchers/built_in/output.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/rspec/matchers/built_in/output.rb b/lib/rspec/matchers/built_in/output.rb index 888ccaee2..cda15a2f4 100644 --- a/lib/rspec/matchers/built_in/output.rb +++ b/lib/rspec/matchers/built_in/output.rb @@ -186,6 +186,7 @@ def capture(block) captured_stream.string ensure $stdout = original_stream + $stdout.write(captured_stream.string) unless $stdout == STDOUT end end @@ -209,6 +210,7 @@ def capture(block) captured_stream.string ensure $stderr = original_stream + $stderr.write(captured_stream.string) unless $stderr == STDERR end end From 7e7d4584312bdb482702c17c83cbd34032b3a944 Mon Sep 17 00:00:00 2001 From: Eric Mueller Date: Sun, 19 May 2024 00:44:30 -0400 Subject: [PATCH 3/6] Nearly support compound matchers on CaptureStreamToTempfile Sadly, it doesn't quite work on StdErrSplitter, because of the existing (but possibly non-impactful?) bug around restoring the original stream during the ensure block. --- lib/rspec/matchers/built_in/output.rb | 38 ++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/lib/rspec/matchers/built_in/output.rb b/lib/rspec/matchers/built_in/output.rb index cda15a2f4..218d593dc 100644 --- a/lib/rspec/matchers/built_in/output.rb +++ b/lib/rspec/matchers/built_in/output.rb @@ -186,7 +186,7 @@ def capture(block) captured_stream.string ensure $stdout = original_stream - $stdout.write(captured_stream.string) unless $stdout == STDOUT + $stdout.write(captured_stream.string) unless $stdout == STDOUT # rubocop:disable Style/GlobalStdStream end end @@ -210,7 +210,7 @@ def capture(block) captured_stream.string ensure $stderr = original_stream - $stderr.write(captured_stream.string) unless $stderr == STDERR + $stderr.write(captured_stream.string) unless $stderr == STDERR # rubocop:disable Style/GlobalStdStream end end @@ -225,21 +225,47 @@ def capture(block) # thread, fileutils, etc), so it's worth delaying it until this point. require 'tempfile' + # This is.. awkward-looking. But it's written this way because of how + # compound matchers work - we essentially need to be able to tell if + # we're in an _inner_ matcher, so we can pass the stream-output along + # to the outer matcher for further evaluation in that case. Added to + # that, it's fairly difficult to _tell_, because the only actual state + # we have access to is the stream itself, and in the case of stderr, + # that stream is really a RSpec::Support::StdErrSplitter (which is why + # we're testing `is_a?(File)` in such an obnoxious way). + inner_matcher = stream.to_io.is_a?(File) + + # Careful here - the StdErrSplitter is what is being cloned; we're + # relying on the implemented clone method of that class (in + # rspec-support) to actually clone the File for ensure-reopen. original_stream = stream.clone + captured_stream = Tempfile.new(name) begin captured_stream.sync = true stream.reopen(captured_stream) block.call - captured_stream.rewind - captured_stream.read + read_contents(captured_stream) ensure + captured_content = inner_matcher ? read_contents(captured_stream) : nil stream.reopen(original_stream) - captured_stream.close - captured_stream.unlink + stream.write(captured_content) if captured_content + clean_up_tempfile(captured_stream) end end + + private + + def read_contents(strm) + strm.rewind + strm.read + end + + def clean_up_tempfile(tempfile) + tempfile.close + tempfile.unlink + end end end end From 4175430880e9c3a1ac9e9d2d4b6d16c64414e8f2 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Sun, 18 Aug 2024 14:38:57 +0300 Subject: [PATCH 4/6] Add a changelog entry --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index 6196f978c..9258b0d9c 100644 --- a/Changelog.md +++ b/Changelog.md @@ -12,6 +12,7 @@ Enhancements: * Improve the IO emulation in the output capture matchers (`output(...).to_stdout` et al) by adding `as_tty` and `as_not_tty` to change the `tty?` flags. (Sergio Gil Pérez de la Manga, #1459) +* Add support for compound output matchers. (Eric Mueller, #1460) ### 3.13.1 / 2024-06-13 [Full Changelog](http://github.com/rspec/rspec-expectations/compare/v3.13.0...v3.13.1) From 829f7fec70764ff6850b9db3ea5f2a6c94ce2d82 Mon Sep 17 00:00:00 2001 From: Eric Mueller Date: Sun, 18 Aug 2024 14:43:51 -0400 Subject: [PATCH 5/6] Mark the two compound-stream matcher tests pending for jruby --- spec/rspec/matchers/built_in/output_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/rspec/matchers/built_in/output_spec.rb b/spec/rspec/matchers/built_in/output_spec.rb index 9035aa095..68a1b1672 100644 --- a/spec/rspec/matchers/built_in/output_spec.rb +++ b/spec/rspec/matchers/built_in/output_spec.rb @@ -141,7 +141,7 @@ def invalid_block end context "expect { ... }.to output(matcher1).#{matcher_method}.and output(matcher2).#{matcher_method}" do - it "passes if the block outputs lines to #{stream_name} matching both matchers" do + it "passes if the block outputs lines to #{stream_name} matching both matchers", :pending => RSpec::Support::Ruby.jruby? && matcher_method =~ /any_process/ do expect { print_to_stream "foo_bar" }.to matcher(/foo/).and matcher(/bar/) end end From d44f61f70b569d187093dd1ddb74a57535aecf24 Mon Sep 17 00:00:00 2001 From: Eric Mueller Date: Wed, 21 Aug 2024 13:21:33 -0400 Subject: [PATCH 6/6] Use 'captured_stream' instead of 'strm' (avoid abbreviations) --- lib/rspec/matchers/built_in/output.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rspec/matchers/built_in/output.rb b/lib/rspec/matchers/built_in/output.rb index 218d593dc..503ebaa1f 100644 --- a/lib/rspec/matchers/built_in/output.rb +++ b/lib/rspec/matchers/built_in/output.rb @@ -257,9 +257,9 @@ def capture(block) private - def read_contents(strm) - strm.rewind - strm.read + def read_contents(captured_stream) + captured_stream.rewind + captured_stream.read end def clean_up_tempfile(tempfile)