Skip to content

Commit 8335664

Browse files
skatkovJonRowe
andauthored
Apply suggestions from code review
Commit grammar improvements Co-authored-by: Jon Rowe <[email protected]>
1 parent 6135d5d commit 8335664

File tree

4 files changed

+28
-38
lines changed

4 files changed

+28
-38
lines changed

features/matchers/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,6 @@ expect(assigns(:widget)).to be_a_new(Widget)
2828
### error reporting
2929

3030
```ruby
31-
# passes if Rails.error.report was called with specific error instance and message
31+
# passes when `Rails.error.report` is called with specific error instance and message
3232
expect { Rails.error.report(MyError.new("message")) }.to have_reported_error(MyError.new("message"))
3333
```

features/matchers/have_reported_error_matcher.feature

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ Feature: `have_reported_error` matcher
66

77
The matcher supports several matching strategies:
88
* Any error reported
9-
* Specific error class
9+
* A specific error class
1010
* Specific error instance with message
1111
* Error message patterns using regular expressions
1212
* Error attributes using `.with()`
@@ -18,6 +18,7 @@ Feature: `have_reported_error` matcher
1818
Given a file named "app/models/user.rb" with:
1919
"""ruby
2020
class User < ApplicationRecord
21+
class ValidationError < StandardError; end
2122
def self.process_data
2223
Rails.error.report(StandardError.new("Processing failed"))
2324
end
@@ -31,19 +32,14 @@ Feature: `have_reported_error` matcher
3132
end
3233
end
3334
"""
34-
And a file named "app/errors/validation_error.rb" with:
35-
"""ruby
36-
class ValidationError < StandardError
37-
end
38-
"""
3935

4036
Scenario: Checking for any error being reported
4137
Given a file named "spec/models/user_spec.rb" with:
4238
"""ruby
4339
require "rails_helper"
4440
4541
RSpec.describe User do
46-
it "reports an error during processing" do
42+
it "reports errors" do
4743
expect {
4844
User.process_data
4945
}.to have_reported_error
@@ -53,7 +49,7 @@ Feature: `have_reported_error` matcher
5349
When I run `rspec spec/models/user_spec.rb`
5450
Then the examples should all pass
5551

56-
Scenario: Checking for specific error class
52+
Scenario: Checking for a specific error class
5753
Given a file named "spec/models/user_spec.rb" with:
5854
"""ruby
5955
require "rails_helper"
@@ -97,29 +93,23 @@ Feature: `have_reported_error` matcher
9793
When I run `rspec spec/models/user_spec.rb`
9894
Then the examples should all pass
9995

100-
Scenario: Checking error message patterns with regular expressions
96+
Scenario: Checking error messages using regular expressions
10197
Given a file named "spec/models/user_spec.rb" with:
10298
"""ruby
10399
require "rails_helper"
104100
105101
RSpec.describe User do
106-
it "reports error with message matching pattern" do
102+
it "reports errors with a message matching a pattern" do
107103
expect {
108104
User.process_data
109105
}.to have_reported_error(/Processing/)
110106
end
111-
112-
it "reports error with message containing 'failed'" do
113-
expect {
114-
User.process_data
115-
}.to have_reported_error(/failed$/)
116-
end
117107
end
118108
"""
119109
When I run `rspec spec/models/user_spec.rb`
120110
Then the examples should all pass
121111

