ruby / error_highlight

The gem enhances Exception#message by adding a short explanation where the exception is raised
MIT License
150 stars 23 forks source link

ArgumentError#message changes since 0.5.0 #28

Closed yahonda closed 1 year ago

yahonda commented 1 year ago

Rails CI is getting failures since error_highlight 0.5.0. Refer to https://buildkite.com/rails/rails/builds/90833#0184571b-307d-4e94-8a2d-3c8a2669a12c/1052-1078

It is likely because error_highlight 0.5.0 takes care of ArgumentError via https://github.com/ruby/error_highlight/commit/defcaf1beb95ea9d233cced21863e88ccbe64203

I expect error_highlight does not change the original message.

Steps to reproduce

require 'minitest/autorun'
require 'error_highlight'

def foo(*args)
  raise ArgumentError, 'You have to supply some args' if args.empty?
  args
end

class BugTest < Minitest::Test
  def test_foo
    error = assert_raises(ArgumentError) do
      foo()
    end
    assert_equal 'You have to supply some args', error.message
  end
end

Expected behavior

$ ruby -v
ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [x86_64-linux]
$ gem install error_highlight -v 0.4.0
Fetching error_highlight-0.4.0.gem
Successfully installed error_highlight-0.4.0
Parsing documentation for error_highlight-0.4.0
Installing ri documentation for error_highlight-0.4.0
Done installing documentation for error_highlight after 0 seconds
1 gem installed
$ ruby foo.rb
Run options: --seed 46613

# Running:

.

Finished in 0.005816s, 171.9293 runs/s, 343.8587 assertions/s.
1 runs, 2 assertions, 0 failures, 0 errors, 0 skips
$

Actual behavior

$ ruby -v
ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [x86_64-linux]
$ gem install error_highlight -v 0.5.0
Fetching error_highlight-0.5.0.gem
Successfully installed error_highlight-0.5.0
Parsing documentation for error_highlight-0.5.0
Installing ri documentation for error_highlight-0.5.0
Done installing documentation for error_highlight after 0 seconds
1 gem installed
yahonda@myryzen:~$ ruby foo.rb
Run options: --seed 48895

# Running:

F

Failure:
BugTest#test_foo [foo.rb:14]:
--- expected
+++ actual
@@ -1 +1,4 @@
-"You have to supply some args"
+"You have to supply some args
+
+  raise ArgumentError, 'You have to supply some args' if args.empty?
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^"

rails test foo.rb:10

Finished in 0.008049s, 124.2380 runs/s, 248.4761 assertions/s.
1 runs, 2 assertions, 1 failures, 0 errors, 0 skips
$
rafaelfranca commented 1 year ago

I agree with @yahonda here. This gem should not change the message attribute of exceptions. Only how the message is printed.

mame commented 1 year ago

Sorry! https://github.com/ruby/error_highlight/pull/29 should solve this issue. I will cut error_highlight 0.5.1 soon

yahonda commented 1 year ago

Thanks for the fix. I have confirmed error_highlight 0.5.1 addresses this issue.

$ gem install error_highlight -v 0.5.1
Fetching error_highlight-0.5.1.gem
Successfully installed error_highlight-0.5.1
Parsing documentation for error_highlight-0.5.1
Installing ri documentation for error_highlight-0.5.1
Done installing documentation for error_highlight after 0 seconds
1 gem installed
$ ruby foo.rb
Run options: --seed 30281

# Running:

.

Finished in 0.005720s, 174.8218 runs/s, 349.6436 assertions/s.
1 runs, 2 assertions, 0 failures, 0 errors, 0 skips
$

$ cat foo.rb
require 'minitest/autorun'
require 'error_highlight'

def foo(*args)
  raise ArgumentError, 'You have to supply some args' if args.empty?
  args
end

class BugTest < Minitest::Test
  def test_foo
    error = assert_raises(ArgumentError) do
      foo()
    end
    assert_equal 'You have to supply some args', error.message
  end
end
$