Skip to content

Commit 3a5f730

Browse files
committed
fix: make error messages consistent
Change 1: Error messages should consistently use the "expected ... but it ..." format. Reviewed all error messages in the project. Change 2: Changed all tests that have error message expectations from: ``` expect { subject }.to raise_error(ArgumentError, expected_message) ``` to: ``` expect { subject }.to raise_error(ArgumentError) do |error| expect(error.message).to match(expected_message) end ``` so that error messages are diff'd when they don't match.
1 parent 0a871ca commit 3a5f730

10 files changed

Lines changed: 578 additions & 259 deletions

File tree

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,8 @@ expect('config').to(
287287
```text
288288
'config' was not as expected:
289289
- database.xml
290-
expected owner to be "db_user", but was "root"
291-
expected mode to be "0600", but was "0644"
290+
expected owner to be "db_user", but it was "root"
291+
expected mode to be "0600", but it was "0644"
292292
```
293293

294294
## Development

lib/rspec/path_matchers/matchers/directory_matcher.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ def check_for_unexpected_entries
164164
end
165165

166166
def build_unexpected_entries_message(unexpected_entries)
167-
"contained unexpected entries #{unexpected_entries.sort.inspect}"
167+
"expected no other entries, but found #{unexpected_entries.sort.inspect}"
168168
end
169169
end
170170
end

lib/rspec/path_matchers/options/base.rb

Lines changed: 49 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ def self.validate_expected(expected, errors)
102102

103103
types = ['Matcher', *valid_expected_types.map(&:name)].to_sentence(conjunction: 'or')
104104

105-
errors << "expected `#{key}:` to be a #{types}, but was #{expected.inspect}"
105+
errors << "expected `#{key}:` to be a #{types}, but it was #{expected.inspect}"
106106
end
107107

108108
protected
@@ -189,6 +189,36 @@ def self.validate_expected(expected, errors)
189189
#
190190
private_class_method def self.literal_match?(actual, expected) = actual == expected
191191

192+
# Add to `failures` if actual value matches the normalized expected value
193+
#
194+
# This is called when expected is not an RSpec matcher.
195+
#
196+
# Option subclasses should override this method to provide custom matching
197+
# logic or custom failure messages.
198+
#
199+
# @param actual [Object] the actual value fetched from the file system
200+
#
201+
# @param expected [Object] the expected literal value to match against
202+
#
203+
# @param failures [Array<RSpec::PathMatchers::Failure>] the array to append
204+
# failure objects to (if any)
205+
#
206+
# @return [void]
207+
#
208+
# @api protected
209+
#
210+
private_class_method def self.match_literal(actual, expected, failures)
211+
expected = normalize_expected_literal(expected)
212+
213+
return if literal_match?(actual, expected)
214+
215+
add_failure(literal_failure_message(actual, expected), failures)
216+
end
217+
218+
private_class_method def self.add_failure(message, failures)
219+
failures << RSpec::PathMatchers::Failure.new('.', message)
220+
end
221+
192222
# Generates a failure message for a literal match failure
193223
#
194224
# This is used when the actual value does not match the expected value.
@@ -201,9 +231,9 @@ def self.validate_expected(expected, errors)
201231
# @example generate a failure message for a literal match failure
202232
# def self.literal_failure_message(actual, expected)
203233
# if expected.is_a?(Regexp)
204-
# "expected #{key} to match #{expected.inspect}, but was #{actual.inspect}"
234+
# "expected #{key} to match #{expected.inspect}, but it was #{actual.inspect}"
205235
# else
206-
# "expected #{key} to be #{expected.inspect}, but was #{actual.inspect}"
236+
# "expected #{key} to be #{expected.inspect}, but it was #{actual.inspect}"
207237
# end
208238
# end
209239
#
@@ -216,19 +246,19 @@ def self.validate_expected(expected, errors)
216246
# @api protected
217247
#
218248
private_class_method def self.literal_failure_message(actual, expected)
219-
"expected #{key} to be #{expected.inspect}, but was #{actual.inspect}"
249+
"expected #{key} to be #{expected.inspect}, but it was #{actual.inspect}"
220250
end
221251

222252
# Add to `failures` if actual value matches the normalized expected value
223253
#
224-
# This is called when expected is not an RSpec matcher.
254+
# This is called when expected is an RSpec matcher.
225255
#
226256
# Option subclasses should override this method to provide custom matching
227257
# logic or custom failure messages.
228258
#
229259
# @param actual [Object] the actual value fetched from the file system
230260
#
231-
# @param expected [Object] the expected literal value to match against
261+
# @param expected [RSpec::Matchers::Matcher] the expected matcher to match against
232262
#
233263
# @param failures [Array<RSpec::PathMatchers::Failure>] the array to append
234264
# failure objects to (if any)
@@ -237,42 +267,31 @@ def self.validate_expected(expected, errors)
237267
#
238268
# @api protected
239269
#
240-
private_class_method def self.match_literal(actual, expected, failures)
241-
expected = normalize_expected_literal(expected)
242-
243-
return if literal_match?(actual, expected)
244-
245-
message = literal_failure_message(actual, expected)
246-
add_failure(message, failures)
247-
end
270+
private_class_method def self.match_matcher(actual, expected, failures)
271+
return if expected.matches?(actual)
248272

249-
private_class_method def self.add_failure(message, failures)
250-
failures << RSpec::PathMatchers::Failure.new('.', message)
273+
add_failure(matcher_failure_message(actual, expected), failures)
251274
end
252275

253-
# Add to `failures` if actual value matches the normalized expected value
276+
# Generates a failure message for a matcher match failure
254277
#
255-
# This is called when expected is an RSpec matcher.
278+
# This is used when the actual value does not match the expected value.
279+
# It provides a clear message indicating what was expected and what was
280+
# actually found.
256281
#
257-
# Option subclasses should override this method to provide custom matching
258-
# logic or custom failure messages.
282+
# Option subclasses should override this method to provide custom failure
283+
# messages for specific types of options.
259284
#
260285
# @param actual [Object] the actual value fetched from the file system
261286
#
262-
# @param expected [RSpec::Matchers::Matcher] the expected matcher to match against
263-
#
264-
# @param failures [Array<RSpec::PathMatchers::Failure>] the array to append
265-
# failure objects to (if any)
287+
# @param expected [Object] the expected literal value to match against
266288
#
267-
# @return [void]
289+
# @return [String] the failure message
268290
#
269291
# @api protected
270292
#
271-
private_class_method def self.match_matcher(actual, expected, failures)
272-
return if expected.matches?(actual)
273-
274-
message = "expected #{key} to #{expected.description}, but was #{actual.inspect}"
275-
add_failure(message, failures)
293+
private_class_method def self.matcher_failure_message(actual, expected)
294+
"expected #{key} to #{expected.description}, but it was #{actual.inspect}"
276295
end
277296

278297
# Warning message for unsupported expectations

lib/rspec/path_matchers/options/content.rb

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,28 @@ def self.key = :content
1111
def self.fetch_actual(path, _failures) = File.read(path)
1212
def self.valid_expected_types = [String, Regexp]
1313

14+
# Override to provide custom matching logic for regexp literals
1415
def self.literal_match?(actual, expected)
15-
return expected.match?(actual) if expected.is_a?(Regexp)
16+
expected.is_a?(Regexp) ? expected.match?(actual) : super
17+
end
1618

17-
super
19+
# Handles failures when a matcher is used (e.g., content: include('...'))
20+
def self.matcher_failure_message(actual, expected)
21+
actual_summary = actual.length > 100 ? 'did not' : "was #{actual.inspect}"
22+
"expected content to #{expected.description}, but it #{actual_summary}"
1823
end
1924

2025
def self.literal_failure_message(actual, expected)
21-
if expected.is_a?(Regexp)
22-
"expected content to match #{expected.inspect}, but got #{actual.inspect}"
23-
else
24-
super
25-
end
26+
verb = expected.is_a?(Regexp) ? 'match' : 'be'
27+
28+
actual_summary =
29+
if actual.length > 100
30+
verb == 'match' ? 'did not' : 'was not'
31+
else
32+
"was #{actual.inspect}"
33+
end
34+
35+
"expected content to #{verb} #{expected.inspect}, but it #{actual_summary}"
2636
end
2737
end
2838
end

lib/rspec/path_matchers/options/parsed_content_base.rb

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,16 @@ def self.fetch_actual(path, failures)
2929
# This is the `xxxx_content: true` case. A successful fetch_actual is sufficient
3030
def self.match_literal(_actual, _expected, _failures); end
3131

32-
# Compares the parsed content against the given RSpec matcher.
32+
# Failure message for when a matcher is used (e.g., `json_content: include(...)`)
3333
#
3434
# @param (see RSpec::PathMatchers::Options::Base.match_matcher)
3535
#
3636
# @return [void]
3737
#
38-
def self.match_matcher(actual, expected, failures)
39-
return if expected.matches?(actual)
38+
def self.matcher_failure_message(actual, expected)
39+
actual_summary = actual.inspect.length > 100 ? 'it did not' : "was #{actual.inspect}"
4040

41-
message = "expected #{content_type} content to #{expected.description}"
42-
add_failure(message, failures)
41+
"expected #{content_type} content to #{expected.description}, but #{actual_summary}"
4342
end
4443

4544
# Provides a human-readable description for the option

spec/rspec/path_matchers/be_dir_exact_spec.rb

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@
119119
expected_message = <<~MESSAGE.chomp
120120
#{tmpdir} was not as expected:
121121
- #{entry_name}
122-
contained unexpected entries ["file1.txt"]
122+
expected no other entries, but found ["file1.txt"]
123123
MESSAGE
124124

125125
expect { subject }.to raise_error(expectation_not_met_error) do |error|
@@ -195,7 +195,9 @@
195195
it 'should fail with the `no_file` failure message' do
196196
# This ensures the `no_file` check runs and its failure message is prioritized.
197197
expected_message = /expected file 'file2.txt' not to be found at '.*', but it exists/
198-
expect { subject }.to raise_error(expectation_not_met_error, expected_message)
198+
expect { subject }.to raise_error(expectation_not_met_error) do |error|
199+
expect(error.message).to match(expected_message)
200+
end
199201
end
200202
end
201203

@@ -266,7 +268,7 @@
266268
expected_message = <<~MESSAGE.chomp
267269
#{tmpdir} was not as expected:
268270
- dir/nested_dir
269-
contained unexpected entries ["unexpected_file1.txt", "unexpected_file2.txt"]
271+
expected no other entries, but found ["unexpected_file1.txt", "unexpected_file2.txt"]
270272
MESSAGE
271273
expect { subject }.to raise_error(expectation_not_met_error) do |error|
272274
expect(error.message).to eq(expected_message)

0 commit comments

Comments
 (0)