122-
Scenario: Checking error attributes using `with`
112+
Scenario: Constraining error matches to their attributes using `with`
123113
Given a file named "spec/models/user_spec.rb" with:
124114
"""ruby
125115
require "rails_helper"

lib/rspec/rails/matchers/have_reported_error.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
module RSpec
44
module Rails
55
module Matchers
6-
# @private
6+
# @api private
77
class ErrorSubscriber
88
attr_reader :events
99

@@ -20,7 +20,7 @@ def report(error, **attrs)
2020

2121
# Matcher class for `have_reported_error`. Should not be instantiated directly.
2222
#
23-
# @private
23+
# @api private
2424
# @see RSpec::Rails::Matchers#have_reported_error
2525
class HaveReportedError < RSpec::Rails::Matchers::BaseMatcher
2626
def initialize(expected_error = nil)

spec/rspec/rails/matchers/have_reported_error_spec.rb

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,38 +4,38 @@
44
class TestError < StandardError; end
55
class AnotherTestError < StandardError; end
66

7-
it "has an reports_error alias" do
7+
it "is aliased as reports_error" do
88
expect {Rails.error.report(StandardError.new("test error"))}.to reports_error
99
end
1010

11-
it "warns that passing value expectation doesn't work" do
11+
it "warns when used as a value expectation" do
1212
expect {
1313
expect(Rails.error.report(StandardError.new("test error"))).to have_reported_error
1414
}.to raise_error(ArgumentError, "this matcher doesn’t work with value expectations")
1515
end
1616

17-
describe "basic functionality" do
17+
context "without constraint" do
1818
it "passes when an error is reported" do
1919
expect {Rails.error.report(StandardError.new("test error"))}.to have_reported_error
2020
end
2121

22-
it "fails when no error is reported" do
22+
it "fails when no errors are reported" do
2323
expect {
2424
expect { "no error" }.to have_reported_error
2525
}.to fail_with(/Expected the block to report an error, but none was reported./)
2626
end
2727

28-
it "passes when negated and no error is reported" do
28+
it "passes when negated and no errors are reported" do
2929
expect { "no error" }.not_to have_reported_error
3030
end
3131
end
3232

33-
describe "error class matching" do
34-
it "passes when correct error class is reported" do
33+
context "constrained to a specific error class" do
34+
it "passes when an error with the correct class is reported" do
3535
expect { Rails.error.report(TestError.new("test error")) }.to have_reported_error(TestError)
3636
end
3737

38-
it "fails when wrong error class is reported" do
38+
it "fails when an error with the wrong class is reported" do
3939
expect {
4040
expect {
4141
Rails.error.report(AnotherTestError.new("wrong error"))
@@ -44,20 +44,20 @@ class AnotherTestError < StandardError; end
4444
end
4545
end
4646

47-
describe "error instance matching" do
48-
it "passes when error instance matches exactly" do
47+
context "constrained to a matching error (class and message)" do
48+
it "passes with an error that matches exactly" do
4949
expect {
5050
Rails.error.report(TestError.new("exact message"))
5151
}.to have_reported_error(TestError.new("exact message"))
5252
end
5353

54-
it "passes when error instance has empty expected message" do
54+
it "passes any error of the same class if the expected message is empty" do
5555
expect {
5656
Rails.error.report(TestError.new("any message"))
5757
}.to have_reported_error(TestError.new(""))
5858
end
5959

60-
it "fails when error instance has different message" do
60+
it "fails when the error has different message to the expected" do
6161
expect {
6262
expect {
6363
Rails.error.report(TestError.new("actual message"))
@@ -66,14 +66,14 @@ class AnotherTestError < StandardError; end
6666
end
6767
end
6868

69-
describe "regex pattern matching" do
70-
it "passes when error message matches pattern" do
69+
context "constrained by regex pattern matching" do
70+
it "passes when an error message matches the pattern" do
7171
expect {
7272
Rails.error.report(StandardError.new("error with pattern"))
7373
}.to have_reported_error(/with pattern/)
7474
end
7575

76-
it "fails when error message does not match pattern" do
76+
it "fails when no error messages match the pattern" do
7777
expect {
7878
expect {
7979
Rails.error.report(StandardError.new("error without match"))
@@ -82,8 +82,8 @@ class AnotherTestError < StandardError; end
8282
end
8383
end
8484

85-
describe "failure messages for attribute mismatches" do
86-
it "provides detailed failure message when attributes don't match" do
85+
describe "#failure_message" do
86+
it "provides details about mismatched attributes" do
8787
expect {
8888
expect {
8989
Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "actual" })
@@ -116,7 +116,7 @@ class AnotherTestError < StandardError; end
116116
end
117117
end
118118

119-
describe "attribute matching with .with chain" do
119+
describe "#with" do
120120
it "passes when attributes match exactly" do
121121
expect {
122122
Rails.error.report(StandardError.new("test"), context: { user_id: 123, context: "test" })

0 commit comments

Comments
 (0)