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

raise_error with ArgumentError fails with incorrect message #1388

Closed
denisdefreyne opened this issue Nov 12, 2022 · 14 comments
Closed

raise_error with ArgumentError fails with incorrect message #1388

denisdefreyne opened this issue Nov 12, 2022 · 14 comments

Comments

@denisdefreyne
Copy link

Subject of the issue

A raise_error expectation with ArgumentError will not match the correct message.

Your environment

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

Steps to reproduce

Write the following to spec/foo_spec.rb:

# frozen_string_literal: true

describe 'Foo' do
  example do
    expect { raise ArgumentError, 'foo' }
      .to raise_error(ArgumentError, 'foo')
  end
end

Run bundle exec rspec spec/foo_spec.rb.

Expected behavior

Spec passes.

Actual behavior

  1) Foo is expected to raise ArgumentError with "foo"
     Failure/Error:
       expect { raise ArgumentError, 'foo' }
         .to raise_error(ArgumentError, 'foo')
     
       expected ArgumentError with "foo", got #<ArgumentError: foo
     
           expect { raise ArgumentError, 'foo' }
                          ^^^^^^^^^^^^^^^^^^^^> with backtrace:
         # ./spec/foo_spec.rb:5:in `block (3 levels) in <top (required)>'
         # /Users/denis/.rubies/ruby-3.1.2/lib/ruby/gems/3.1.0/gems/rspec-expectations-3.12.0/lib/rspec/matchers/built_in/raise_error.rb:59:in `matches?'

The message of the raised ArgumentError is a String with the following contents:

foo

    expect { raise ArgumentError, 'foo' }
                   ^^^^^^^^^^^^^^^^^^^^

More details

This only appears to be an issue for ArgumentError.

This started failing in the week or so. I’ve not been able to narrow down the cause just yet.

This fails with stock Ruby, no gems loaded.

To reproduce without RSpec:

begin
  raise ArgumentError, 'zing'
rescue => e
  p e.message
end

This prints "zing\n\n raise ArgumentError, 'zing'\n ^^^^^^^^^^^^^^^^^^^^^".

It should (I think) print "zing" instead.

@denisdefreyne
Copy link
Author

I’m 99% sure this is caused by error_highlight.

I’m not sure whether this really is a RSpec issue, but… if it is not, would it be good for rspec-expectation to handle it anyway?

@denisdefreyne
Copy link
Author

denisdefreyne commented Nov 12, 2022

This is likely a duplicate of #1362.

@denisdefreyne
Copy link
Author

It looks like error_highlight version 0.5 (released on November 08, 2022) made this break. Most likely candidate for the cause is this commit: ruby/error_highlight@defcaf1

@denisdefreyne
Copy link
Author

Relevant issue in error_highlight repo: ruby/error_highlight#28

@pirj
Copy link
Member

pirj commented Nov 12, 2022

Thanks for reporting.
I'm a bit confused by examples in the description of a relevant error_highlight issue:

works without failures with error_highlight 0.4.0
gem install error_highlight -v 0.5.0

Is this right? Do I understand it correctly that there's a regression in error_highlight 0.5.0?

Would version pinning in Rails

gem "error_highlight", ">= 0.4.0", "< 0.5.0"

fix the issue? Can we just recommend using the next Rails version to fix this for our users?

I would like to point out that rspec-expectations do not depend on error_highlight, and pinning it here would make no sense.

If you run your example on a blank slate project without error_highlight, it would pass.

A related fix mentioning error_highlight.

Do you think there is anything we can fix by making changes in rspec-expectations, @denisdefreyne ?

@denisdefreyne
Copy link
Author

denisdefreyne commented Nov 12, 2022

That is correct:

  • With error_highlight 0.5, this issue exists.
  • With error_highlight 0.4, this issue does not exist.

Do I understand it correctly that there's a regression in error_highlight 0.5.0?

Yes

Would version pinning in Rails […] fix the issue?

I can definitely pin the error_highlight gem to 0.4.x in my own (non-Rails) projects, though that would only solve the problem for these personal projects.

If you run your example on a blank slate project without error_highlight, it would pass.

Yes and no -- error_highlight is part of Ruby (at least version 0.3) and required by default. Luckily, versions 0.3 and 0.4 do not exhibit this problem, but if you would have error_highlight 0.5 installed, the example would fail even without invoking any requires.

Do you think there is anything we can fix by making changes in rspec-expectations

It’s a good question! It’s also a question that might not need to be answered right now, depending on how/if ruby/error_highlight#28 gets resolved.

As a hack, I imagine rspec-expectations could strip off a trailing \n\n.*\n\s*\^+ regex -- but oh, I do not like that hack. 😅

@denisdefreyne
Copy link
Author

I think the better way forward might be to encourage people to not write

.to raise_error(ArgumentError, 'foo')

but rather use an anchored regex:

.to raise_error(ArgumentError, /\Afoo$/)

@JonRowe
Copy link
Member

JonRowe commented Nov 12, 2022

Is it just new line characters added?

@yahonda
Copy link

yahonda commented Nov 14, 2022

I believe error_highlight 0.5.1 including ruby/error_highlight#29 address this issue.

@pirj
Copy link
Member

pirj commented Nov 14, 2022

Thank you, @yahonda . I also see a Rails PR.
So what most users would have to do is to:

bundle update --conservative error_highlight

Is this correct?

Is there anything left to be done in rspec-expectations?

@yahonda
Copy link

yahonda commented Nov 14, 2022

Just updating error_highlight version to 0.5.1 is fine.

  • Without bundler
gem update error_highlight
  • Using bundler
bundle update --conservative error_highlight

--conservative option is not mandatory. I added it because I just want to update error_highlight gem. Refer to https://bundler.io/v1.14/whats_new.html#conservative-updates

@pirj
Copy link
Member

pirj commented Nov 14, 2022

@denisdefreyne @JonRowe Any objections on closing this issue?

@JonRowe JonRowe closed this as completed Nov 14, 2022
@JonRowe
Copy link
Member

JonRowe commented Nov 14, 2022

I think as error_highlight has released a change fixing this (unintentional breaking of #message) this is closable by us.

@denisdefreyne
Copy link
Author

Perfect, thanks :)

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

No branches or pull requests

4 participants