TEST_CODEOWNERS/rails
3
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Improve error message when passing a proc to assert_difference

Browse Source 
Previously if `assert_difference` called with a proc fails, the inspect
output of the proc object was shown. This is not helpful to identify
what went wrong.

With this commit we leverage the experimental
`RubyVM::AbstractSyntaxTree` api of MRI to print the source code of the
proc that was passed to `assert_difference`. On all other platforms the
behavior stays the same.

The same applies to `assert_changes`.
This commit is contained in:
Richard Böhme 2024-06-06 22:09:28 +02:00 committed by Jean Boussier
parent a472403d55
commit 38e9695c10
3 changed files with 153 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
activesupport/CHANGELOG.md
View File

@ -1,3 +1,6 @@
* Improve error message when using `assert_difference` or `assert_changes` with a
proc by printing the proc's source code (MRI only).
*Richard Böhme*, *Jean Boussier*
Please check [7-2-stable](https://github.com/rails/rails/blob/7-2-stable/activesupport/CHANGELOG.md) for previous changes.

39
activesupport/lib/active_support/testing/assertions.rb
View File

@ -118,7 +118,8 @@ def assert_difference(expression, *args, &block)
expressions.zip(exps, before) do |(code, diff), exp, before_value|
actual = exp.call
error = "#{code.inspect} didn't change by #{diff}, but by #{actual - before_value}"
code_string = code.respond_to?(:call) ? _callable_to_source_string(code) : code
error = "`#{code_string}` didn't change by #{diff}, but by #{actual - before_value}"
error = "#{message}.\n#{error}" if message
assert_equal(before_value + diff, actual, error)
end
@ -202,7 +203,8 @@ def assert_changes(expression, message = nil, from: UNTRACKED, to: UNTRACKED, &b
after = exp.call
error = "#{expression.inspect} didn't change"
code_string = expression.respond_to?(:call) ? _callable_to_source_string(expression) : expression
error = "`#{code_string}` didn't change"
error = "#{error}. It was already #{to.inspect}" if before == to
error = "#{message}.\n#{error}" if message
refute_equal before, after, error
@ -249,7 +251,8 @@ def assert_no_changes(expression, message = nil, from: UNTRACKED, &block)
after = exp.call
error = "#{expression.inspect} changed"
code_string = expression.respond_to?(:call) ? _callable_to_source_string(expression) : expression
error = "`#{code_string}` changed"
error = "#{message}.\n#{error}" if message
if before.nil?
@ -276,6 +279,36 @@ def _assert_nothing_raised_or_warn(assertion, &block)
raise
end
def _callable_to_source_string(callable)
if defined?(RubyVM::AbstractSyntaxTree) && callable.is_a?(Proc)
ast = begin
RubyVM::AbstractSyntaxTree.of(callable, keep_script_lines: true)
rescue SystemCallError
# Failed to get the source somehow
return callable
end
return callable unless ast
source = ast.source
source.strip!
# We ignore procs defined with do/end as they are likely multi-line anyway.
if source.start_with?("{")
source.delete_suffix!("}")
source.delete_prefix!("{")
source.strip!
# It won't read nice if the callable contains multiple
# lines, and it should be a rare occurence anyway.
# Same if it takes arguments.
if !source.include?("\n") && !source.start_with?("|")
return source
end
end
end
callable
end
end
end
end

122
activesupport/test/test_case_test.rb
View File

@ -54,7 +54,7 @@ def test_assert_no_difference_fail
@object.increment
end
end
assert_equal "\"@object.num\" didn't change by 0, but by 1.\nExpected: 0\n Actual: 1", error.message
assert_equal "`@object.num` didn't change by 0, but by 1.\nExpected: 0\n Actual: 1", error.message
end
def test_assert_no_difference_with_message_fail
@ -63,7 +63,7 @@ def test_assert_no_difference_with_message_fail
@object.increment
end
end
assert_equal "Object Changed.\n\"@object.num\" didn't change by 0, but by 1.\nExpected: 0\n Actual: 1", error.message
assert_equal "Object Changed.\n`@object.num` didn't change by 0, but by 1.\nExpected: 0\n Actual: 1", error.message
end
def test_assert_no_difference_with_multiple_expressions_pass
@ -157,7 +157,7 @@ def test_hash_of_expressions_with_message
@object.increment
end
end
assert_equal "Object Changed.\n\"@object.num\" didn't change by 0, but by 1.\nExpected: 0\n Actual: 1", error.message
assert_equal "Object Changed.\n`@object.num` didn't change by 0, but by 1.\nExpected: 0\n Actual: 1", error.message
end
def test_assert_difference_message_includes_change
@ -167,7 +167,17 @@ def test_assert_difference_message_includes_change
@object.increment
end
end
assert_equal "\"@object.num\" didn't change by 5, but by 2.\nExpected: 5\n Actual: 2", error.message
assert_equal "`@object.num` didn't change by 5, but by 2.\nExpected: 5\n Actual: 2", error.message
end
def test_assert_difference_message_with_lambda
skip if !defined?(RubyVM::AbstractSyntaxTree)
error = assert_raises Minitest::Assertion do
assert_difference(-> { @object.num }, 1, "Object Changed") do
end
end
assert_equal "Object Changed.\n`@object.num` didn't change by 1, but by 0.\nExpected: 1\n Actual: 0", error.message
end
def test_hash_of_lambda_expressions
@ -233,7 +243,19 @@ def test_assert_changes_with_to_option_but_no_change_has_special_message
end
end
assert_equal "\"@object.num\" didn't change. It was already 0.\nExpected 0 to not be equal to 0.", error.message
assert_equal "`@object.num` didn't change. It was already 0.\nExpected 0 to not be equal to 0.", error.message
end
def test_assert_changes_message_with_lambda
skip if !defined?(RubyVM::AbstractSyntaxTree)
error = assert_raises Minitest::Assertion do
assert_changes -> { @object.num }, to: 0 do
# no changes
end
end
assert_equal "`@object.num` didn't change. It was already 0.\nExpected 0 to not be equal to 0.", error.message
end
def test_assert_changes_with_wrong_to_option
@ -349,7 +371,91 @@ def test_assert_no_changes_with_message
end
end
assert_equal "@object.num should not change.\n\"@object.num\" changed.\nExpected: 0\n Actual: 1", error.message
assert_equal "@object.num should not change.\n`@object.num` changed.\nExpected: 0\n Actual: 1", error.message
end
def test_assert_no_changes_message_with_lambda
skip if !defined?(RubyVM::AbstractSyntaxTree)
error = assert_raises Minitest::Assertion do
assert_no_changes -> { @object.num } do
@object.increment
end
end
assert_equal "`@object.num` changed.\nExpected: 0\n Actual: 1", error.message
check = Proc.new {
@object.num
}
error = assert_raises Minitest::Assertion do
assert_no_changes check do
@object.increment
end
end
assert_equal "`@object.num` changed.\nExpected: 1\n Actual: 2", error.message
check = lambda {
@object.num
}
error = assert_raises Minitest::Assertion do
assert_no_changes check do
@object.increment
end
end
assert_equal "`@object.num` changed.\nExpected: 2\n Actual: 3", error.message
error = assert_raises Minitest::Assertion do
assert_no_changes -> { @object.num } do
@object.increment
end
end
assert_equal "`@object.num` changed.\nExpected: 3\n Actual: 4", error.message
error = assert_raises Minitest::Assertion do
assert_no_changes ->(a = nil) { @object.num } do
@object.increment
end
end
assert_match(/#<Proc:0x.*changed/, error.message)
end
def test_assert_no_changes_message_with_multi_line_lambda
check = lambda {
"title".upcase
@object.num
}
error = assert_raises Minitest::Assertion do
assert_no_changes check do
@object.increment
end
end
assert_match(/#<Proc:0x.*changed/, error.message)
check = lambda {
"title".upcase
@object.num
}
error = assert_raises Minitest::Assertion do
assert_no_changes check do
@object.increment
end
end
assert_match(/#<Proc:0x.*changed/, error.message)
end
def test_assert_no_changes_message_with_not_real_callable
check = Object.new
def check.call
@object.num
end
check.instance_variable_set(:@object, @object)
error = assert_raises Minitest::Assertion do
assert_no_changes check do
@object.increment
end
end
assert_match(/#<Object:0x.*changed/, error.message)
end
def test_assert_no_changes_with_long_string_wont_output_everything
@ -362,7 +468,7 @@ def test_assert_no_changes_with_long_string_wont_output_everything
end
assert_match <<~output, error.message
"lines" changed.
`lines` changed.
--- expected
+++ actual
@@ -10,4 +10,5 @@
@ -403,7 +509,7 @@ def test_warning_is_not_logged_if_assertions_are_nested_correctly
run_test_that_should_fail_but_not_log_a_warning
end
assert_not @out.string.include?("assert_nothing_raised")
assert error.message.include?("(lambda)> changed")
assert error.message.include?("`rand` changed")
end
def test_fails_and_warning_is_logged_if_wrong_error_caught