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

Allow configuring exceptions raise_error should capture to display webmock errors correctly #137

Open
grosser opened this issue Jun 4, 2020 · 9 comments

Comments

@grosser
Copy link
Contributor

grosser commented Jun 4, 2020

I'm doing: expect { call }.to raise_error(/ABORTED/)

this triggers a http request and then it ends up as:

       expected /ABORTED/, got #<WebMock::NetConnectNotAllowedError: Real HTTP connections are disabled. Unregistered request: GET h...e.sk/projects/sfn_kubernetes/stages")

but what I want to see is:

     WebMock::NetConnectNotAllowedError:
       Real HTTP connections are disabled. Unregistered request: GET https://samson.zende.sk/locks with body '{"per_page":100,"page":1}' with headers {'Accept'=>'application/json', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Authorization'=>'Bearer a', 'Content-Type'=>'application/json', 'User-Agent'=>'Ruby'}

       You can stub this request with the following snippet:
blah blah

... this does not surface though since raise_error captures Exception and not StandardError like minitest does.
I assume changing the default would be too disruptive, so I'd suggest we make that configureable ?

workaround I'm using atm:

  undef :raise_error
  undef :raise_exception
  def expect_error(regex)
    yield
  rescue StandardError => e
    raise unless e.message.match?(regex)
  else
    raise "Nothing raised"
  end
@pirj
Copy link
Member

pirj commented Jun 5, 2020

Would you consider something like this:

def raise_standard_error(message, &block)
  raise_error(StandardError, message, &block)
end

There's also this option if you're not happy with clipping of the error text https://github.com/rspec/rspec-expectations/blob/eb1787f0e1a5ec2c2650048ee60a45d4c9738d78/lib/rspec/expectations/configuration.rb#L59.

What makes you uncomfortable with the current behaviour specifically, how does it slow you down exactly?

@grosser
Copy link
Contributor Author

grosser commented Jun 5, 2020

raise_error(StandardError, message, &block) does not work since it would still capture all exception and not show the full context of the error message.

The current behavior makes debugging webmock errors hard because it clips the error message.
Changing the max_formatted_output_length would work, but means I have to change it globally to ~2000 and then other tests get more output too.

So ideally I'd like to opt-out of the "capture everything" so I get the raw error message :)

@pirj
Copy link
Member

pirj commented Jun 11, 2020

max_formatted_output_length would work, but means I have to change it globally

Not necessarily, you can change it locally to webmocked examples only:

RSpec.configure do |config|
  around(:example, :webmock) do |example|
    old = Rails.configuration.max_formatted_output_length
    Rails.configuration.max_formatted_output_length = 2000
    example.run
  ensure
    Rails.configuration.max_formatted_output_length = old
  end
end

@grosser
Copy link
Contributor Author

grosser commented Jun 11, 2020

hmm yeah that could work, but I'm more happier with my workaround script, seems simpler/cleaner and removes a level of indirection

@pirj
Copy link
Member

pirj commented Jun 11, 2020

Got it.
Do you think there's anything we can do code-wise, or should we close this?

@grosser
Copy link
Contributor Author

grosser commented Jun 16, 2020

Not rescuing Exception by default would be great, but could break a lot of tests that play with SystemExit.

Splitting raise_error and raise_exception into "Exception" and "StandardError" would be interesting but maybe confusing.

A global setting for "raise_exception_base: StandardError" would be nice and not break anyones workflow.

@JonRowe
Copy link
Member

JonRowe commented Jun 18, 2020

Unfortunately thats not possible, as it would mean there is a state where the matcher wouldn't pass, but wouldn't fail either. I don't want to open that can of worms. I'm looking at an alternative formatting for the error instead. e.g.

expected /ABORTED/, got #<WebMock::NetConnectNotAllowedError> with message:
  #
  # Real HTTP connections are disabled. Unregistered request: GET https://samson.zende.sk/locks with body '{"per_page":100,"page":1}' with headers {'Accept'=>'application/json', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Authorization'=>'Bearer a', 'Content-Type'=>'application/json', 'User-Agent'=>'Ruby'}
  #
  # You can stub this request with the following snippet:
  #    blah blah

@grosser
Copy link
Contributor Author

grosser commented Jun 20, 2020

oh yeah, that looks nice!

@genehsu
Copy link

genehsu commented Oct 27, 2021

I added a sample implementation idea. I need to generate a few Webmock errors to really see if this idea works in practice.

@JonRowe JonRowe transferred this issue from rspec/rspec-expectations Nov 30, 2024
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.

4 participants