Introduce have_reported_error matcher #2849

skatkov · 2025-06-04T22:53:33Z

rspec-rails is missing support for Rails ErrorReporter, this was introduced to rails in v7 and has been evolving ever since. With my client, we have moved to using ErrorReporter as a unified error reporting interface, so we can easily move from one error tracking software to another with minimal code changes. And we had a need to test this interface with rspec, so we implemented our own matcher to handle this.

I'm suggesting our internal implementation as is. This is probably not suitable as is for this gem, but I'd like to open discussion with this starting point.

Example usage

@example Checking for any error
  expect { Rails.error.report(StandardError.new) }.to have_reported_error

@example Checking for specific error class
  expect { Rails.error.report(MyError.new) }.to have_reported_error(MyError)

@example Checking for specific error instance with message
  expect { Rails.error.report(MyError.new("message")) }.to have_reported_error(MyError.new("message"))

@example Checking error attributes
  expect { Rails.error.report(StandardError.new, context: "test") }.to have_reported_error.with(context: "test")

@example Checking error message patterns
  expect { Rails.error.report(StandardError.new("test message")) }.to have_reported_error(/test/)

@example Negation
  expect { "safe code" }.not_to have_reported_error

pirj

Thank you!

spec/rspec/rails/matchers/have_reported_error_spec.rb

features/matchers/README.md

lib/rspec/rails/matchers/have_reported_error.rb

pirj · 2025-06-21T06:59:54Z

lib/rspec/rails/matchers/have_reported_error.rb

+          return true if @attributes.empty?
+          return false if @error_subscriber.events.empty?
+
+          event_context = @error_subscriber.events.last[1].with_indifferent_access["context"]


Does it lock and match only on the last one reported?

@pirj yeah, it does. I've been debating this, but this works in our case just fine.

But questions keep popping in my head. Should we look through array of errors? what to do if there will be more than 1? Should we match amount of errors too?

I don't have good answers for these. So this is my dumbest approach -- checking last error only.

We should match like we do with scheduled jobs, if there is a matching one among reported, it matches.

I don’t see a reason to add count matches now. This can be added later.

lib/rspec/rails/matchers/have_reported_error.rb

spec/rspec/rails/matchers/have_reported_error_spec.rb

features/matchers/README.md

features/matchers/have_reported_error_matcher.feature

JonRowe · 2025-06-25T17:14:51Z

features/matchers/have_reported_error_matcher.feature

+  Scenario: Using in controller specs
+    Given a file named "spec/controllers/users_controller_spec.rb" with:
+      """ruby
+      require "rails_helper"
+
+      RSpec.describe UsersController, type: :controller do
+        describe "POST #create" do
+          it "reports validation errors" do
+            expect {
+              post :create, params: { user: { email: "invalid" } }
+            }.to have_reported_error(ValidationError)
+          end
+        end
+      end
+      """
+    When I run `rspec spec/controllers/users_controller_spec.rb`
+    Then the examples should all pass
+
+  Scenario: Using in request specs
+    Given a file named "spec/requests/users_spec.rb" with:
+      """ruby
+      require "rails_helper"
+
+      RSpec.describe "Users", type: :request do
+        describe "POST /users" do
+          it "reports processing errors" do
+            expect {
+              post "/users", params: { user: { name: "Test" } }
+            }.to have_reported_error.with(context: "user_creation")
+          end
+        end
+      end
+      """
+    When I run `rspec spec/requests/users_spec.rb`
+    Then the examples should all pass
+


This isn't a controller / request specific matcher so these feel out of place, why these, why not every aspect of Rails... I'd just cut them.

spec/rspec/rails/matchers/have_reported_error_spec.rb

JonRowe · 2025-06-25T17:25:27Z

👋 I think this would be a great addition, sorry for the size of the review its been on my todo list for a while, its mostly just grammar / wording tweaks plus a few "fit" things.

The one change I do want to see though is dropping instance matching, I don't think it makes sense over just providing class / message / using with.

Commit grammar improvements Co-authored-by: Jon Rowe <mail@jonrowe.co.uk>

skatkov · 2025-06-27T11:45:47Z

@JonRowe thanks for review.

The one change I do want to see though is dropping instance matching, I don't think it makes sense over just providing class / message / using with.

Which of these you would rather go with?
.have_reported_error(StandardError, /wrong attributes/).with(param1: 'test')

or

.have_reported_error(StandardError).with(message:/wrong attributes/, params1: 'test1')

??

I assume, that first option is prefered to keep similarity with raised_error matcher.
https://rspec.info/features/3-13/rspec-expectations/built-in-matchers/raise-error/

JonRowe · 2025-06-27T11:59:45Z

Given that you already have with, I think:

.have_reported_error(StandardError).with(/wrong attributes/, params1: 'test1')

But if you insisted on message: in with I could be persuaded

pirj · 2025-06-27T14:31:21Z

It makes sense to separate matching of args passed to report and exception’s own args, including the message. Both can receive args, right? It would open a way for ambiguity and confusion.
raise_error accepts have_attributes as an argument matcher. It supports with_message. Wdyt if we reserve with for additional args passed to report, but keep it separate from with_message?

skatkov · 2025-06-29T18:44:41Z

It makes sense to separate matching of args passed to report and exception’s own args, including the message. Both can receive args, right? It would open a way for ambiguity and confusion.

The only argument that the exception class accepts is "message". We can of course subclass it and support way more than that, but I'm not sure if we should support this in a matcher?

I have a feeling, that having a matcher interface similar to raise_error would make it simpler for people to memorize it. I don't mind changing .with to .have_attributes as well.

