Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compound expectations of OutputMatcher do not print a diff in their failure output, but Single expectations do #1406

Closed
henrahmagix opened this issue Feb 24, 2023 · 9 comments · Fixed by #1440

Comments

@henrahmagix
Copy link

Subject of the issue

Using output('something').to_stdout.and output('other thing').to_stderr does not print a helpful diff when either of both of the assertions fail.

Using output('something').to_stdout or output('other thing').to_stderr individually always print a helpful diff when they fail.

This means a helpful failure diff is lost if I want to assert on both stdout and stderr in the same assertion. I'm currently testing on large output that gets truncated, so the diff failure output is extremely helpful to me =)

A workaround I thought about is to have two separate assertions, but then that means the assertion with .to_stdout causes stderr to be printed to the terminal that runs the test, and vice versa for .to_stderr allowing stdout to be printed to the terminal.

But it was too much output and very messy to inspect the full output at the end of it. I'd rather have both stdout and stderr suppressed (and captured of course) so they can be diffed when an assertion fails =)

Your environment

  • Ruby version: ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [arm64-darwin21]
  • rspec-expectations version: 3.12.2

Steps to reproduce

# frozen_string_literal: true

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"

  gem "rspec", "3.12.0" # Activate the gem and version you are reporting the issue against.
end

puts "Ruby version is: #{RUBY_VERSION}"
require 'rspec/autorun'

RSpec.describe 'compount output matchers' do
  it 'prints a diff failure when .to_stdout is used individually' do
    expect { puts "out " * 50 }.to output('hello').to_stdout
  end

  it 'prints a diff failure when .to_stderr is used individually' do
    expect { warn "err " * 50 }.to output('hello').to_stderr
  end

  it 'prints a diff failure when both .to_stdout and .to_stderr are combined' do
    expect { puts "out " * 50; warn "err " * 50 }.to output('hello').to_stdout.and output('hello').to_stderr
  end
end

Expected behavior

The failure output in the specific example above isn't particularly representative of how helpful the diff is, but when there are many, many lines of different content, it's very helpful =)

Failures:

  1) compount output matchers prints a diff failure when .to_stdout is used individually
     Failure/Error: expect { puts "out " * 50 }.to output('hello').to_stdout

       expected block to output "hello" to stdout, but output "out out out out out out out out out out out out out out out out out out out out out out out out out ... out out out out out out out out out out out out out out out out out out out out out out out out \n"
       Diff:
       @@ -1 +1 @@
       -hello
       +out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out

     # rspec-diff.rb:21:in `block (2 levels) in <main>'

  2) compount output matchers prints a diff failure when .to_stderr is used individually
     Failure/Error: expect { warn "err " * 50 }.to output('hello').to_stderr

       expected block to output "hello" to stderr, but output "err err err err err err err err err err err err err err err err err err err err err err err err err ... err err err err err err err err err err err err err err err err err err err err err err err err \n"
       Diff:
       @@ -1 +1 @@
       -hello
       +err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err

     # rspec-diff.rb:25:in `block (2 levels) in <main>'

  3) compount output matchers prints a diff failure when both .to_stdout and .to_stderr are combined
     Failure/Error: expect { puts "out " * 50; warn "err " * 50 }.to output('hello').to_stdout.and output('hello').to_stderr

          expected block to output "hello" to stdout, but output "out out out out out out out out out out out out out out out out out out out out out out out out out ... out out out out out out out out out out out out out out out out out out out out out out out out \n"
          Diff:
          @@ -1 +1 @@
          -hello
          +out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out

       ...and:

          expected block to output "hello" to stderr, but output "err err err err err err err err err err err err err err err err err err err err err err err err err ... err err err err err err err err err err err err err err err err err err err err err err err err \n"
          Diff:
          @@ -1 +1 @@
          -hello
          +err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err
     # rspec-diff.rb:29:in `block (2 levels) in <main>'

Actual behavior

