From 5e6efcc3bdd372914aca610dae61fbfcd0810ff9 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Thu, 5 Jun 2025 00:36:36 +0200 Subject: [PATCH 01/22] first pass at have_reported_error matcher --- lib/rspec/rails/matchers.rb | 1 + .../rails/matchers/have_reported_error.rb | 193 +++++++++++++++ .../matchers/have_reported_error_spec.rb | 233 ++++++++++++++++++ 3 files changed, 427 insertions(+) create mode 100644 lib/rspec/rails/matchers/have_reported_error.rb create mode 100644 spec/rspec/rails/matchers/have_reported_error_spec.rb diff --git a/lib/rspec/rails/matchers.rb b/lib/rspec/rails/matchers.rb index fb297eabc..36fa5c0b3 100644 --- a/lib/rspec/rails/matchers.rb +++ b/lib/rspec/rails/matchers.rb @@ -20,6 +20,7 @@ module Matchers require 'rspec/rails/matchers/relation_match_array' require 'rspec/rails/matchers/be_valid' require 'rspec/rails/matchers/have_http_status' +require 'rspec/rails/matchers/have_reported_error' require 'rspec/rails/matchers/send_email' if RSpec::Rails::FeatureCheck.has_active_job? diff --git a/lib/rspec/rails/matchers/have_reported_error.rb b/lib/rspec/rails/matchers/have_reported_error.rb new file mode 100644 index 000000000..a1ec27844 --- /dev/null +++ b/lib/rspec/rails/matchers/have_reported_error.rb @@ -0,0 +1,193 @@ +require "rspec/rails/matchers/base_matcher" + +module RSpec + module Rails + module Matchers + # @private + class ErrorSubscriber + attr_reader :events + + def initialize + @events = [] + end + + def report(error, **attrs) + @events << [error, attrs] + end + end + + # Matcher class for `have_reported_error`. Should not be instantiated directly. + # + # @private + # @see RSpec::Rails::Matchers#have_reported_error + class HaveReportedError < RSpec::Rails::Matchers::BaseMatcher + def initialize(expected_error = nil) + @expected_error = expected_error + @attributes = {} + @error_subscriber = nil + end + + def with(expected_attributes) + @attributes.merge!(expected_attributes) + self + end + + def matches?(block) + @error_subscriber = ErrorSubscriber.new + ::Rails.error.subscribe(@error_subscriber) + + block&.call + + return false if @error_subscriber.events.empty? && !@expected_error.nil? + return false unless error_matches_expectation? + + return attributes_match_if_specified? + ensure + ::Rails.error.unsubscribe(@error_subscriber) if @error_subscriber + end + + def supports_block_expectations? + true + end + + def description + desc = "report an error" + case @expected_error + when Class + desc = "report a #{@expected_error} error" + when Exception + desc = "report a #{@expected_error.class} error" + desc += " with message '#{@expected_error.message}'" unless @expected_error.message.empty? + when Regexp + desc = "report an error with message matching #{@expected_error}" + when Symbol + desc = "report #{@expected_error}" + end + desc += " with #{@attributes}" unless @attributes.empty? + desc + end + + def failure_message + if !@error_subscriber.events.empty? && !@attributes.empty? + event_data = @error_subscriber.events.last[1] + if defined?(ActiveSupport::HashWithIndifferentAccess) + event_data = event_data.with_indifferent_access + end + unmatched = unmatched_attributes(event_data) + unless unmatched.empty? + return "Expected error attributes to match #{@attributes}, but got these mismatches: #{unmatched} and actual values are #{event_data}" + end + elsif @error_subscriber.events.empty? + return 'Expected the block to report an error, but none was reported.' + else + case @expected_error + when Class + return "Expected error to be an instance of #{@expected_error}, but got #{actual_error.class} with message: '#{actual_error.message}'" + when Exception + return "Expected error to be #{@expected_error.class} with message '#{@expected_error.message}', but got #{actual_error.class} with message: '#{actual_error.message}'" + when Regexp + return "Expected error message to match #{@expected_error}, but got: '#{actual_error.message}'" + when Symbol + return "Expected error to be #{@expected_error}, but got: #{actual_error}" + else + return "Expected specific error, but got #{actual_error.class} with message: '#{actual_error.message}'" + end + end + end + + def failure_message_when_negated + error_count = @error_subscriber.events.count + if defined?(ActiveSupport::Inflector) + error_word = 'error'.pluralize(error_count) + verb = error_count == 1 ? 'has' : 'have' + else + error_word = error_count == 1 ? 'error' : 'errors' + verb = error_count == 1 ? 'has' : 'have' + end + "Expected the block not to report any errors, but #{error_count} #{error_word} #{verb} been reported." + end + + private + + def error_matches_expectation? + return true if @expected_error.nil? && @error_subscriber.events.any? + return false if actual_error.nil? + + case @expected_error + when Class + actual_error.is_a?(@expected_error) + when Exception + actual_error.is_a?(@expected_error.class) && + (@expected_error.message.empty? || actual_error.message == @expected_error.message) + when Regexp + actual_error.message&.match(@expected_error) + when Symbol + actual_error == @expected_error + else + true + end + end + + def attributes_match_if_specified? + return true if @attributes.empty? + return false if @error_subscriber.events.empty? + + event_data = @error_subscriber.events.last[1] + attributes_match?(event_data) + end + + def actual_error + @error_subscriber.events.empty? ? nil : @error_subscriber.events.last[0] + end + + def attributes_match?(actual) + @attributes.all? do |key, value| + if defined?(RSpec::Matchers) && value.respond_to?(:matches?) + value.matches?(actual[key]) + else + actual[key] == value + end + end + end + + def unmatched_attributes(actual) + @attributes.reject do |key, value| + if defined?(RSpec::Matchers) && value.respond_to?(:matches?) + value.matches?(actual[key]) + else + actual[key] == value + end + end + end + end + + # @api public + # Passes if the block reports an error to `Rails.error`. + # + # This matcher asserts that ActiveSupport::ErrorReporter has received an error report. + # + # @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 + # + # @param expected_error [Class, Exception, Regexp, Symbol, nil] the expected error to match + def have_reported_error(expected_error = nil) + HaveReportedError.new(expected_error) + end + end + end +end diff --git a/spec/rspec/rails/matchers/have_reported_error_spec.rb b/spec/rspec/rails/matchers/have_reported_error_spec.rb new file mode 100644 index 000000000..700d0206b --- /dev/null +++ b/spec/rspec/rails/matchers/have_reported_error_spec.rb @@ -0,0 +1,233 @@ +require "rspec/rails/matchers/have_reported_error" + +RSpec.describe "have_reported_error matcher" do + class TestError < StandardError; end + class AnotherTestError < StandardError; end + + let(:mock_error_reporter) { double("ErrorReporter") } + let(:subscribers) { [] } + + before do + allow(Rails).to receive(:error).and_return(mock_error_reporter) + + allow(mock_error_reporter).to receive(:subscribe) do |subscriber| + subscribers << subscriber + end + + allow(mock_error_reporter).to receive(:unsubscribe) do |subscriber| + subscribers.delete(subscriber) + end + + allow(mock_error_reporter).to receive(:report) do |error, **attrs| + subscribers.each { |subscriber| subscriber.report(error, **attrs) } + end + end + + after do + subscribers.clear + end + + describe "basic functionality" do + it "passes when an error is reported" do + test_block = proc do + Rails.error.report(StandardError.new("test error")) + end + + expect(test_block).to have_reported_error + end + + it "fails when no error is reported" do + test_block = proc { "no error" } + matcher = have_reported_error + + expect(matcher.matches?(test_block)).to be false + end + + it "passes when negated and no error is reported" do + test_block = proc { "no error" } + + expect(test_block).not_to have_reported_error + end + end + + describe "error class matching" do + it "passes when correct error class is reported" do + test_block = proc do + Rails.error.report(TestError.new("test error")) + end + + expect(test_block).to have_reported_error(TestError) + end + + it "fails when wrong error class is reported" do + test_block = proc do + Rails.error.report(AnotherTestError.new("wrong error")) + end + matcher = have_reported_error(TestError) + + expect(matcher.matches?(test_block)).to be false + end + end + + describe "error instance matching" do + it "passes when error instance matches exactly" do + expected_error = TestError.new("exact message") + test_block = proc do + Rails.error.report(TestError.new("exact message")) + end + + expect(test_block).to have_reported_error(expected_error) + end + + it "passes when error instance has empty expected message" do + expected_error = TestError.new("") + test_block = proc do + Rails.error.report(TestError.new("any message")) + end + + expect(test_block).to have_reported_error(expected_error) + end + + it "fails when error instance has different message" do + expected_error = TestError.new("expected message") + test_block = proc do + Rails.error.report(TestError.new("actual message")) + end + matcher = have_reported_error(expected_error) + + expect(matcher.matches?(test_block)).to be false + end + end + + describe "regex pattern matching" do + it "passes when error message matches pattern" do + test_block = proc do + Rails.error.report(StandardError.new("error with pattern")) + end + + expect(test_block).to have_reported_error(/with pattern/) + end + + it "fails when error message does not match pattern" do + test_block = proc do + Rails.error.report(StandardError.new("error without match")) + end + matcher = have_reported_error(/different pattern/) + + expect(matcher.matches?(test_block)).to be false + end + end + + describe "symbol error matching" do + it "passes when symbol matches" do + test_block = proc do + Rails.error.report(:test_symbol) + end + + expect(test_block).to have_reported_error(:test_symbol) + end + + it "fails when symbol does not match" do + test_block = proc do + Rails.error.report(:actual_symbol) + end + matcher = have_reported_error(:expected_symbol) + + expect(matcher.matches?(test_block)).to be false + end + end + + describe "attribute matching with .with chain" do + it "passes when attributes match exactly" do + test_block = proc do + Rails.error.report(StandardError.new("test"), user_id: 123, context: "test") + end + + expect(test_block).to have_reported_error.with(user_id: 123, context: "test") + end + + it "passes with partial attribute matching" do + test_block = proc do + Rails.error.report(StandardError.new("test"), user_id: 123, context: "test", extra: "data") + end + + expect(test_block).to have_reported_error.with(user_id: 123) + end + + it "passes with hash matching using RSpec matchers" do + test_block = proc do + Rails.error.report(StandardError.new("test"), params: { foo: "bar", baz: "qux" }) + end + + expect(test_block).to have_reported_error.with(params: a_hash_including(foo: "bar")) + end + + it "fails when attributes do not match" do + test_block = proc do + Rails.error.report(StandardError.new("test"), user_id: 123, context: "actual") + end + matcher = have_reported_error.with(user_id: 456, context: "expected") + + expect(matcher.matches?(test_block)).to be false + end + + it "fails when no error is reported but attributes are expected" do + test_block = proc { "no error" } + matcher = have_reported_error.with(user_id: 123) + + expect(matcher.matches?(test_block)).to be false + end + end + + describe "cleanup behavior" do + it "unsubscribes from error reporter on successful completion" do + test_block = proc do + Rails.error.report(StandardError.new("test")) + end + + expect(mock_error_reporter).to receive(:unsubscribe) + + expect(test_block).to have_reported_error + end + + it "unsubscribes from error reporter even when exception is raised" do + test_block = proc do + Rails.error.report(StandardError.new("test")) + raise "unexpected error" + end + + expect(mock_error_reporter).to receive(:unsubscribe) + + expect { + have_reported_error.matches?(test_block) + }.to raise_error("unexpected error") + end + end + + describe "block expectations support" do + it "declares support for block expectations" do + matcher = have_reported_error + expect(matcher).to respond_to(:supports_block_expectations?) + expect(matcher.supports_block_expectations?).to be true + end + end + + describe "integration with actual usage patterns" do + it "works with multiple error reports in a block" do + test_block = proc do + Rails.error.report(StandardError.new("first error")) + Rails.error.report(TestError.new("second error")) + end + + expect(test_block).to have_reported_error(StandardError) + end + + it "works with matcher chaining" do + test_block = proc do + Rails.error.report(TestError.new("test"), user_id: 123) + end + + expect(test_block).to have_reported_error(TestError).and have_reported_error + end + end +end From 5dd2c5ce3ccb96a06b7750f6f52df30fcdfd4b53 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Sat, 7 Jun 2025 21:47:16 +0200 Subject: [PATCH 02/22] Adding a feature --- features/matchers/README.md | 19 ++ .../have_reported_error_matcher.feature | 239 ++++++++++++++++++ 2 files changed, 258 insertions(+) create mode 100644 features/matchers/have_reported_error_matcher.feature diff --git a/features/matchers/README.md b/features/matchers/README.md index 1fc7b5388..ed6fdb329 100644 --- a/features/matchers/README.md +++ b/features/matchers/README.md @@ -24,3 +24,22 @@ expect(response).to render_template(template_name) # and it is not persisted expect(assigns(:widget)).to be_a_new(Widget) ``` + +### error reporting + +```ruby +# passes if Rails.error.report was called with any error +expect { Rails.error.report(StandardError.new) }.to have_reported_error + +# passes if Rails.error.report was called with a specific error class +expect { Rails.error.report(MyError.new) }.to have_reported_error(MyError) + +# passes if Rails.error.report was called with specific error instance and message +expect { Rails.error.report(MyError.new("message")) }.to have_reported_error(MyError.new("message")) + +# passes if Rails.error.report was called with error matching regex pattern +expect { Rails.error.report(StandardError.new("test message")) }.to have_reported_error(/test/) + +# passes if Rails.error.report was called with specific attributes +expect { Rails.error.report(StandardError.new, context: "test") }.to have_reported_error.with(context: "test") +``` diff --git a/features/matchers/have_reported_error_matcher.feature b/features/matchers/have_reported_error_matcher.feature new file mode 100644 index 000000000..f66819dd4 --- /dev/null +++ b/features/matchers/have_reported_error_matcher.feature @@ -0,0 +1,239 @@ +Feature: `have_reported_error` matcher + + The `have_reported_error` matcher is used to check if an error was reported + to Rails error reporting system (`Rails.error`). It can match against error + classes, instances, messages, and attributes. + + The matcher supports several matching strategies: + * Any error reported + * Specific error class + * Specific error instance with message + * Error message patterns using regular expressions + * Error attributes using `.with()` + * Symbol errors + + The matcher is available in all spec types where Rails error reporting is used. + + Background: + Given a file named "app/models/user.rb" with: + """ruby + class User < ApplicationRecord + def self.process_data + Rails.error.report(StandardError.new("Processing failed")) + end + + def self.process_with_context + Rails.error.report(ArgumentError.new("Invalid input"), context: "user_processing", severity: "high") + end + + def self.process_custom_error + Rails.error.report(ValidationError.new("Email is invalid")) + end + end + """ + And a file named "app/errors/validation_error.rb" with: + """ruby + class ValidationError < StandardError + end + """ + + Scenario: Checking for any error being reported + Given a file named "spec/models/user_spec.rb" with: + """ruby + require "rails_helper" + + RSpec.describe User do + it "reports an error during processing" do + expect { + User.process_data + }.to have_reported_error + end + end + """ + When I run `rspec spec/models/user_spec.rb` + Then the examples should all pass + + Scenario: Checking for specific error class + Given a file named "spec/models/user_spec.rb" with: + """ruby + require "rails_helper" + + RSpec.describe User do + it "reports a StandardError" do + expect { + User.process_data + }.to have_reported_error(StandardError) + end + + it "reports an ArgumentError" do + expect { + User.process_with_context + }.to have_reported_error(ArgumentError) + end + end + """ + When I run `rspec spec/models/user_spec.rb` + Then the examples should all pass + + Scenario: Checking for specific error instance with message + Given a file named "spec/models/user_spec.rb" with: + """ruby + require "rails_helper" + + RSpec.describe User do + it "reports error with specific message" do + expect { + User.process_data + }.to have_reported_error(StandardError.new("Processing failed")) + end + + it "reports ArgumentError with specific message" do + expect { + User.process_with_context + }.to have_reported_error(ArgumentError.new("Invalid input")) + end + end + """ + When I run `rspec spec/models/user_spec.rb` + Then the examples should all pass + + Scenario: Checking error message patterns with regular expressions + Given a file named "spec/models/user_spec.rb" with: + """ruby + require "rails_helper" + + RSpec.describe User do + it "reports error with message matching pattern" do + expect { + User.process_data + }.to have_reported_error(/Processing/) + end + + it "reports error with message containing 'failed'" do + expect { + User.process_data + }.to have_reported_error(/failed$/) + end + end + """ + When I run `rspec spec/models/user_spec.rb` + Then the examples should all pass + + Scenario: Checking error attributes using `with` + Given a file named "spec/models/user_spec.rb" with: + """ruby + require "rails_helper" + + RSpec.describe User do + it "reports error with specific context" do + expect { + User.process_with_context + }.to have_reported_error.with(context: "user_processing") + end + + it "reports error with multiple attributes" do + expect { + User.process_with_context + }.to have_reported_error(ArgumentError).with(context: "user_processing", severity: "high") + end + end + """ + When I run `rspec spec/models/user_spec.rb` + Then the examples should all pass + + Scenario: Checking custom error classes + Given a file named "spec/models/user_spec.rb" with: + """ruby + require "rails_helper" + + RSpec.describe User do + it "reports a ValidationError" do + expect { + User.process_custom_error + }.to have_reported_error(ValidationError) + end + + it "reports ValidationError with specific message" do + expect { + User.process_custom_error + }.to have_reported_error(ValidationError.new("Email is invalid")) + end + end + """ + When I run `rspec spec/models/user_spec.rb` + Then the examples should all pass + + Scenario: Using negation - expecting no errors + Given a file named "spec/models/user_spec.rb" with: + """ruby + require "rails_helper" + + RSpec.describe User do + it "does not report any errors for safe operations" do + expect { + # Safe operation that doesn't report errors + "safe code" + }.not_to have_reported_error + end + end + """ + When I run `rspec spec/models/user_spec.rb` + Then the examples should all pass + + 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 + + Scenario: Complex error matching with multiple conditions + Given a file named "spec/models/user_spec.rb" with: + """ruby + require "rails_helper" + + RSpec.describe User do + it "reports error with class, message pattern, and attributes" do + expect { + Rails.error.report( + ArgumentError.new("Invalid user data provided"), + context: "validation", + severity: "critical", + user_id: 123 + ) + }.to have_reported_error(ArgumentError) + .with(context: "validation", severity: "critical") + end + end + """ + When I run `rspec spec/models/user_spec.rb` + Then the examples should all pass \ No newline at end of file From ce1a162ea0ed00b09a971e8fceb4fc6544ad37f4 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Sun, 15 Jun 2025 00:58:24 +0200 Subject: [PATCH 03/22] simplify matcher/readmer.md --- features/matchers/README.md | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/features/matchers/README.md b/features/matchers/README.md index ed6fdb329..df4910f79 100644 --- a/features/matchers/README.md +++ b/features/matchers/README.md @@ -28,18 +28,6 @@ expect(assigns(:widget)).to be_a_new(Widget) ### error reporting ```ruby -# passes if Rails.error.report was called with any error -expect { Rails.error.report(StandardError.new) }.to have_reported_error - -# passes if Rails.error.report was called with a specific error class -expect { Rails.error.report(MyError.new) }.to have_reported_error(MyError) - # passes if Rails.error.report was called with specific error instance and message expect { Rails.error.report(MyError.new("message")) }.to have_reported_error(MyError.new("message")) - -# passes if Rails.error.report was called with error matching regex pattern -expect { Rails.error.report(StandardError.new("test message")) }.to have_reported_error(/test/) - -# passes if Rails.error.report was called with specific attributes -expect { Rails.error.report(StandardError.new, context: "test") }.to have_reported_error.with(context: "test") ``` From 550c25e146017694c5bd91cc6591a94a322f9ffd Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Sun, 15 Jun 2025 01:25:04 +0200 Subject: [PATCH 04/22] Remove mocks out of tests --- .../rails/matchers/have_reported_error.rb | 11 ++-- .../matchers/have_reported_error_spec.rb | 56 ++----------------- 2 files changed, 9 insertions(+), 58 deletions(-) diff --git a/lib/rspec/rails/matchers/have_reported_error.rb b/lib/rspec/rails/matchers/have_reported_error.rb index a1ec27844..2db179903 100644 --- a/lib/rspec/rails/matchers/have_reported_error.rb +++ b/lib/rspec/rails/matchers/have_reported_error.rb @@ -69,11 +69,8 @@ def description def failure_message if !@error_subscriber.events.empty? && !@attributes.empty? - event_data = @error_subscriber.events.last[1] - if defined?(ActiveSupport::HashWithIndifferentAccess) - event_data = event_data.with_indifferent_access - end - unmatched = unmatched_attributes(event_data) + event_data = @error_subscriber.events.last[1].with_indifferent_access + unmatched = unmatched_attributes(event_data["context"]) unless unmatched.empty? return "Expected error attributes to match #{@attributes}, but got these mismatches: #{unmatched} and actual values are #{event_data}" end @@ -132,8 +129,8 @@ def attributes_match_if_specified? return true if @attributes.empty? return false if @error_subscriber.events.empty? - event_data = @error_subscriber.events.last[1] - attributes_match?(event_data) + event_data = @error_subscriber.events.last[1].with_indifferent_access + attributes_match?(event_data["context"]) end def actual_error diff --git a/spec/rspec/rails/matchers/have_reported_error_spec.rb b/spec/rspec/rails/matchers/have_reported_error_spec.rb index 700d0206b..d861acb98 100644 --- a/spec/rspec/rails/matchers/have_reported_error_spec.rb +++ b/spec/rspec/rails/matchers/have_reported_error_spec.rb @@ -4,29 +4,6 @@ class TestError < StandardError; end class AnotherTestError < StandardError; end - let(:mock_error_reporter) { double("ErrorReporter") } - let(:subscribers) { [] } - - before do - allow(Rails).to receive(:error).and_return(mock_error_reporter) - - allow(mock_error_reporter).to receive(:subscribe) do |subscriber| - subscribers << subscriber - end - - allow(mock_error_reporter).to receive(:unsubscribe) do |subscriber| - subscribers.delete(subscriber) - end - - allow(mock_error_reporter).to receive(:report) do |error, **attrs| - subscribers.each { |subscriber| subscriber.report(error, **attrs) } - end - end - - after do - subscribers.clear - end - describe "basic functionality" do it "passes when an error is reported" do test_block = proc do @@ -118,29 +95,10 @@ class AnotherTestError < StandardError; end end end - describe "symbol error matching" do - it "passes when symbol matches" do - test_block = proc do - Rails.error.report(:test_symbol) - end - - expect(test_block).to have_reported_error(:test_symbol) - end - - it "fails when symbol does not match" do - test_block = proc do - Rails.error.report(:actual_symbol) - end - matcher = have_reported_error(:expected_symbol) - - expect(matcher.matches?(test_block)).to be false - end - end - describe "attribute matching with .with chain" do it "passes when attributes match exactly" do test_block = proc do - Rails.error.report(StandardError.new("test"), user_id: 123, context: "test") + Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "test" }) end expect(test_block).to have_reported_error.with(user_id: 123, context: "test") @@ -148,7 +106,7 @@ class AnotherTestError < StandardError; end it "passes with partial attribute matching" do test_block = proc do - Rails.error.report(StandardError.new("test"), user_id: 123, context: "test", extra: "data") + Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "test", extra: "data" }) end expect(test_block).to have_reported_error.with(user_id: 123) @@ -156,7 +114,7 @@ class AnotherTestError < StandardError; end it "passes with hash matching using RSpec matchers" do test_block = proc do - Rails.error.report(StandardError.new("test"), params: { foo: "bar", baz: "qux" }) + Rails.error.report(StandardError.new("test"), context: { params: { foo: "bar", baz: "qux" } }) end expect(test_block).to have_reported_error.with(params: a_hash_including(foo: "bar")) @@ -164,7 +122,7 @@ class AnotherTestError < StandardError; end it "fails when attributes do not match" do test_block = proc do - Rails.error.report(StandardError.new("test"), user_id: 123, context: "actual") + Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "actual" }) end matcher = have_reported_error.with(user_id: 456, context: "expected") @@ -185,8 +143,6 @@ class AnotherTestError < StandardError; end Rails.error.report(StandardError.new("test")) end - expect(mock_error_reporter).to receive(:unsubscribe) - expect(test_block).to have_reported_error end @@ -196,8 +152,6 @@ class AnotherTestError < StandardError; end raise "unexpected error" end - expect(mock_error_reporter).to receive(:unsubscribe) - expect { have_reported_error.matches?(test_block) }.to raise_error("unexpected error") @@ -224,7 +178,7 @@ class AnotherTestError < StandardError; end it "works with matcher chaining" do test_block = proc do - Rails.error.report(TestError.new("test"), user_id: 123) + Rails.error.report(TestError.new("test"), context: { user_id: 123 }) end expect(test_block).to have_reported_error(TestError).and have_reported_error From e074991b6523bd59f24823eca1df903f21021787 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Sun, 15 Jun 2025 01:54:37 +0200 Subject: [PATCH 05/22] test failure messages better --- .../rails/matchers/have_reported_error.rb | 24 ++++----- .../matchers/have_reported_error_spec.rb | 50 +++++++++++++++++++ 2 files changed, 60 insertions(+), 14 deletions(-) diff --git a/lib/rspec/rails/matchers/have_reported_error.rb b/lib/rspec/rails/matchers/have_reported_error.rb index 2db179903..11d816955 100644 --- a/lib/rspec/rails/matchers/have_reported_error.rb +++ b/lib/rspec/rails/matchers/have_reported_error.rb @@ -69,10 +69,10 @@ def description def failure_message if !@error_subscriber.events.empty? && !@attributes.empty? - event_data = @error_subscriber.events.last[1].with_indifferent_access - unmatched = unmatched_attributes(event_data["context"]) + event_context = @error_subscriber.events.last[1].with_indifferent_access["context"] + unmatched = unmatched_attributes(event_context) unless unmatched.empty? - return "Expected error attributes to match #{@attributes}, but got these mismatches: #{unmatched} and actual values are #{event_data}" + return "Expected error attributes to match #{@attributes}, but got these mismatches: #{unmatched} and actual values are #{event_context}" end elsif @error_subscriber.events.empty? return 'Expected the block to report an error, but none was reported.' @@ -94,13 +94,9 @@ def failure_message def failure_message_when_negated error_count = @error_subscriber.events.count - if defined?(ActiveSupport::Inflector) - error_word = 'error'.pluralize(error_count) - verb = error_count == 1 ? 'has' : 'have' - else - error_word = error_count == 1 ? 'error' : 'errors' - verb = error_count == 1 ? 'has' : 'have' - end + error_word = 'error'.pluralize(error_count) + verb = error_count == 1 ? 'has' : 'have' + "Expected the block not to report any errors, but #{error_count} #{error_word} #{verb} been reported." end @@ -129,8 +125,8 @@ def attributes_match_if_specified? return true if @attributes.empty? return false if @error_subscriber.events.empty? - event_data = @error_subscriber.events.last[1].with_indifferent_access - attributes_match?(event_data["context"]) + event_context = @error_subscriber.events.last[1].with_indifferent_access["context"] + attributes_match?(event_context) end def actual_error @@ -139,7 +135,7 @@ def actual_error def attributes_match?(actual) @attributes.all? do |key, value| - if defined?(RSpec::Matchers) && value.respond_to?(:matches?) + if value.respond_to?(:matches?) value.matches?(actual[key]) else actual[key] == value @@ -149,7 +145,7 @@ def attributes_match?(actual) def unmatched_attributes(actual) @attributes.reject do |key, value| - if defined?(RSpec::Matchers) && value.respond_to?(:matches?) + if value.respond_to?(:matches?) value.matches?(actual[key]) else actual[key] == value diff --git a/spec/rspec/rails/matchers/have_reported_error_spec.rb b/spec/rspec/rails/matchers/have_reported_error_spec.rb index d861acb98..976d91e17 100644 --- a/spec/rspec/rails/matchers/have_reported_error_spec.rb +++ b/spec/rspec/rails/matchers/have_reported_error_spec.rb @@ -95,6 +95,56 @@ class AnotherTestError < StandardError; end end end + describe "failure messages for attribute mismatches" do + it "provides detailed failure message when attributes don't match" do + test_block = proc do + Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "actual" }) + end + matcher = have_reported_error.with(user_id: 456, context: "expected") + + matcher.matches?(test_block) + + expect(matcher.failure_message).to include("Expected error attributes to match") + expect(matcher.failure_message).to include("user_id: 456") + expect(matcher.failure_message).to include("context: \"expected\"") + end + + it "identifies partial attribute mismatches correctly" do + test_block = proc do + Rails.error.report(StandardError.new("test"), context: { user_id: 123, status: "active", role: "admin" }) + end + matcher = have_reported_error.with(user_id: 456, status: "active") # user_id wrong, status correct + + matcher.matches?(test_block) + + failure_msg = matcher.failure_message + expect(failure_msg).to include("got these mismatches: {user_id: 456}") + end + + it "handles RSpec matcher mismatches in failure messages" do + test_block = proc do + Rails.error.report(StandardError.new("test"), context: { params: { foo: "different" } }) + end + matcher = have_reported_error.with(params: a_hash_including(foo: "bar")) + + matcher.matches?(test_block) + + expect(matcher.failure_message).to include("Expected error attributes to match") + end + + it "shows actual context values when attributes don't match" do + test_block = proc do + Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "actual" }) + end + matcher = have_reported_error.with(user_id: 456) + + matcher.matches?(test_block) + + failure_msg = matcher.failure_message + expect(failure_msg).to include("actual values are {\"user_id\" => 123, \"context\" => \"actual\"}") + end + end + describe "attribute matching with .with chain" do it "passes when attributes match exactly" do test_block = proc do From 1fe7e910518f54f337a710028d8c8092bcb7aa19 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Sat, 21 Jun 2025 01:00:25 +0200 Subject: [PATCH 06/22] rewrite specs --- .../matchers/have_reported_error_spec.rb | 216 ++++++------------ 1 file changed, 70 insertions(+), 146 deletions(-) diff --git a/spec/rspec/rails/matchers/have_reported_error_spec.rb b/spec/rspec/rails/matchers/have_reported_error_spec.rb index 976d91e17..5f4420ba4 100644 --- a/spec/rspec/rails/matchers/have_reported_error_spec.rb +++ b/spec/rspec/rails/matchers/have_reported_error_spec.rb @@ -6,232 +6,156 @@ class AnotherTestError < StandardError; end describe "basic functionality" do it "passes when an error is reported" do - test_block = proc do - Rails.error.report(StandardError.new("test error")) - end - - expect(test_block).to have_reported_error + expect {Rails.error.report(StandardError.new("test error"))}.to have_reported_error end it "fails when no error is reported" do - test_block = proc { "no error" } - matcher = have_reported_error - - expect(matcher.matches?(test_block)).to be false + expect { + expect { "no error" }.to have_reported_error + }.to fail_with(/Expected the block to report an error, but none was reported./) end it "passes when negated and no error is reported" do - test_block = proc { "no error" } - - expect(test_block).not_to have_reported_error + expect { "no error" }.not_to have_reported_error end end describe "error class matching" do it "passes when correct error class is reported" do - test_block = proc do - Rails.error.report(TestError.new("test error")) - end - - expect(test_block).to have_reported_error(TestError) + expect { Rails.error.report(TestError.new("test error")) }.to have_reported_error(TestError) end it "fails when wrong error class is reported" do - test_block = proc do - Rails.error.report(AnotherTestError.new("wrong error")) - end - matcher = have_reported_error(TestError) - - expect(matcher.matches?(test_block)).to be false + expect { + expect { + Rails.error.report(AnotherTestError.new("wrong error")) + }.to have_reported_error(TestError) + }.to fail_with(/Expected error to be an instance of TestError, but got AnotherTestError/) end end describe "error instance matching" do it "passes when error instance matches exactly" do - expected_error = TestError.new("exact message") - test_block = proc do + expect { Rails.error.report(TestError.new("exact message")) - end - - expect(test_block).to have_reported_error(expected_error) + }.to have_reported_error(TestError.new("exact message")) end it "passes when error instance has empty expected message" do - expected_error = TestError.new("") - test_block = proc do + expect { Rails.error.report(TestError.new("any message")) - end - - expect(test_block).to have_reported_error(expected_error) + }.to have_reported_error(TestError.new("")) end it "fails when error instance has different message" do - expected_error = TestError.new("expected message") - test_block = proc do - Rails.error.report(TestError.new("actual message")) - end - matcher = have_reported_error(expected_error) - - expect(matcher.matches?(test_block)).to be false + expect { + expect { + Rails.error.report(TestError.new("actual message")) + }.to have_reported_error(TestError.new("expected message")) + }.to fail_with(/Expected error to be TestError with message 'expected message', but got TestError with message: 'actual message'/) end end describe "regex pattern matching" do it "passes when error message matches pattern" do - test_block = proc do + expect { Rails.error.report(StandardError.new("error with pattern")) - end - - expect(test_block).to have_reported_error(/with pattern/) + }.to have_reported_error(/with pattern/) end it "fails when error message does not match pattern" do - test_block = proc do - Rails.error.report(StandardError.new("error without match")) - end - matcher = have_reported_error(/different pattern/) - - expect(matcher.matches?(test_block)).to be false + expect { + expect { + Rails.error.report(StandardError.new("error without match")) + }.to have_reported_error(/different pattern/) + }.to fail_with(/Expected error message to match/) end end describe "failure messages for attribute mismatches" do it "provides detailed failure message when attributes don't match" do - test_block = proc do - Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "actual" }) - end - matcher = have_reported_error.with(user_id: 456, context: "expected") - - matcher.matches?(test_block) - - expect(matcher.failure_message).to include("Expected error attributes to match") - expect(matcher.failure_message).to include("user_id: 456") - expect(matcher.failure_message).to include("context: \"expected\"") + expect { + expect { + Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "actual" }) + }.to have_reported_error.with(user_id: 456, context: "expected") + }.to fail_with(/Expected error attributes to match {user_id: 456, context: "expected"}, but got these mismatches: {user_id: 456, context: "expected"} and actual values are {"user_id" => 123, "context" => "actual"}/) end it "identifies partial attribute mismatches correctly" do - test_block = proc do - Rails.error.report(StandardError.new("test"), context: { user_id: 123, status: "active", role: "admin" }) - end - matcher = have_reported_error.with(user_id: 456, status: "active") # user_id wrong, status correct - - matcher.matches?(test_block) - - failure_msg = matcher.failure_message - expect(failure_msg).to include("got these mismatches: {user_id: 456}") + expect { + expect { + Rails.error.report(StandardError.new("test"), context: { user_id: 123, status: "active", role: "admin" }) + }.to have_reported_error.with(user_id: 456, status: "active") # user_id wrong, status correct + }.to fail_with(/got these mismatches: {user_id: 456}/) end it "handles RSpec matcher mismatches in failure messages" do - test_block = proc do - Rails.error.report(StandardError.new("test"), context: { params: { foo: "different" } }) - end - matcher = have_reported_error.with(params: a_hash_including(foo: "bar")) - - matcher.matches?(test_block) - - expect(matcher.failure_message).to include("Expected error attributes to match") + expect { + expect { + Rails.error.report(StandardError.new("test"), context: { params: { foo: "different" } }) + }.to have_reported_error.with(params: a_hash_including(foo: "bar")) + }.to fail_with(/Expected error attributes to match/) end it "shows actual context values when attributes don't match" do - test_block = proc do - Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "actual" }) - end - matcher = have_reported_error.with(user_id: 456) - - matcher.matches?(test_block) - - failure_msg = matcher.failure_message - expect(failure_msg).to include("actual values are {\"user_id\" => 123, \"context\" => \"actual\"}") + expect { + expect { + Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "actual" }) + }.to have_reported_error.with(user_id: 456) + }.to fail_with(/actual values are {"user_id" => 123, "context" => "actual"}/) end end describe "attribute matching with .with chain" do it "passes when attributes match exactly" do - test_block = proc do + expect { Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "test" }) - end - - expect(test_block).to have_reported_error.with(user_id: 123, context: "test") + }.to have_reported_error.with(user_id: 123, context: "test") end it "passes with partial attribute matching" do - test_block = proc do - Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "test", extra: "data" }) - end - - expect(test_block).to have_reported_error.with(user_id: 123) + expect { + Rails.error.report( + StandardError.new("test"), context: { user_id: 123, context: "test", extra: "data" } + ) + }.to have_reported_error.with(user_id: 123) end it "passes with hash matching using RSpec matchers" do - test_block = proc do - Rails.error.report(StandardError.new("test"), context: { params: { foo: "bar", baz: "qux" } }) - end - - expect(test_block).to have_reported_error.with(params: a_hash_including(foo: "bar")) + expect { + Rails.error.report( + StandardError.new("test"), context: { params: { foo: "bar", baz: "qux" } } + ) + }.to have_reported_error.with(params: a_hash_including(foo: "bar")) end it "fails when attributes do not match" do - test_block = proc do - Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "actual" }) - end - matcher = have_reported_error.with(user_id: 456, context: "expected") - - expect(matcher.matches?(test_block)).to be false + expect { + expect { + Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "actual" }) + }.to have_reported_error.with(user_id: 456, context: "expected") + }.to fail_with(/Expected error attributes to match {user_id: 456, context: "expected"}, but got these mismatches: {user_id: 456, context: "expected"} and actual values are {"user_id" => 123, "context" => "actual"}/) end it "fails when no error is reported but attributes are expected" do - test_block = proc { "no error" } - matcher = have_reported_error.with(user_id: 123) - - expect(matcher.matches?(test_block)).to be false - end - end - - describe "cleanup behavior" do - it "unsubscribes from error reporter on successful completion" do - test_block = proc do - Rails.error.report(StandardError.new("test")) - end - - expect(test_block).to have_reported_error - end - - it "unsubscribes from error reporter even when exception is raised" do - test_block = proc do - Rails.error.report(StandardError.new("test")) - raise "unexpected error" - end - expect { - have_reported_error.matches?(test_block) - }.to raise_error("unexpected error") - end - end - - describe "block expectations support" do - it "declares support for block expectations" do - matcher = have_reported_error - expect(matcher).to respond_to(:supports_block_expectations?) - expect(matcher.supports_block_expectations?).to be true + expect { "no error" }.to have_reported_error.with(user_id: 123) + }.to fail_with(/Expected the block to report an error, but none was reported./) end end describe "integration with actual usage patterns" do it "works with multiple error reports in a block" do - test_block = proc do + expect { Rails.error.report(StandardError.new("first error")) Rails.error.report(TestError.new("second error")) - end - - expect(test_block).to have_reported_error(StandardError) + }.to have_reported_error(StandardError) end it "works with matcher chaining" do - test_block = proc do + expect { Rails.error.report(TestError.new("test"), context: { user_id: 123 }) - end - - expect(test_block).to have_reported_error(TestError).and have_reported_error + }.to have_reported_error(TestError).and have_reported_error end end end From 6b41d2b27edc8c096f04519043b8d32f694a0e15 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Sun, 22 Jun 2025 22:36:20 +0200 Subject: [PATCH 07/22] should require a block matcher --- lib/rspec/rails/matchers/have_reported_error.rb | 6 ++++++ spec/rspec/rails/matchers/have_reported_error_spec.rb | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/lib/rspec/rails/matchers/have_reported_error.rb b/lib/rspec/rails/matchers/have_reported_error.rb index 11d816955..f5b407033 100644 --- a/lib/rspec/rails/matchers/have_reported_error.rb +++ b/lib/rspec/rails/matchers/have_reported_error.rb @@ -33,6 +33,10 @@ def with(expected_attributes) end def matches?(block) + if block.nil? + raise ArgumentError, "block is required for have_reported_error matcher" + end + @error_subscriber = ErrorSubscriber.new ::Rails.error.subscribe(@error_subscriber) @@ -181,6 +185,8 @@ def unmatched_attributes(actual) def have_reported_error(expected_error = nil) HaveReportedError.new(expected_error) end + + alias_method :reports_error, :have_reported_error end end end diff --git a/spec/rspec/rails/matchers/have_reported_error_spec.rb b/spec/rspec/rails/matchers/have_reported_error_spec.rb index 5f4420ba4..e3c9a5d99 100644 --- a/spec/rspec/rails/matchers/have_reported_error_spec.rb +++ b/spec/rspec/rails/matchers/have_reported_error_spec.rb @@ -4,6 +4,16 @@ class TestError < StandardError; end class AnotherTestError < StandardError; end + it "has an reports_error alias" do + expect {Rails.error.report(StandardError.new("test error"))}.to reports_error + end + + it "warns that passing value expectation doesn't work" do + expect { + expect(Rails.error.report(StandardError.new("test error"))).to have_reported_error + }.to raise_error(ArgumentError, "block is required for have_reported_error matcher") + end + describe "basic functionality" do it "passes when an error is reported" do expect {Rails.error.report(StandardError.new("test error"))}.to have_reported_error From 3859b04ae4a2d0a41a3c63669dcf98a8e52a55b0 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Sun, 22 Jun 2025 22:42:53 +0200 Subject: [PATCH 08/22] Create ErrorEvent struct to store errors --- lib/rspec/rails/matchers/have_reported_error.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/rspec/rails/matchers/have_reported_error.rb b/lib/rspec/rails/matchers/have_reported_error.rb index f5b407033..692201553 100644 --- a/lib/rspec/rails/matchers/have_reported_error.rb +++ b/lib/rspec/rails/matchers/have_reported_error.rb @@ -7,12 +7,14 @@ module Matchers class ErrorSubscriber attr_reader :events + ErrorEvent = Struct.new(:error, :attributes) + def initialize @events = [] end def report(error, **attrs) - @events << [error, attrs] + @events << ErrorEvent.new(error, attrs.with_indifferent_access) end end @@ -47,7 +49,7 @@ def matches?(block) return attributes_match_if_specified? ensure - ::Rails.error.unsubscribe(@error_subscriber) if @error_subscriber + ::Rails.error.unsubscribe(@error_subscriber) end def supports_block_expectations? @@ -73,7 +75,7 @@ def description def failure_message if !@error_subscriber.events.empty? && !@attributes.empty? - event_context = @error_subscriber.events.last[1].with_indifferent_access["context"] + event_context = @error_subscriber.events.last.attributes[:context] unmatched = unmatched_attributes(event_context) unless unmatched.empty? return "Expected error attributes to match #{@attributes}, but got these mismatches: #{unmatched} and actual values are #{event_context}" @@ -129,12 +131,12 @@ def attributes_match_if_specified? return true if @attributes.empty? return false if @error_subscriber.events.empty? - event_context = @error_subscriber.events.last[1].with_indifferent_access["context"] + event_context = @error_subscriber.events.last.attributes[:context] attributes_match?(event_context) end def actual_error - @error_subscriber.events.empty? ? nil : @error_subscriber.events.last[0] + @error_subscriber.events.empty? ? nil : @error_subscriber.events.last.error end def attributes_match?(actual) From 83526688605f99ab9a671d0942e960987f14a5d0 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Sun, 22 Jun 2025 22:45:01 +0200 Subject: [PATCH 09/22] simplify matcher --- lib/rspec/rails/matchers/have_reported_error.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/rspec/rails/matchers/have_reported_error.rb b/lib/rspec/rails/matchers/have_reported_error.rb index 692201553..356a77968 100644 --- a/lib/rspec/rails/matchers/have_reported_error.rb +++ b/lib/rspec/rails/matchers/have_reported_error.rb @@ -122,8 +122,6 @@ def error_matches_expectation? actual_error.message&.match(@expected_error) when Symbol actual_error == @expected_error - else - true end end From 030c27ac3525f368b81fa6f8cb4ed1683d7e5432 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Sun, 22 Jun 2025 22:51:47 +0200 Subject: [PATCH 10/22] disable matcher chaining --- lib/rspec/rails/matchers/have_reported_error.rb | 4 ++++ spec/rspec/rails/matchers/have_reported_error_spec.rb | 6 ++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/rspec/rails/matchers/have_reported_error.rb b/lib/rspec/rails/matchers/have_reported_error.rb index 356a77968..180391f5b 100644 --- a/lib/rspec/rails/matchers/have_reported_error.rb +++ b/lib/rspec/rails/matchers/have_reported_error.rb @@ -34,6 +34,10 @@ def with(expected_attributes) self end + def and(_) + raise ArgumentError, "Chaining is not supported" + end + def matches?(block) if block.nil? raise ArgumentError, "block is required for have_reported_error matcher" diff --git a/spec/rspec/rails/matchers/have_reported_error_spec.rb b/spec/rspec/rails/matchers/have_reported_error_spec.rb index e3c9a5d99..b5fbc8b5d 100644 --- a/spec/rspec/rails/matchers/have_reported_error_spec.rb +++ b/spec/rspec/rails/matchers/have_reported_error_spec.rb @@ -164,8 +164,10 @@ class AnotherTestError < StandardError; end it "works with matcher chaining" do expect { - Rails.error.report(TestError.new("test"), context: { user_id: 123 }) - }.to have_reported_error(TestError).and have_reported_error + expect { + Rails.error.report(TestError.new("test")) + }.to have_reported_error(TestError).and have_reported_error + }.to raise_error(ArgumentError, "Chaining is not supported") end end end From 2b9bd021b22b6a5ee69fbefcb62c0db54a922943 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Sun, 22 Jun 2025 22:53:34 +0200 Subject: [PATCH 11/22] improved argument error message in case block is not passed to matcher --- lib/rspec/rails/matchers/have_reported_error.rb | 2 +- spec/rspec/rails/matchers/have_reported_error_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rspec/rails/matchers/have_reported_error.rb b/lib/rspec/rails/matchers/have_reported_error.rb index 180391f5b..ba3f9abd9 100644 --- a/lib/rspec/rails/matchers/have_reported_error.rb +++ b/lib/rspec/rails/matchers/have_reported_error.rb @@ -40,7 +40,7 @@ def and(_) def matches?(block) if block.nil? - raise ArgumentError, "block is required for have_reported_error matcher" + raise ArgumentError, "this matcher doesn’t work with value expectations" end @error_subscriber = ErrorSubscriber.new diff --git a/spec/rspec/rails/matchers/have_reported_error_spec.rb b/spec/rspec/rails/matchers/have_reported_error_spec.rb index b5fbc8b5d..b670b556f 100644 --- a/spec/rspec/rails/matchers/have_reported_error_spec.rb +++ b/spec/rspec/rails/matchers/have_reported_error_spec.rb @@ -11,7 +11,7 @@ class AnotherTestError < StandardError; end it "warns that passing value expectation doesn't work" do expect { expect(Rails.error.report(StandardError.new("test error"))).to have_reported_error - }.to raise_error(ArgumentError, "block is required for have_reported_error matcher") + }.to raise_error(ArgumentError, "this matcher doesn’t work with value expectations") end describe "basic functionality" do From 6135d5d0bdfd80d6e3051428639facbab6bd831f Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Sun, 22 Jun 2025 22:56:36 +0200 Subject: [PATCH 12/22] further clean-up --- lib/rspec/rails/matchers/have_reported_error.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/rspec/rails/matchers/have_reported_error.rb b/lib/rspec/rails/matchers/have_reported_error.rb index ba3f9abd9..dd9f83c4e 100644 --- a/lib/rspec/rails/matchers/have_reported_error.rb +++ b/lib/rspec/rails/matchers/have_reported_error.rb @@ -114,7 +114,6 @@ def failure_message_when_negated def error_matches_expectation? return true if @expected_error.nil? && @error_subscriber.events.any? - return false if actual_error.nil? case @expected_error when Class From 833566468a8f9d71b181bd8a03aba249862580b4 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Wed, 25 Jun 2025 23:21:12 +0200 Subject: [PATCH 13/22] Apply suggestions from code review Commit grammar improvements Co-authored-by: Jon Rowe --- features/matchers/README.md | 2 +- .../have_reported_error_matcher.feature | 24 ++++--------- .../rails/matchers/have_reported_error.rb | 4 +-- .../matchers/have_reported_error_spec.rb | 36 +++++++++---------- 4 files changed, 28 insertions(+), 38 deletions(-) diff --git a/features/matchers/README.md b/features/matchers/README.md index df4910f79..570687edc 100644 --- a/features/matchers/README.md +++ b/features/matchers/README.md @@ -28,6 +28,6 @@ expect(assigns(:widget)).to be_a_new(Widget) ### error reporting ```ruby -# passes if Rails.error.report was called with specific error instance and message +# passes when `Rails.error.report` is called with specific error instance and message expect { Rails.error.report(MyError.new("message")) }.to have_reported_error(MyError.new("message")) ``` diff --git a/features/matchers/have_reported_error_matcher.feature b/features/matchers/have_reported_error_matcher.feature index f66819dd4..ad639c52d 100644 --- a/features/matchers/have_reported_error_matcher.feature +++ b/features/matchers/have_reported_error_matcher.feature @@ -6,7 +6,7 @@ Feature: `have_reported_error` matcher The matcher supports several matching strategies: * Any error reported - * Specific error class + * A specific error class * Specific error instance with message * Error message patterns using regular expressions * Error attributes using `.with()` @@ -18,6 +18,7 @@ Feature: `have_reported_error` matcher Given a file named "app/models/user.rb" with: """ruby class User < ApplicationRecord + class ValidationError < StandardError; end def self.process_data Rails.error.report(StandardError.new("Processing failed")) end @@ -31,11 +32,6 @@ Feature: `have_reported_error` matcher end end """ - And a file named "app/errors/validation_error.rb" with: - """ruby - class ValidationError < StandardError - end - """ Scenario: Checking for any error being reported Given a file named "spec/models/user_spec.rb" with: @@ -43,7 +39,7 @@ Feature: `have_reported_error` matcher require "rails_helper" RSpec.describe User do - it "reports an error during processing" do + it "reports errors" do expect { User.process_data }.to have_reported_error @@ -53,7 +49,7 @@ Feature: `have_reported_error` matcher When I run `rspec spec/models/user_spec.rb` Then the examples should all pass - Scenario: Checking for specific error class + Scenario: Checking for a specific error class Given a file named "spec/models/user_spec.rb" with: """ruby require "rails_helper" @@ -97,29 +93,23 @@ Feature: `have_reported_error` matcher When I run `rspec spec/models/user_spec.rb` Then the examples should all pass - Scenario: Checking error message patterns with regular expressions + Scenario: Checking error messages using regular expressions Given a file named "spec/models/user_spec.rb" with: """ruby require "rails_helper" RSpec.describe User do - it "reports error with message matching pattern" do + it "reports errors with a message matching a pattern" do expect { User.process_data }.to have_reported_error(/Processing/) end - - it "reports error with message containing 'failed'" do - expect { - User.process_data - }.to have_reported_error(/failed$/) - end end """ When I run `rspec spec/models/user_spec.rb` Then the examples should all pass - Scenario: Checking error attributes using `with` + Scenario: Constraining error matches to their attributes using `with` Given a file named "spec/models/user_spec.rb" with: """ruby require "rails_helper" diff --git a/lib/rspec/rails/matchers/have_reported_error.rb b/lib/rspec/rails/matchers/have_reported_error.rb index dd9f83c4e..b236c9700 100644 --- a/lib/rspec/rails/matchers/have_reported_error.rb +++ b/lib/rspec/rails/matchers/have_reported_error.rb @@ -3,7 +3,7 @@ module RSpec module Rails module Matchers - # @private + # @api private class ErrorSubscriber attr_reader :events @@ -20,7 +20,7 @@ def report(error, **attrs) # Matcher class for `have_reported_error`. Should not be instantiated directly. # - # @private + # @api private # @see RSpec::Rails::Matchers#have_reported_error class HaveReportedError < RSpec::Rails::Matchers::BaseMatcher def initialize(expected_error = nil) diff --git a/spec/rspec/rails/matchers/have_reported_error_spec.rb b/spec/rspec/rails/matchers/have_reported_error_spec.rb index b670b556f..a4cab1a79 100644 --- a/spec/rspec/rails/matchers/have_reported_error_spec.rb +++ b/spec/rspec/rails/matchers/have_reported_error_spec.rb @@ -4,38 +4,38 @@ class TestError < StandardError; end class AnotherTestError < StandardError; end - it "has an reports_error alias" do + it "is aliased as reports_error" do expect {Rails.error.report(StandardError.new("test error"))}.to reports_error end - it "warns that passing value expectation doesn't work" do + it "warns when used as a value expectation" do expect { expect(Rails.error.report(StandardError.new("test error"))).to have_reported_error }.to raise_error(ArgumentError, "this matcher doesn’t work with value expectations") end - describe "basic functionality" do + context "without constraint" do it "passes when an error is reported" do expect {Rails.error.report(StandardError.new("test error"))}.to have_reported_error end - it "fails when no error is reported" do + it "fails when no errors are reported" do expect { expect { "no error" }.to have_reported_error }.to fail_with(/Expected the block to report an error, but none was reported./) end - it "passes when negated and no error is reported" do + it "passes when negated and no errors are reported" do expect { "no error" }.not_to have_reported_error end end - describe "error class matching" do - it "passes when correct error class is reported" do + context "constrained to a specific error class" do + it "passes when an error with the correct class is reported" do expect { Rails.error.report(TestError.new("test error")) }.to have_reported_error(TestError) end - it "fails when wrong error class is reported" do + it "fails when an error with the wrong class is reported" do expect { expect { Rails.error.report(AnotherTestError.new("wrong error")) @@ -44,20 +44,20 @@ class AnotherTestError < StandardError; end end end - describe "error instance matching" do - it "passes when error instance matches exactly" do + context "constrained to a matching error (class and message)" do + it "passes with an error that matches exactly" do expect { Rails.error.report(TestError.new("exact message")) }.to have_reported_error(TestError.new("exact message")) end - it "passes when error instance has empty expected message" do + it "passes any error of the same class if the expected message is empty" do expect { Rails.error.report(TestError.new("any message")) }.to have_reported_error(TestError.new("")) end - it "fails when error instance has different message" do + it "fails when the error has different message to the expected" do expect { expect { Rails.error.report(TestError.new("actual message")) @@ -66,14 +66,14 @@ class AnotherTestError < StandardError; end end end - describe "regex pattern matching" do - it "passes when error message matches pattern" do + context "constrained by regex pattern matching" do + it "passes when an error message matches the pattern" do expect { Rails.error.report(StandardError.new("error with pattern")) }.to have_reported_error(/with pattern/) end - it "fails when error message does not match pattern" do + it "fails when no error messages match the pattern" do expect { expect { Rails.error.report(StandardError.new("error without match")) @@ -82,8 +82,8 @@ class AnotherTestError < StandardError; end end end - describe "failure messages for attribute mismatches" do - it "provides detailed failure message when attributes don't match" do + describe "#failure_message" do + it "provides details about mismatched attributes" do expect { expect { Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "actual" }) @@ -116,7 +116,7 @@ class AnotherTestError < StandardError; end end end - describe "attribute matching with .with chain" do + describe "#with" do it "passes when attributes match exactly" do expect { Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "test" }) From a14f5c7b1e33adf7556e9fa6b35eb6281fc1f290 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Fri, 27 Jun 2025 13:42:50 +0200 Subject: [PATCH 14/22] clean-up features --- .../have_reported_error_matcher.feature | 58 ------------------- 1 file changed, 58 deletions(-) diff --git a/features/matchers/have_reported_error_matcher.feature b/features/matchers/have_reported_error_matcher.feature index ad639c52d..399f0ebff 100644 --- a/features/matchers/have_reported_error_matcher.feature +++ b/features/matchers/have_reported_error_matcher.feature @@ -169,61 +169,3 @@ Feature: `have_reported_error` matcher """ When I run `rspec spec/models/user_spec.rb` Then the examples should all pass - - 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 - - Scenario: Complex error matching with multiple conditions - Given a file named "spec/models/user_spec.rb" with: - """ruby - require "rails_helper" - - RSpec.describe User do - it "reports error with class, message pattern, and attributes" do - expect { - Rails.error.report( - ArgumentError.new("Invalid user data provided"), - context: "validation", - severity: "critical", - user_id: 123 - ) - }.to have_reported_error(ArgumentError) - .with(context: "validation", severity: "critical") - end - end - """ - When I run `rspec spec/models/user_spec.rb` - Then the examples should all pass \ No newline at end of file From af58d83105f534d139f709ba11a23d54268f1b3b Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Tue, 1 Jul 2025 23:59:52 +0200 Subject: [PATCH 15/22] remove a require from test --- spec/rspec/rails/matchers/have_reported_error_spec.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/rspec/rails/matchers/have_reported_error_spec.rb b/spec/rspec/rails/matchers/have_reported_error_spec.rb index a4cab1a79..d29674023 100644 --- a/spec/rspec/rails/matchers/have_reported_error_spec.rb +++ b/spec/rspec/rails/matchers/have_reported_error_spec.rb @@ -1,5 +1,3 @@ -require "rspec/rails/matchers/have_reported_error" - RSpec.describe "have_reported_error matcher" do class TestError < StandardError; end class AnotherTestError < StandardError; end From 7259a259e1f1cda06f136db3233d885cacbd16c9 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Wed, 2 Jul 2025 00:08:04 +0200 Subject: [PATCH 16/22] rename .with to .with_context --- .../rails/matchers/have_reported_error.rb | 6 +++--- .../matchers/have_reported_error_spec.rb | 20 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/rspec/rails/matchers/have_reported_error.rb b/lib/rspec/rails/matchers/have_reported_error.rb index b236c9700..1022db620 100644 --- a/lib/rspec/rails/matchers/have_reported_error.rb +++ b/lib/rspec/rails/matchers/have_reported_error.rb @@ -29,7 +29,7 @@ def initialize(expected_error = nil) @error_subscriber = nil end - def with(expected_attributes) + def with_context(expected_attributes) @attributes.merge!(expected_attributes) self end @@ -46,7 +46,7 @@ def matches?(block) @error_subscriber = ErrorSubscriber.new ::Rails.error.subscribe(@error_subscriber) - block&.call + block.call return false if @error_subscriber.events.empty? && !@expected_error.nil? return false unless error_matches_expectation? @@ -176,7 +176,7 @@ def unmatched_attributes(actual) # 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") + # expect { Rails.error.report(StandardError.new, context: "test") }.to have_reported_error.with_context(context: "test") # # @example Checking error message patterns # expect { Rails.error.report(StandardError.new("test message")) }.to have_reported_error(/test/) diff --git a/spec/rspec/rails/matchers/have_reported_error_spec.rb b/spec/rspec/rails/matchers/have_reported_error_spec.rb index d29674023..5b3dc67db 100644 --- a/spec/rspec/rails/matchers/have_reported_error_spec.rb +++ b/spec/rspec/rails/matchers/have_reported_error_spec.rb @@ -85,7 +85,7 @@ class AnotherTestError < StandardError; end expect { expect { Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "actual" }) - }.to have_reported_error.with(user_id: 456, context: "expected") + }.to have_reported_error.with_context(user_id: 456, context: "expected") }.to fail_with(/Expected error attributes to match {user_id: 456, context: "expected"}, but got these mismatches: {user_id: 456, context: "expected"} and actual values are {"user_id" => 123, "context" => "actual"}/) end @@ -93,7 +93,7 @@ class AnotherTestError < StandardError; end expect { expect { Rails.error.report(StandardError.new("test"), context: { user_id: 123, status: "active", role: "admin" }) - }.to have_reported_error.with(user_id: 456, status: "active") # user_id wrong, status correct + }.to have_reported_error.with_context(user_id: 456, status: "active") # user_id wrong, status correct }.to fail_with(/got these mismatches: {user_id: 456}/) end @@ -101,7 +101,7 @@ class AnotherTestError < StandardError; end expect { expect { Rails.error.report(StandardError.new("test"), context: { params: { foo: "different" } }) - }.to have_reported_error.with(params: a_hash_including(foo: "bar")) + }.to have_reported_error.with_context(params: a_hash_including(foo: "bar")) }.to fail_with(/Expected error attributes to match/) end @@ -109,16 +109,16 @@ class AnotherTestError < StandardError; end expect { expect { Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "actual" }) - }.to have_reported_error.with(user_id: 456) + }.to have_reported_error.with_context(user_id: 456) }.to fail_with(/actual values are {"user_id" => 123, "context" => "actual"}/) end end - describe "#with" do + describe "#with_context" do it "passes when attributes match exactly" do expect { Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "test" }) - }.to have_reported_error.with(user_id: 123, context: "test") + }.to have_reported_error.with_context(user_id: 123, context: "test") end it "passes with partial attribute matching" do @@ -126,7 +126,7 @@ class AnotherTestError < StandardError; end Rails.error.report( StandardError.new("test"), context: { user_id: 123, context: "test", extra: "data" } ) - }.to have_reported_error.with(user_id: 123) + }.to have_reported_error.with_context(user_id: 123) end it "passes with hash matching using RSpec matchers" do @@ -134,20 +134,20 @@ class AnotherTestError < StandardError; end Rails.error.report( StandardError.new("test"), context: { params: { foo: "bar", baz: "qux" } } ) - }.to have_reported_error.with(params: a_hash_including(foo: "bar")) + }.to have_reported_error.with_context(params: a_hash_including(foo: "bar")) end it "fails when attributes do not match" do expect { expect { Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "actual" }) - }.to have_reported_error.with(user_id: 456, context: "expected") + }.to have_reported_error.with_context(user_id: 456, context: "expected") }.to fail_with(/Expected error attributes to match {user_id: 456, context: "expected"}, but got these mismatches: {user_id: 456, context: "expected"} and actual values are {"user_id" => 123, "context" => "actual"}/) end it "fails when no error is reported but attributes are expected" do expect { - expect { "no error" }.to have_reported_error.with(user_id: 123) + expect { "no error" }.to have_reported_error.with_context(user_id: 123) }.to fail_with(/Expected the block to report an error, but none was reported./) end end From 1b6deb643ea27f89f35e64dddee4c84dc0eaf5f6 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Wed, 2 Jul 2025 00:15:59 +0200 Subject: [PATCH 17/22] have_reported_error to accept class name and a message --- features/matchers/README.md | 17 ++- .../rails/matchers/have_reported_error.rb | 112 ++++++++++++------ .../matchers/have_reported_error_spec.rb | 52 ++++++-- 3 files changed, 135 insertions(+), 46 deletions(-) diff --git a/features/matchers/README.md b/features/matchers/README.md index 570687edc..d729fef95 100644 --- a/features/matchers/README.md +++ b/features/matchers/README.md @@ -28,6 +28,21 @@ expect(assigns(:widget)).to be_a_new(Widget) ### error reporting ```ruby -# passes when `Rails.error.report` is called with specific error instance and message +# passes when any error is reported +expect { Rails.error.report(StandardError.new) }.to have_reported_error + +# passes when specific error class is reported +expect { Rails.error.report(MyError.new) }.to have_reported_error(MyError) + +# passes when specific error class with exact message is reported +expect { Rails.error.report(MyError.new("message")) }.to have_reported_error(MyError, "message") + +# passes when specific error class with message matching pattern is reported +expect { Rails.error.report(MyError.new("test message")) }.to have_reported_error(MyError, /test/) + +# passes when error is reported with specific context attributes +expect { Rails.error.report(StandardError.new, context: { user_id: 123 }) }.to have_reported_error.with_context(user_id: 123) + +# backward compatibility - accepts error instances expect { Rails.error.report(MyError.new("message")) }.to have_reported_error(MyError.new("message")) ``` diff --git a/lib/rspec/rails/matchers/have_reported_error.rb b/lib/rspec/rails/matchers/have_reported_error.rb index 1022db620..2e5eba485 100644 --- a/lib/rspec/rails/matchers/have_reported_error.rb +++ b/lib/rspec/rails/matchers/have_reported_error.rb @@ -23,8 +23,24 @@ def report(error, **attrs) # @api private # @see RSpec::Rails::Matchers#have_reported_error class HaveReportedError < RSpec::Rails::Matchers::BaseMatcher - def initialize(expected_error = nil) - @expected_error = expected_error + def initialize(expected_error_class = nil, expected_message = nil) + # Handle backward compatibility with old API + if expected_error_class.is_a?(Exception) + @expected_error_class = expected_error_class.class + @expected_message = expected_error_class.message.empty? ? nil : expected_error_class.message + elsif expected_error_class.is_a?(Regexp) + @expected_error_class = nil + @expected_message = expected_error_class + elsif expected_error_class.is_a?(Symbol) + @expected_error_symbol = expected_error_class + @expected_error_class = nil + @expected_message = nil + else + @expected_error_class = expected_error_class + @expected_message = expected_message + @expected_error_symbol = nil + end + @attributes = {} @error_subscriber = nil end @@ -48,7 +64,7 @@ def matches?(block) block.call - return false if @error_subscriber.events.empty? && !@expected_error.nil? + return false if @error_subscriber.events.empty? return false unless error_matches_expectation? return attributes_match_if_specified? @@ -62,16 +78,18 @@ def supports_block_expectations? def description desc = "report an error" - case @expected_error - when Class - desc = "report a #{@expected_error} error" - when Exception - desc = "report a #{@expected_error.class} error" - desc += " with message '#{@expected_error.message}'" unless @expected_error.message.empty? - when Regexp - desc = "report an error with message matching #{@expected_error}" - when Symbol - desc = "report #{@expected_error}" + if @expected_error_symbol + desc = "report #{@expected_error_symbol}" + elsif @expected_error_class + desc = "report a #{@expected_error_class} error" + end + if @expected_message + case @expected_message + when Regexp + desc += " with message matching #{@expected_message}" + when String + desc += " with message '#{@expected_message}'" + end end desc += " with #{@attributes}" unless @attributes.empty? desc @@ -87,15 +105,17 @@ def failure_message elsif @error_subscriber.events.empty? return 'Expected the block to report an error, but none was reported.' else - case @expected_error - when Class - return "Expected error to be an instance of #{@expected_error}, but got #{actual_error.class} with message: '#{actual_error.message}'" - when Exception - return "Expected error to be #{@expected_error.class} with message '#{@expected_error.message}', but got #{actual_error.class} with message: '#{actual_error.message}'" - when Regexp - return "Expected error message to match #{@expected_error}, but got: '#{actual_error.message}'" - when Symbol - return "Expected error to be #{@expected_error}, but got: #{actual_error}" + if @expected_error_symbol + return "Expected error to be #{@expected_error_symbol}, but got: #{actual_error}" + elsif @expected_error_class && !actual_error.is_a?(@expected_error_class) + return "Expected error to be an instance of #{@expected_error_class}, but got #{actual_error.class} with message: '#{actual_error.message}'" + elsif @expected_message + case @expected_message + when Regexp + return "Expected error message to match #{@expected_message}, but got: '#{actual_error.message}'" + when String + return "Expected error message to be '#{@expected_message}', but got: '#{actual_error.message}'" + end else return "Expected specific error, but got #{actual_error.class} with message: '#{actual_error.message}'" end @@ -113,19 +133,30 @@ def failure_message_when_negated private def error_matches_expectation? - return true if @expected_error.nil? && @error_subscriber.events.any? - - case @expected_error - when Class - actual_error.is_a?(@expected_error) - when Exception - actual_error.is_a?(@expected_error.class) && - (@expected_error.message.empty? || actual_error.message == @expected_error.message) - when Regexp - actual_error.message&.match(@expected_error) - when Symbol - actual_error == @expected_error + # If no events were reported, we can't match anything + return false if @error_subscriber.events.empty? + + # Handle symbol matching (backward compatibility) + if @expected_error_symbol + return actual_error == @expected_error_symbol + end + + # If no constraints are given, any error should match + return true if @expected_error_class.nil? && @expected_message.nil? + + class_matches = @expected_error_class.nil? || actual_error.is_a?(@expected_error_class) + + message_matches = if @expected_message.nil? + true + elsif @expected_message.is_a?(Regexp) + actual_error.message&.match(@expected_message) + elsif @expected_message.is_a?(String) + actual_error.message == @expected_message + else + false end + + class_matches && message_matches end def attributes_match_if_specified? @@ -172,21 +203,26 @@ def unmatched_attributes(actual) # @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 + # @example Checking for specific error class with message + # expect { Rails.error.report(MyError.new("message")) }.to have_reported_error(MyError, "message") + # + # @example Checking for specific error instance (backward compatibility) # 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(context: "test") # # @example Checking error message patterns + # expect { Rails.error.report(StandardError.new("test message")) }.to have_reported_error(StandardError, /test/) # expect { Rails.error.report(StandardError.new("test message")) }.to have_reported_error(/test/) # # @example Negation # expect { "safe code" }.not_to have_reported_error # - # @param expected_error [Class, Exception, Regexp, Symbol, nil] the expected error to match - def have_reported_error(expected_error = nil) - HaveReportedError.new(expected_error) + # @param expected_error_class [Class, Exception, Regexp, Symbol, nil] the expected error class to match, or error instance for backward compatibility + # @param expected_message [String, Regexp, nil] the expected error message to match + def have_reported_error(expected_error_class = nil, expected_message = nil) + HaveReportedError.new(expected_error_class, expected_message) end alias_method :reports_error, :have_reported_error diff --git a/spec/rspec/rails/matchers/have_reported_error_spec.rb b/spec/rspec/rails/matchers/have_reported_error_spec.rb index 5b3dc67db..9ae56ef97 100644 --- a/spec/rspec/rails/matchers/have_reported_error_spec.rb +++ b/spec/rspec/rails/matchers/have_reported_error_spec.rb @@ -46,21 +46,21 @@ class AnotherTestError < StandardError; end it "passes with an error that matches exactly" do expect { Rails.error.report(TestError.new("exact message")) - }.to have_reported_error(TestError.new("exact message")) + }.to have_reported_error(TestError, "exact message") end - it "passes any error of the same class if the expected message is empty" do + it "passes any error of the same class if no message is specified" do expect { Rails.error.report(TestError.new("any message")) - }.to have_reported_error(TestError.new("")) + }.to have_reported_error(TestError) end it "fails when the error has different message to the expected" do expect { expect { Rails.error.report(TestError.new("actual message")) - }.to have_reported_error(TestError.new("expected message")) - }.to fail_with(/Expected error to be TestError with message 'expected message', but got TestError with message: 'actual message'/) + }.to have_reported_error(TestError, "expected message") + }.to fail_with(/Expected error message to be 'expected message', but got: 'actual message'/) end end @@ -68,14 +68,14 @@ class AnotherTestError < StandardError; end it "passes when an error message matches the pattern" do expect { Rails.error.report(StandardError.new("error with pattern")) - }.to have_reported_error(/with pattern/) + }.to have_reported_error(StandardError, /with pattern/) end it "fails when no error messages match the pattern" do expect { expect { Rails.error.report(StandardError.new("error without match")) - }.to have_reported_error(/different pattern/) + }.to have_reported_error(StandardError, /different pattern/) }.to fail_with(/Expected error message to match/) end end @@ -152,6 +152,44 @@ class AnotherTestError < StandardError; end end end + context "backward compatibility with old API" do + it "accepts Exception instances and matches class and message" do + expect { + Rails.error.report(TestError.new("exact message")) + }.to have_reported_error(TestError.new("exact message")) + end + + it "accepts Exception instances with empty message and matches any message of that class" do + expect { + Rails.error.report(TestError.new("any message")) + }.to have_reported_error(TestError.new("")) + end + + it "accepts Regexp directly for message matching" do + expect { + Rails.error.report(StandardError.new("error with pattern")) + }.to have_reported_error(/with pattern/) + end + + + + it "fails when Exception instance class doesn't match" do + expect { + expect { + Rails.error.report(AnotherTestError.new("message")) + }.to have_reported_error(TestError.new("message")) + }.to fail_with(/Expected error to be an instance of TestError, but got AnotherTestError/) + end + + it "fails when Exception instance message doesn't match" do + expect { + expect { + Rails.error.report(TestError.new("actual message")) + }.to have_reported_error(TestError.new("expected message")) + }.to fail_with(/Expected error message to be 'expected message', but got: 'actual message'/) + end + end + describe "integration with actual usage patterns" do it "works with multiple error reports in a block" do expect { From c77fca50de21f74d1cbca0a9822984b49603c485 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Wed, 2 Jul 2025 00:29:41 +0200 Subject: [PATCH 18/22] switch to error_report matcher type of api --- features/matchers/README.md | 9 +- .../have_reported_error_matcher.feature | 56 +++++++++---- .../rails/matchers/have_reported_error.rb | 83 +++++++++---------- .../matchers/have_reported_error_spec.rb | 36 ++++---- 4 files changed, 101 insertions(+), 83 deletions(-) diff --git a/features/matchers/README.md b/features/matchers/README.md index d729fef95..9b67683ee 100644 --- a/features/matchers/README.md +++ b/features/matchers/README.md @@ -40,9 +40,14 @@ expect { Rails.error.report(MyError.new("message")) }.to have_reported_error(MyE # passes when specific error class with message matching pattern is reported expect { Rails.error.report(MyError.new("test message")) }.to have_reported_error(MyError, /test/) +# passes when any error with exact message is reported +expect { Rails.error.report(StandardError.new("exact message")) }.to have_reported_error("exact message") + +# passes when any error with message matching pattern is reported +expect { Rails.error.report(StandardError.new("test message")) }.to have_reported_error(/test/) + # passes when error is reported with specific context attributes expect { Rails.error.report(StandardError.new, context: { user_id: 123 }) }.to have_reported_error.with_context(user_id: 123) -# backward compatibility - accepts error instances -expect { Rails.error.report(MyError.new("message")) }.to have_reported_error(MyError.new("message")) + ``` diff --git a/features/matchers/have_reported_error_matcher.feature b/features/matchers/have_reported_error_matcher.feature index 399f0ebff..3c3a4379d 100644 --- a/features/matchers/have_reported_error_matcher.feature +++ b/features/matchers/have_reported_error_matcher.feature @@ -2,15 +2,15 @@ Feature: `have_reported_error` matcher The `have_reported_error` matcher is used to check if an error was reported to Rails error reporting system (`Rails.error`). It can match against error - classes, instances, messages, and attributes. + classes, messages, and attributes. The matcher supports several matching strategies: * Any error reported * A specific error class - * Specific error instance with message + * A specific error class with message * Error message patterns using regular expressions - * Error attributes using `.with()` - * Symbol errors + * Message-only matching (any class) + * Error attributes using `.with_context()` The matcher is available in all spec types where Rails error reporting is used. @@ -24,7 +24,7 @@ Feature: `have_reported_error` matcher end def self.process_with_context - Rails.error.report(ArgumentError.new("Invalid input"), context: "user_processing", severity: "high") + Rails.error.report(ArgumentError.new("Invalid input"), context: { context: "user_processing", severity: :error }) end def self.process_custom_error @@ -49,6 +49,28 @@ Feature: `have_reported_error` matcher When I run `rspec spec/models/user_spec.rb` Then the examples should all pass + Scenario: Checking for message-only matching + Given a file named "spec/models/user_spec.rb" with: + """ruby + require "rails_helper" + + RSpec.describe User do + it "reports error with exact message (any class)" do + expect { + User.process_data + }.to have_reported_error("Processing failed") + end + + it "reports error with message pattern (any class)" do + expect { + User.process_custom_error + }.to have_reported_error(/Email/) + end + end + """ + When I run `rspec spec/models/user_spec.rb` + Then the examples should all pass + Scenario: Checking for a specific error class Given a file named "spec/models/user_spec.rb" with: """ruby @@ -71,7 +93,7 @@ Feature: `have_reported_error` matcher When I run `rspec spec/models/user_spec.rb` Then the examples should all pass - Scenario: Checking for specific error instance with message + Scenario: Checking for specific error class with message Given a file named "spec/models/user_spec.rb" with: """ruby require "rails_helper" @@ -80,13 +102,13 @@ Feature: `have_reported_error` matcher it "reports error with specific message" do expect { User.process_data - }.to have_reported_error(StandardError.new("Processing failed")) + }.to have_reported_error(StandardError, "Processing failed") end it "reports ArgumentError with specific message" do expect { User.process_with_context - }.to have_reported_error(ArgumentError.new("Invalid input")) + }.to have_reported_error(ArgumentError, "Invalid input") end end """ @@ -99,17 +121,23 @@ Feature: `have_reported_error` matcher require "rails_helper" RSpec.describe User do - it "reports errors with a message matching a pattern" do + it "reports errors with a message matching a pattern (any class)" do expect { User.process_data }.to have_reported_error(/Processing/) end + + it "reports specific class with message matching a pattern" do + expect { + User.process_data + }.to have_reported_error(StandardError, /Processing/) + end end """ When I run `rspec spec/models/user_spec.rb` Then the examples should all pass - Scenario: Constraining error matches to their attributes using `with` + Scenario: Constraining error matches to their attributes using `with_context` Given a file named "spec/models/user_spec.rb" with: """ruby require "rails_helper" @@ -118,13 +146,13 @@ Feature: `have_reported_error` matcher it "reports error with specific context" do expect { User.process_with_context - }.to have_reported_error.with(context: "user_processing") + }.to have_reported_error.with_context(context: "user_processing") end it "reports error with multiple attributes" do expect { User.process_with_context - }.to have_reported_error(ArgumentError).with(context: "user_processing", severity: "high") + }.to have_reported_error(ArgumentError).with_context(context: "user_processing", severity: :error) end end """ @@ -140,13 +168,13 @@ Feature: `have_reported_error` matcher it "reports a ValidationError" do expect { User.process_custom_error - }.to have_reported_error(ValidationError) + }.to have_reported_error(User::ValidationError) end it "reports ValidationError with specific message" do expect { User.process_custom_error - }.to have_reported_error(ValidationError.new("Email is invalid")) + }.to have_reported_error(User::ValidationError, "Email is invalid") end end """ diff --git a/lib/rspec/rails/matchers/have_reported_error.rb b/lib/rspec/rails/matchers/have_reported_error.rb index 2e5eba485..8bc67f61e 100644 --- a/lib/rspec/rails/matchers/have_reported_error.rb +++ b/lib/rspec/rails/matchers/have_reported_error.rb @@ -23,22 +23,16 @@ def report(error, **attrs) # @api private # @see RSpec::Rails::Matchers#have_reported_error class HaveReportedError < RSpec::Rails::Matchers::BaseMatcher - def initialize(expected_error_class = nil, expected_message = nil) - # Handle backward compatibility with old API - if expected_error_class.is_a?(Exception) - @expected_error_class = expected_error_class.class - @expected_message = expected_error_class.message.empty? ? nil : expected_error_class.message - elsif expected_error_class.is_a?(Regexp) + def initialize(expected_error_or_message = nil, expected_message = nil) + if expected_error_or_message.is_a?(Regexp) @expected_error_class = nil - @expected_message = expected_error_class - elsif expected_error_class.is_a?(Symbol) - @expected_error_symbol = expected_error_class + @expected_message = expected_error_or_message + elsif expected_error_or_message.is_a?(String) @expected_error_class = nil - @expected_message = nil + @expected_message = expected_error_or_message else - @expected_error_class = expected_error_class + @expected_error_class = expected_error_or_message @expected_message = expected_message - @expected_error_symbol = nil end @attributes = {} @@ -77,22 +71,26 @@ def supports_block_expectations? end def description - desc = "report an error" - if @expected_error_symbol - desc = "report #{@expected_error_symbol}" - elsif @expected_error_class - desc = "report a #{@expected_error_class} error" - end - if @expected_message - case @expected_message - when Regexp - desc += " with message matching #{@expected_message}" - when String - desc += " with message '#{@expected_message}'" - end - end - desc += " with #{@attributes}" unless @attributes.empty? - desc + base_desc = if @expected_error_class + "report a #{@expected_error_class} error" + else + "report an error" + end + + message_desc = if @expected_message + case @expected_message + when Regexp + " with message matching #{@expected_message}" + when String + " with message '#{@expected_message}'" + end + else + "" + end + + attributes_desc = @attributes.empty? ? "" : " with #{@attributes}" + + base_desc + message_desc + attributes_desc end def failure_message @@ -105,9 +103,7 @@ def failure_message elsif @error_subscriber.events.empty? return 'Expected the block to report an error, but none was reported.' else - if @expected_error_symbol - return "Expected error to be #{@expected_error_symbol}, but got: #{actual_error}" - elsif @expected_error_class && !actual_error.is_a?(@expected_error_class) + if @expected_error_class && !actual_error.is_a?(@expected_error_class) return "Expected error to be an instance of #{@expected_error_class}, but got #{actual_error.class} with message: '#{actual_error.message}'" elsif @expected_message case @expected_message @@ -136,11 +132,6 @@ def error_matches_expectation? # If no events were reported, we can't match anything return false if @error_subscriber.events.empty? - # Handle symbol matching (backward compatibility) - if @expected_error_symbol - return actual_error == @expected_error_symbol - end - # If no constraints are given, any error should match return true if @expected_error_class.nil? && @expected_message.nil? @@ -206,23 +197,25 @@ def unmatched_attributes(actual) # @example Checking for specific error class with message # expect { Rails.error.report(MyError.new("message")) }.to have_reported_error(MyError, "message") # - # @example Checking for specific error instance (backward compatibility) - # expect { Rails.error.report(MyError.new("message")) }.to have_reported_error(MyError.new("message")) + # @example Checking for error with exact message (any class) + # expect { Rails.error.report(StandardError.new("exact message")) }.to have_reported_error("exact message") # - # @example Checking error attributes - # expect { Rails.error.report(StandardError.new, context: "test") }.to have_reported_error.with_context(context: "test") + # @example Checking for error with message pattern (any class) + # expect { Rails.error.report(StandardError.new("test message")) }.to have_reported_error(/test/) # - # @example Checking error message patterns + # @example Checking for specific error class with message pattern # expect { Rails.error.report(StandardError.new("test message")) }.to have_reported_error(StandardError, /test/) - # expect { Rails.error.report(StandardError.new("test message")) }.to have_reported_error(/test/) + # + # @example Checking error attributes + # expect { Rails.error.report(StandardError.new, context: "test") }.to have_reported_error.with_context(context: "test") # # @example Negation # expect { "safe code" }.not_to have_reported_error # - # @param expected_error_class [Class, Exception, Regexp, Symbol, nil] the expected error class to match, or error instance for backward compatibility + # @param expected_error_or_message [Class, String, Regexp, nil] the expected error class, message string, or message pattern # @param expected_message [String, Regexp, nil] the expected error message to match - def have_reported_error(expected_error_class = nil, expected_message = nil) - HaveReportedError.new(expected_error_class, expected_message) + def have_reported_error(expected_error_or_message = nil, expected_message = nil) + HaveReportedError.new(expected_error_or_message, expected_message) end alias_method :reports_error, :have_reported_error diff --git a/spec/rspec/rails/matchers/have_reported_error_spec.rb b/spec/rspec/rails/matchers/have_reported_error_spec.rb index 9ae56ef97..1eda987a1 100644 --- a/spec/rspec/rails/matchers/have_reported_error_spec.rb +++ b/spec/rspec/rails/matchers/have_reported_error_spec.rb @@ -152,41 +152,33 @@ class AnotherTestError < StandardError; end end end - context "backward compatibility with old API" do - it "accepts Exception instances and matches class and message" do + context "constrained by message only" do + it "passes when any error with exact message is reported" do expect { - Rails.error.report(TestError.new("exact message")) - }.to have_reported_error(TestError.new("exact message")) + Rails.error.report(StandardError.new("exact message")) + }.to have_reported_error("exact message") end - it "accepts Exception instances with empty message and matches any message of that class" do + it "passes when any error with message matching pattern is reported" do expect { - Rails.error.report(TestError.new("any message")) - }.to have_reported_error(TestError.new("")) - end - - it "accepts Regexp directly for message matching" do - expect { - Rails.error.report(StandardError.new("error with pattern")) + Rails.error.report(AnotherTestError.new("error with pattern")) }.to have_reported_error(/with pattern/) end - - - it "fails when Exception instance class doesn't match" do + it "fails when no error with exact message is reported" do expect { expect { - Rails.error.report(AnotherTestError.new("message")) - }.to have_reported_error(TestError.new("message")) - }.to fail_with(/Expected error to be an instance of TestError, but got AnotherTestError/) + Rails.error.report(StandardError.new("actual message")) + }.to have_reported_error("expected message") + }.to fail_with(/Expected error message to be 'expected message', but got: 'actual message'/) end - it "fails when Exception instance message doesn't match" do + it "fails when no error with matching pattern is reported" do expect { expect { - Rails.error.report(TestError.new("actual message")) - }.to have_reported_error(TestError.new("expected message")) - }.to fail_with(/Expected error message to be 'expected message', but got: 'actual message'/) + Rails.error.report(StandardError.new("error without match")) + }.to have_reported_error(/different pattern/) + }.to fail_with(/Expected error message to match/) end end From f5e427a3aee9fbb22a07da68d42907eaf4df2735 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Wed, 2 Jul 2025 00:34:17 +0200 Subject: [PATCH 19/22] refactor --- .../rails/matchers/have_reported_error.rb | 70 ++++++++++++------- .../matchers/have_reported_error_spec.rb | 2 +- 2 files changed, 45 insertions(+), 27 deletions(-) diff --git a/lib/rspec/rails/matchers/have_reported_error.rb b/lib/rspec/rails/matchers/have_reported_error.rb index 8bc67f61e..9a47ce6bb 100644 --- a/lib/rspec/rails/matchers/have_reported_error.rb +++ b/lib/rspec/rails/matchers/have_reported_error.rb @@ -20,23 +20,34 @@ def report(error, **attrs) # Matcher class for `have_reported_error`. Should not be instantiated directly. # + # Provides a way to test that an error was reported to Rails.error. + # This matcher follows the same patterns as RSpec's built-in `raise_error` matcher. + # # @api private # @see RSpec::Rails::Matchers#have_reported_error class HaveReportedError < RSpec::Rails::Matchers::BaseMatcher + # Initialize the matcher following raise_error patterns + # + # @param expected_error_or_message [Class, String, Regexp, nil] + # Error class, message string, or message pattern + # @param expected_message [String, Regexp, nil] + # Expected message when first param is a class def initialize(expected_error_or_message = nil, expected_message = nil) - if expected_error_or_message.is_a?(Regexp) - @expected_error_class = nil - @expected_message = expected_error_or_message - elsif expected_error_or_message.is_a?(String) - @expected_error_class = nil + @actual_error = nil + @attributes = {} + @error_subscriber = nil + + case expected_error_or_message + when nil + @expected_error = nil + @expected_message = expected_message + when String, Regexp + @expected_error = nil @expected_message = expected_error_or_message else - @expected_error_class = expected_error_or_message + @expected_error = expected_error_or_message @expected_message = expected_message end - - @attributes = {} - @error_subscriber = nil end def with_context(expected_attributes) @@ -48,9 +59,10 @@ def and(_) raise ArgumentError, "Chaining is not supported" end + # Check if the block reports an error matching our expectations def matches?(block) if block.nil? - raise ArgumentError, "this matcher doesn’t work with value expectations" + raise ArgumentError, "this matcher doesn't work with value expectations" end @error_subscriber = ErrorSubscriber.new @@ -71,8 +83,8 @@ def supports_block_expectations? end def description - base_desc = if @expected_error_class - "report a #{@expected_error_class} error" + base_desc = if @expected_error + "report a #{@expected_error} error" else "report an error" end @@ -103,8 +115,8 @@ def failure_message elsif @error_subscriber.events.empty? return 'Expected the block to report an error, but none was reported.' else - if @expected_error_class && !actual_error.is_a?(@expected_error_class) - return "Expected error to be an instance of #{@expected_error_class}, but got #{actual_error.class} with message: '#{actual_error.message}'" + if @expected_error && !actual_error.is_a?(@expected_error) + return "Expected error to be an instance of #{@expected_error}, but got #{actual_error.class} with message: '#{actual_error.message}'" elsif @expected_message case @expected_message when Regexp @@ -128,26 +140,31 @@ def failure_message_when_negated private + # Check if the reported error matches our class and message expectations def error_matches_expectation? - # If no events were reported, we can't match anything return false if @error_subscriber.events.empty? - - # If no constraints are given, any error should match - return true if @expected_error_class.nil? && @expected_message.nil? + return true if @expected_error.nil? && @expected_message.nil? + + error_class_matches? && error_message_matches? + end - class_matches = @expected_error_class.nil? || actual_error.is_a?(@expected_error_class) + # Check if the actual error class matches the expected error class + def error_class_matches? + @expected_error.nil? || actual_error.is_a?(@expected_error) + end + + # Check if the actual error message matches the expected message pattern + def error_message_matches? + return true if @expected_message.nil? - message_matches = if @expected_message.nil? - true - elsif @expected_message.is_a?(Regexp) + case @expected_message + when Regexp actual_error.message&.match(@expected_message) - elsif @expected_message.is_a?(String) + when String actual_error.message == @expected_message else false end - - class_matches && message_matches end def attributes_match_if_specified? @@ -158,8 +175,9 @@ def attributes_match_if_specified? attributes_match?(event_context) end + # Get the actual error that was reported (cached) def actual_error - @error_subscriber.events.empty? ? nil : @error_subscriber.events.last.error + @actual_error ||= (@error_subscriber.events.empty? ? nil : @error_subscriber.events.last.error) end def attributes_match?(actual) diff --git a/spec/rspec/rails/matchers/have_reported_error_spec.rb b/spec/rspec/rails/matchers/have_reported_error_spec.rb index 1eda987a1..4f912e089 100644 --- a/spec/rspec/rails/matchers/have_reported_error_spec.rb +++ b/spec/rspec/rails/matchers/have_reported_error_spec.rb @@ -9,7 +9,7 @@ class AnotherTestError < StandardError; end it "warns when used as a value expectation" do expect { expect(Rails.error.report(StandardError.new("test error"))).to have_reported_error - }.to raise_error(ArgumentError, "this matcher doesn’t work with value expectations") + }.to raise_error(ArgumentError, "this matcher doesn't work with value expectations") end context "without constraint" do From 7b3abc595ac9e7d45fa0404884859d5272b19295 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Wed, 2 Jul 2025 00:46:42 +0200 Subject: [PATCH 20/22] Using UndefinedValue as a default attribute --- lib/rspec/rails/matchers/have_reported_error.rb | 14 +++++++++++--- .../rails/matchers/have_reported_error_spec.rb | 10 ++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/lib/rspec/rails/matchers/have_reported_error.rb b/lib/rspec/rails/matchers/have_reported_error.rb index 9a47ce6bb..fb74e7246 100644 --- a/lib/rspec/rails/matchers/have_reported_error.rb +++ b/lib/rspec/rails/matchers/have_reported_error.rb @@ -3,6 +3,11 @@ module RSpec module Rails module Matchers + # @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 + # @api private class ErrorSubscriber attr_reader :events @@ -28,17 +33,20 @@ def report(error, **attrs) class HaveReportedError < RSpec::Rails::Matchers::BaseMatcher # Initialize the matcher following raise_error patterns # + # Uses UndefinedValue as default to distinguish between no argument + # passed vs explicitly passed nil (same as raise_error matcher). + # # @param expected_error_or_message [Class, String, Regexp, nil] # Error class, message string, or message pattern # @param expected_message [String, Regexp, nil] # Expected message when first param is a class - def initialize(expected_error_or_message = nil, expected_message = nil) + def initialize(expected_error_or_message = UndefinedValue, expected_message = nil) @actual_error = nil @attributes = {} @error_subscriber = nil case expected_error_or_message - when nil + when nil, UndefinedValue @expected_error = nil @expected_message = expected_message when String, Regexp @@ -232,7 +240,7 @@ def unmatched_attributes(actual) # # @param expected_error_or_message [Class, String, Regexp, nil] the expected error class, message string, or message pattern # @param expected_message [String, Regexp, nil] the expected error message to match - def have_reported_error(expected_error_or_message = nil, expected_message = nil) + def have_reported_error(expected_error_or_message = UndefinedValue, expected_message = nil) HaveReportedError.new(expected_error_or_message, expected_message) end diff --git a/spec/rspec/rails/matchers/have_reported_error_spec.rb b/spec/rspec/rails/matchers/have_reported_error_spec.rb index 4f912e089..8bcb997d9 100644 --- a/spec/rspec/rails/matchers/have_reported_error_spec.rb +++ b/spec/rspec/rails/matchers/have_reported_error_spec.rb @@ -17,12 +17,22 @@ class AnotherTestError < StandardError; end expect {Rails.error.report(StandardError.new("test error"))}.to have_reported_error end + it "passes when an error is reported with explicit nil argument" do + expect {Rails.error.report(StandardError.new("test error"))}.to have_reported_error(nil) + end + it "fails when no errors are reported" do expect { expect { "no error" }.to have_reported_error }.to fail_with(/Expected the block to report an error, but none was reported./) end + it "fails when no errors are reported with explicit nil argument" do + expect { + expect { "no error" }.to have_reported_error(nil) + }.to fail_with(/Expected the block to report an error, but none was reported./) + end + it "passes when negated and no errors are reported" do expect { "no error" }.not_to have_reported_error end From 2488a714062e9736aa7471f463901a663928cdfa Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Wed, 2 Jul 2025 01:00:00 +0200 Subject: [PATCH 21/22] Look through all errors, don't just go with last error --- .../rails/matchers/have_reported_error.rb | 76 ++++++++++--------- .../matchers/have_reported_error_spec.rb | 10 +-- 2 files changed, 46 insertions(+), 40 deletions(-) diff --git a/lib/rspec/rails/matchers/have_reported_error.rb b/lib/rspec/rails/matchers/have_reported_error.rb index fb74e7246..db5aebcf4 100644 --- a/lib/rspec/rails/matchers/have_reported_error.rb +++ b/lib/rspec/rails/matchers/have_reported_error.rb @@ -1,5 +1,3 @@ -require "rspec/rails/matchers/base_matcher" - module RSpec module Rails module Matchers @@ -26,25 +24,20 @@ def report(error, **attrs) # Matcher class for `have_reported_error`. Should not be instantiated directly. # # Provides a way to test that an error was reported to Rails.error. - # This matcher follows the same patterns as RSpec's built-in `raise_error` matcher. # # @api private # @see RSpec::Rails::Matchers#have_reported_error class HaveReportedError < RSpec::Rails::Matchers::BaseMatcher - # Initialize the matcher following raise_error patterns - # - # Uses UndefinedValue as default to distinguish between no argument - # passed vs explicitly passed nil (same as raise_error matcher). + # Uses UndefinedValue as default to distinguish between no argument + # passed vs explicitly passed nil. # - # @param expected_error_or_message [Class, String, Regexp, nil] + # @param expected_error_or_message [Class, String, Regexp, nil] # Error class, message string, or message pattern - # @param expected_message [String, Regexp, nil] + # @param expected_message [String, Regexp, nil] # Expected message when first param is a class def initialize(expected_error_or_message = UndefinedValue, expected_message = nil) - @actual_error = nil @attributes = {} - @error_subscriber = nil - + case expected_error_or_message when nil, UndefinedValue @expected_error = nil @@ -67,7 +60,6 @@ def and(_) raise ArgumentError, "Chaining is not supported" end - # Check if the block reports an error matching our expectations def matches?(block) if block.nil? raise ArgumentError, "this matcher doesn't work with value expectations" @@ -122,16 +114,22 @@ def failure_message end elsif @error_subscriber.events.empty? return 'Expected the block to report an error, but none was reported.' + elsif actual_error.nil? + reported_errors = @error_subscriber.events.map { |event| "#{event.error.class}: '#{event.error.message}'" }.join(', ') + if @expected_error && @expected_message + return "Expected error to be an instance of #{@expected_error} with message '#{@expected_message}', but got: #{reported_errors}" + elsif @expected_error + return "Expected error to be an instance of #{@expected_error}, but got: #{reported_errors}" + elsif @expected_message.is_a?(Regexp) + return "Expected error message to match #{@expected_message}, but got: #{reported_errors}" + elsif @expected_message.is_a?(String) + return "Expected error message to be '#{@expected_message}', but got: #{reported_errors}" + end else if @expected_error && !actual_error.is_a?(@expected_error) return "Expected error to be an instance of #{@expected_error}, but got #{actual_error.class} with message: '#{actual_error.message}'" elsif @expected_message - case @expected_message - when Regexp - return "Expected error message to match #{@expected_message}, but got: '#{actual_error.message}'" - when String - return "Expected error message to be '#{@expected_message}', but got: '#{actual_error.message}'" - end + return "Expected error message to #{@expected_message.is_a?(Regexp) ? "match" : "be" } #{@expected_message}, but got: '#{actual_error.message}'" else return "Expected specific error, but got #{actual_error.class} with message: '#{actual_error.message}'" end @@ -148,28 +146,27 @@ def failure_message_when_negated private - # Check if the reported error matches our class and message expectations def error_matches_expectation? - return false if @error_subscriber.events.empty? - return true if @expected_error.nil? && @expected_message.nil? + return true if @expected_error.nil? && @expected_message.nil? && @error_subscriber.events.count.positive? - error_class_matches? && error_message_matches? + @error_subscriber.events.any? do |event| + error_class_matches?(event.error) && error_message_matches?(event.error) + end end - # Check if the actual error class matches the expected error class - def error_class_matches? - @expected_error.nil? || actual_error.is_a?(@expected_error) + def error_class_matches?(error) + @expected_error.nil? || error.is_a?(@expected_error) end - # Check if the actual error message matches the expected message pattern - def error_message_matches? + # Check if the given error message matches the expected message pattern + def error_message_matches?(error) return true if @expected_message.nil? - + case @expected_message when Regexp - actual_error.message&.match(@expected_message) + error.message&.match(@expected_message) when String - actual_error.message == @expected_message + error.message == @expected_message else false end @@ -177,15 +174,24 @@ def error_message_matches? def attributes_match_if_specified? return true if @attributes.empty? - return false if @error_subscriber.events.empty? + return false unless matching_event - event_context = @error_subscriber.events.last.attributes[:context] + event_context = matching_event.attributes[:context] attributes_match?(event_context) end - # Get the actual error that was reported (cached) def actual_error - @actual_error ||= (@error_subscriber.events.empty? ? nil : @error_subscriber.events.last.error) + @actual_error ||= matching_event&.error + end + + def matching_event + @matching_event ||= find_matching_event + end + + def find_matching_event + @error_subscriber.events.find do |event| + error_class_matches?(event.error) && error_message_matches?(event.error) + end end def attributes_match?(actual) diff --git a/spec/rspec/rails/matchers/have_reported_error_spec.rb b/spec/rspec/rails/matchers/have_reported_error_spec.rb index 8bcb997d9..68e91dc1d 100644 --- a/spec/rspec/rails/matchers/have_reported_error_spec.rb +++ b/spec/rspec/rails/matchers/have_reported_error_spec.rb @@ -48,7 +48,7 @@ class AnotherTestError < StandardError; end expect { Rails.error.report(AnotherTestError.new("wrong error")) }.to have_reported_error(TestError) - }.to fail_with(/Expected error to be an instance of TestError, but got AnotherTestError/) + }.to fail_with(/Expected error to be an instance of TestError, but got: AnotherTestError/) end end @@ -70,7 +70,7 @@ class AnotherTestError < StandardError; end expect { Rails.error.report(TestError.new("actual message")) }.to have_reported_error(TestError, "expected message") - }.to fail_with(/Expected error message to be 'expected message', but got: 'actual message'/) + }.to fail_with(/Expected error to be an instance of TestError with message 'expected message', but got: TestError/) end end @@ -86,7 +86,7 @@ class AnotherTestError < StandardError; end expect { Rails.error.report(StandardError.new("error without match")) }.to have_reported_error(StandardError, /different pattern/) - }.to fail_with(/Expected error message to match/) + }.to fail_with(/Expected error to be an instance of StandardError with message/) end end @@ -180,7 +180,7 @@ class AnotherTestError < StandardError; end expect { Rails.error.report(StandardError.new("actual message")) }.to have_reported_error("expected message") - }.to fail_with(/Expected error message to be 'expected message', but got: 'actual message'/) + }.to fail_with(/Expected error message to be 'expected message', but got: StandardError/) end it "fails when no error with matching pattern is reported" do @@ -188,7 +188,7 @@ class AnotherTestError < StandardError; end expect { Rails.error.report(StandardError.new("error without match")) }.to have_reported_error(/different pattern/) - }.to fail_with(/Expected error message to match/) + }.to fail_with(/Expected error message to match .+different pattern.+, but got: StandardError/) end end From c1816e2f2b14a547e26ddfc0b2b3d9a02c4b9286 Mon Sep 17 00:00:00 2001 From: "Stanislav (Stas) Katkov" Date: Thu, 10 Jul 2025 00:24:04 +0200 Subject: [PATCH 22/22] warn about nil being passed --- lib/rspec/rails/matchers/have_reported_error.rb | 17 ++++++++++++++++- .../rails/matchers/have_reported_error_spec.rb | 8 +++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/lib/rspec/rails/matchers/have_reported_error.rb b/lib/rspec/rails/matchers/have_reported_error.rb index db5aebcf4..e1c04f446 100644 --- a/lib/rspec/rails/matchers/have_reported_error.rb +++ b/lib/rspec/rails/matchers/have_reported_error.rb @@ -37,9 +37,10 @@ class HaveReportedError < RSpec::Rails::Matchers::BaseMatcher # Expected message when first param is a class def initialize(expected_error_or_message = UndefinedValue, expected_message = nil) @attributes = {} + @warn_about_nil_error = expected_error_or_message.nil? case expected_error_or_message - when nil, UndefinedValue + when UndefinedValue @expected_error = nil @expected_message = expected_message when String, Regexp @@ -65,6 +66,8 @@ def matches?(block) raise ArgumentError, "this matcher doesn't work with value expectations" end + warn_about_nil_error! if @warn_about_nil_error + @error_subscriber = ErrorSubscriber.new ::Rails.error.subscribe(@error_subscriber) @@ -213,6 +216,18 @@ def unmatched_attributes(actual) end end end + + def warn_about_nil_error! + RSpec.warn_with("Using the `have_reported_error` matcher with a `nil` error is probably " \ + "unintentional, it risks false positives, since `have_reported_error` " \ + "will match when any error is reported to Rails.error, potentially " \ + "allowing the expectation to pass without the specific error you are " \ + "intending to test for being reported. " \ + "Instead consider providing a specific error class or message. " \ + "This message can be suppressed by setting: " \ + "`RSpec::Expectations.configuration.on_potential_false" \ + "_positives = :nothing`") + end end # @api public diff --git a/spec/rspec/rails/matchers/have_reported_error_spec.rb b/spec/rspec/rails/matchers/have_reported_error_spec.rb index 68e91dc1d..a15476817 100644 --- a/spec/rspec/rails/matchers/have_reported_error_spec.rb +++ b/spec/rspec/rails/matchers/have_reported_error_spec.rb @@ -17,8 +17,10 @@ class AnotherTestError < StandardError; end expect {Rails.error.report(StandardError.new("test error"))}.to have_reported_error end - it "passes when an error is reported with explicit nil argument" do - expect {Rails.error.report(StandardError.new("test error"))}.to have_reported_error(nil) + it "fails when an error is reported with explicit nil argument" do + expect { + expect {Rails.error.report(StandardError.new("test error"))}.to have_reported_error(nil) + }.to raise_error(RuntimeError, /Using the `have_reported_error` matcher with a `nil` error is probably unintentional/) end it "fails when no errors are reported" do @@ -29,7 +31,7 @@ class AnotherTestError < StandardError; end it "fails when no errors are reported with explicit nil argument" do expect { - expect { "no error" }.to have_reported_error(nil) + expect { "no error" }.to have_reported_error }.to fail_with(/Expected the block to report an error, but none was reported./) end