JonRowe · 2025-06-30T08:02:50Z

I hadn't clocked that the extra attributes were in addition to the error, I think matching the raise_error interface in terms of report_error(ErrorClass, message_or_regexp) makes sense with the existing with becoming something clearer, with_metadata? What does Rails call the extra data?

skatkov · 2025-06-30T09:58:19Z

Here is a source code for Rails.error.report.

https://github.com/rails/rails/blob/36601bbb02bc3570f5609db2127c77afca575d6c/activesupport/lib/active_support/error_reporter.rb#L233C23-L233C112

These attributes in rails case have a name "context". so with_context might be more appropriate.

pirj · 2025-07-02T05:37:44Z

lib/rspec/rails/matchers/have_reported_error.rb

+      # @api private
+      # Sentinel value to distinguish between no argument passed vs explicitly passed nil.
+      # This follows the same pattern as RSpec's raise_error matcher.
+      UndefinedValue = Object.new.freeze


Such a constant is already defined in the basematcher, do we need another one?

pirj · 2025-07-02T05:40:32Z

lib/rspec/rails/matchers/have_reported_error.rb

          @actual_error = nil
          @attributes = {}
          @error_subscriber = nil

          case expected_error_or_message
-          when nil
+          when nil, UndefinedValue


We usually make a distinction between those two, where nil is typically a sign of a mistake. For some matchers, we warn if nil is passed. Do you think there can be legitimate cases of passing nil as an argument?

Yeah, similar to error_raised you can just expect an error to be raise, but don't care which one.

The thing is raise_error() is to expect any exception, no matter which; but raise_error(nil) is usually an indication of a mistake in the spec itself. See rspec/rspec-expectations#1142
This warning may explain it better.

Similarly, expect { report… }.to have_reported_error() should pass when any kind of error has been reported, while passing a nil as the first argument should emit a warning.

skatkov force-pushed the have-reported-error branch from 8b837a8 to d2d0e51 Compare June 7, 2025 20:44

pirj reviewed Jun 7, 2025

View reviewed changes

pirj reviewed Jun 21, 2025

View reviewed changes

lib/rspec/rails/matchers/have_reported_error.rb Outdated Show resolved Hide resolved

pirj reviewed Jun 21, 2025

View reviewed changes

lib/rspec/rails/matchers/have_reported_error.rb Outdated Show resolved Hide resolved

pirj reviewed Jun 21, 2025

View reviewed changes

lib/rspec/rails/matchers/have_reported_error.rb Show resolved Hide resolved

pirj reviewed Jun 21, 2025

View reviewed changes

lib/rspec/rails/matchers/have_reported_error.rb Outdated Show resolved Hide resolved

skatkov force-pushed the have-reported-error branch 2 times, most recently from b38ff1c to 87ebd2a Compare June 22, 2025 20:50

skatkov added 12 commits June 22, 2025 22:59

first pass at have_reported_error matcher

5e6efcc

Adding a feature

5dd2c5c

simplify matcher/readmer.md

ce1a162

Remove mocks out of tests

550c25e

test failure messages better

e074991

rewrite specs

1fe7e91

should require a block matcher

6b41d2b

Create ErrorEvent struct to store errors

3859b04

simplify matcher

8352668

disable matcher chaining

030c27a

improved argument error message in case block is not passed to matcher

2b9bd02

further clean-up

6135d5d

skatkov force-pushed the have-reported-error branch from 71685ad to 6135d5d Compare June 22, 2025 20:59

pirj reviewed Jun 23, 2025

View reviewed changes

lib/rspec/rails/matchers/have_reported_error.rb Outdated Show resolved Hide resolved

JonRowe requested changes Jun 25, 2025

View reviewed changes

skatkov and others added 2 commits June 25, 2025 23:21

Apply suggestions from code review

8335664

Commit grammar improvements Co-authored-by: Jon Rowe <mail@jonrowe.co.uk>

clean-up features

a14f5c7

skatkov added 6 commits July 1, 2025 23:59

remove a require from test

af58d83

rename .with to .with_context

7259a25

have_reported_error to accept class name and a message

1b6deb6

switch to error_report matcher type of api

c77fca5

refactor

f5e427a

Using UndefinedValue as a default attribute

7b3abc5

skatkov force-pushed the have-reported-error branch 3 times, most recently from c1134da to f90fc3b Compare July 1, 2025 23:20

Look through all errors, don't just go with last error

2488a71

skatkov force-pushed the have-reported-error branch from f90fc3b to 2488a71 Compare July 1, 2025 23:28

pirj reviewed Jul 2, 2025

View reviewed changes

Uh oh!

Introduce have_reported_error matcher #2849

Are you sure you want to change the base?

Introduce have_reported_error matcher #2849

Uh oh!

Conversation

skatkov commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Example usage

Uh oh!

pirj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pirj Jun 21, 2025

Choose a reason for hiding this comment

Uh oh!

skatkov Jun 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pirj Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JonRowe Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JonRowe commented Jun 25, 2025

Uh oh!

skatkov commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JonRowe commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pirj commented Jun 27, 2025

Uh oh!

skatkov commented Jun 29, 2025

Uh oh!

JonRowe commented Jun 30, 2025

Uh oh!

skatkov commented Jun 30, 2025

Uh oh!

pirj Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

pirj Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

skatkov Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

pirj Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

skatkov commented Jun 4, 2025 •

edited

Loading

skatkov Jun 22, 2025 •

edited

Loading

skatkov commented Jun 27, 2025 •

edited

Loading

JonRowe commented Jun 27, 2025 •

edited

Loading