Failures:

  1) compount output matchers prints a diff failure when .to_stdout is used individually
     Failure/Error: expect { puts "out " * 50 }.to output('hello').to_stdout

       expected block to output "hello" to stdout, but output "out out out out out out out out out out out out out out out out out out out out out out out out out ... out out out out out out out out out out out out out out out out out out out out out out out out \n"
       Diff:
       @@ -1 +1 @@
       -hello
       +out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out out

     # rspec-diff.rb:21:in `block (2 levels) in <main>'

  2) compount output matchers prints a diff failure when .to_stderr is used individually
     Failure/Error: expect { warn "err " * 50 }.to output('hello').to_stderr

       expected block to output "hello" to stderr, but output "err err err err err err err err err err err err err err err err err err err err err err err err err ... err err err err err err err err err err err err err err err err err err err err err err err err \n"
       Diff:
       @@ -1 +1 @@
       -hello
       +err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err err

     # rspec-diff.rb:25:in `block (2 levels) in <main>'

  3) compount output matchers prints a diff failure when both .to_stdout and .to_stderr are combined
     Failure/Error: expect { puts "out " * 50; warn "err " * 50 }.to output('hello').to_stdout.and output('hello').to_stderr

          expected block to output "hello" to stdout, but output "out out out out out out out out out out out out out out out out out out out out out out out out out ... out out out out out out out out out out out out out out out out out out out out out out out out \n"

       ...and:

          expected block to output "hello" to stderr, but output "err err err err err err err err err err err err err err err err err err err err err err err err err ... err err err err err err err err err err err err err err err err err err err err err err err err \n"
     # rspec-diff.rb:29:in `block (2 levels) in <main>'
@henrahmagix
Copy link
Author

I've done some debugging and have found that when the failure output lands in RSpec::Support::Differ#diff here, then it finds actual is a Proc, whereas I believe by that point it should be the String that OutputMatcher captures. If it was a string, the diff output would succeed =)

@pirj
Copy link
Member

pirj commented Feb 24, 2023

Same as rspec/rspec#89?
There's a suggestion how to fix this in the comments, a PR is welcome.

@pirj pirj closed this as completed Feb 24, 2023
@henrahmagix
Copy link
Author

@pirj i saw that issue but I don’t believe this is the same issue. The capturing of the output is ok, no problems there. It’s just on the diff part of the failure message where there’s an issue, so this is separate and different, and the fix commented in that issue doesn’t apply here.

i.E. the assertions are fine; it’s the failure output that can be improved :)

I’ll see if I can make a fix but I got so dizzy trawling through the code to find where actual can be set to a string instead, so I paused that and made this issue in case anyone had quick insider knowledge on where the fix needs to go.

@pirj
Copy link
Member

pirj commented Feb 24, 2023

Fair enough.

@pirj pirj reopened this Feb 24, 2023
@pirj
Copy link
Member

pirj commented Feb 24, 2023

#1319 fixes this. But it's targeted to RSpec 4.

  1) can capture stdin and stderr prints diff for both when both fail
     Failure/Error:
       expect {
         print "foo"
         $stderr.print("bar")
       }
         .to output(/baz/).to_stdout
         .and output(/qux/).to_stderr

          expected block to output /baz/ to stdout, but output "foo"

       ...and:

          expected block to output /qux/ to stderr, but output "bar"
       Diff for (output /baz/ to stdout):
       @@ -1 +1 @@
       -/baz/
       +"foo"

       Diff for (output /qux/ to stderr):
       @@ -1 +1 @@
       -/qux/
       +"bar"

@henrahmagix
Copy link
Author

Ah wonderful! Thanks @pirj =) I've looked at that PR and it seems like it might be applicable to RSpec 3 too? Do you know if that's something you and the team would look at backporting? If not, is there a vague milestone or date for RSpec 4 to be released? Thanks v much =)

@pirj
Copy link
Member

pirj commented Feb 24, 2023

It was quite some time ago, and I don't see any discussion why we've explicitly decided to target 4.0.

@JonRowe WDYT of releasing #1319 in 3.x? I could quickly rebase it on main and re-target.

@henrahmagix
Copy link
Author

Would really love this in 3.x if it's an easy rebase ☺️

henrahmagix pushed a commit to henrahmagix/rspec-expectations that referenced this issue Dec 17, 2023
henrahmagix pushed a commit to henrahmagix/rspec-expectations that referenced this issue Dec 17, 2023
pirj added a commit to henrahmagix/rspec-expectations that referenced this issue Dec 28, 2023
pirj added a commit to henrahmagix/rspec-expectations that referenced this issue Dec 28, 2023
@henrahmagix
Copy link
Author

🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants