Skip to content

Commit d298e7f

Browse files
author
Stanislav (Stas) Katkov
committed
handle hegated matchers safely, not allowing any attributes
1 parent f01a731 commit d298e7f

File tree

2 files changed

+68
-13
lines changed

2 files changed

+68
-13
lines changed

lib/rspec/rails/matchers/have_reported_error.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,8 @@ def failure_message
124124
end
125125

126126
def failure_message_when_negated
127+
warn_about_negated_with_qualifiers! if has_qualifiers?
128+
127129
error_count = @reports.count
128130
error_word = 'error'.pluralize(error_count)
129131
verb = error_count == 1 ? 'has' : 'have'
@@ -214,6 +216,19 @@ def warn_about_nil_error!
214216
"`RSpec::Expectations.configuration.on_potential_false" \
215217
"_positives = :nothing`")
216218
end
219+
220+
def warn_about_negated_with_qualifiers!
221+
RSpec.warn_with("Using `expect { }.not_to have_reported_error(error_class_or_message)` " \
222+
"or with `.with_context()` is prone to false positives, since any error " \
223+
"that doesn't match the specific error class, message, or context can " \
224+
"cause the expectation to pass. Instead consider using " \
225+
"`expect { }.not_to have_reported_error` with no qualifiers to ensure " \
226+
"that no errors are reported at all.")
227+
end
228+
229+
def has_qualifiers?
230+
!@expected_error.nil? || !@expected_message.nil? || !@attributes.empty?
231+
end
217232
end
218233

219234
# @api public

spec/rspec/rails/matchers/have_reported_error_spec.rb

Lines changed: 53 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,6 @@ class AnotherTestError < StandardError; end
3535
expect { "no error" }.to have_reported_error
3636
}.to fail_with(/Expected the block to report an error, but none was reported./)
3737
end
38-
39-
it "passes when negated and no errors are reported" do
40-
expect { "no error" }.not_to have_reported_error
41-
end
4238
end
4339

4440
context "constrained to a specific error class" do
@@ -209,20 +205,64 @@ class AnotherTestError < StandardError; end
209205
end
210206
end
211207

212-
describe "integration with actual usage patterns" do
213-
it "works with multiple error reports in a block" do
208+
it "works with multiple error reports in a block" do
209+
expect {
210+
Rails.error.report(StandardError.new("first error"))
211+
Rails.error.report(TestError.new("second error"))
212+
}.to have_reported_error(StandardError)
213+
end
214+
215+
it "works with matcher chaining" do
216+
expect {
217+
expect {
218+
Rails.error.report(TestError.new("test"))
219+
}.to have_reported_error(TestError).and have_reported_error
220+
}.to raise_error(ArgumentError, "Chaining is not supported")
221+
end
222+
223+
describe "negation with qualifiers warnings" do
224+
it "warns when negated with error class qualifier fails" do
214225
expect {
215-
Rails.error.report(StandardError.new("first error"))
216-
Rails.error.report(TestError.new("second error"))
217-
}.to have_reported_error(StandardError)
226+
expect {
227+
Rails.error.report(TestError.new("test"))
228+
}.not_to have_reported_error(TestError)
229+
}.to raise_error(RuntimeError, /prone to false positives/)
218230
end
219231

220-
it "works with matcher chaining" do
232+
it "warns when negated with message qualifier fails" do
221233
expect {
222234
expect {
223-
Rails.error.report(TestError.new("test"))
224-
}.to have_reported_error(TestError).and have_reported_error
225-
}.to raise_error(ArgumentError, "Chaining is not supported")
235+
Rails.error.report(StandardError.new("specific message"))
236+
}.not_to have_reported_error("specific message")
237+
}.to raise_error(RuntimeError, /prone to false positives/)
238+
end
239+
240+
it "warns when negated with regex message qualifier fails" do
241+
expect {
242+
expect {
243+
Rails.error.report(StandardError.new("test pattern"))
244+
}.not_to have_reported_error(/pattern/)
245+
}.to raise_error(RuntimeError, /prone to false positives/)
246+
end
247+
248+
it "warns when negated with context qualifier fails" do
249+
expect {
250+
expect {
251+
Rails.error.report(StandardError.new("test"), context: { user_id: 123 })
252+
}.not_to have_reported_error.with_context(user_id: 123)
253+
}.to raise_error(RuntimeError, /prone to false positives/)
254+
end
255+
256+
it "warns when negated with both error class and message qualifiers fails" do
257+
expect {
258+
expect {
259+
Rails.error.report(TestError.new("message"))
260+
}.not_to have_reported_error(TestError, "message")
261+
}.to raise_error(RuntimeError, /prone to false positives/)
262+
end
263+
264+
it "does not warn when negated without any qualifiers" do
265+
expect { "no error" }.not_to have_reported_error
226266
end
227267
end
228268
end

0 commit comments

Comments
 (0)