Skip to content

Conversation

@jeremyevans
Copy link

This fixes an issue in Sequel's tests on ruby 3.1.0-preview1, where a
test was showing the wrong output in an exception message. The
related code rescues NameError to provide more information, then
reraises. Here's the output of Sequel's test without changes to
error_highlight:

 1) Failure: Sequel::Model::Associations::AssociationReflection::#associated_class#test_0006_should include association inspect output if an exception would be raised [/data/code/sequel/spec/model/associ ation_reflection_spec.rb:75]: Expected "undefined method `last_lineno' for nil:NilClass\n\n if nd_parent.last_lineno == @node.last_lineno\n ^^^^^^^^^^^^" to include # encoding: UTF-8 "#<Sequel::Model::Associations::ManyToOneAssociationReflection #<Class:0x000002594112e458>.many_to_one :c>". 

This change makes the Sequel test pass. There probably is a better
fix.

I tried to reproduce this outside of Sequel's tests in order to create
a test for error_highlight, but I wasn't able to do so quickly.

This fixes an issue in Sequel's tests on ruby 3.1.0-preview1, where a test was showing the wrong output in an exception message. The related code rescues NameError to provide more information, then reraises. Here's the output of Sequel's test without changes to error_highlight: ``` 1) Failure: Sequel::Model::Associations::AssociationReflection::#associated_class#test_0006_should include association inspect output if an exception would be raised [/data/code/sequel/spec/model/associ ation_reflection_spec.rb:75]: Expected "undefined method `last_lineno' for nil:NilClass\n\n if nd_parent.last_lineno == @node.last_lineno\n ^^^^^^^^^^^^" to include # encoding: UTF-8 "#<Sequel::Model::Associations::ManyToOneAssociationReflection #<Class:0x000002594112e458>.many_to_one :c>". ``` This change makes the Sequel test pass. There probably is a better fix. I tried to reproduce this outside of Sequel's tests in order to create a test for error_highlight, but I wasn't able to do so quickly.
@jeremyevans jeremyevans requested a review from mame November 16, 2021 22:07
@mame
Copy link
Member

mame commented Dec 18, 2021

Thank you for your report. I think I got a clue. The following code will trigger an unknown bug of error_highlight (or RubyVM::AST or something).

module Foo Object.module_eval("::Bar", __FILE__, __LINE__) end 

The error message changes if error_highlight is enabled:

$ ruby --disable-error_highlight t.rb t.rb:2:in `<module:Foo>': uninitialized constant Bar (NameError) from t.rb:2:in `module_eval' from t.rb:2:in `<module:Foo>' from t.rb:1:in `<main>' $ ruby t.rb t.rb:2:in `<module:Foo>': NameError from t.rb:2:in `module_eval' from t.rb:2:in `<module:Foo>' from t.rb:1:in `<main>' 

I'll continue to investigate the issue tonight

matzbot pushed a commit to ruby/ruby that referenced this pull request Dec 18, 2021
This check is needed to fix a bug of error_highlight when NameError occurred in eval'ed code. ruby/error_highlight#16 The same check for proc/method has been already introduced since 64ac984.
@mame mame closed this in d2140d7 Dec 18, 2021
@mame
Copy link
Member

mame commented Dec 18, 2021

@jeremyevans I think the issue was fixed by the combination of d2140d7 and ruby/ruby@6a51c3e. Could you try ruby/ruby master with your code base? Let me know if it is not fixed yet.

@mame
Copy link
Member

mame commented Dec 18, 2021

Ah, no, the issue is not fixed yet. Continue to investigate...

@mame
Copy link
Member

mame commented Dec 18, 2021

Argh, a pilot error. I used the older ruby to check if the issue is fixed or not. I think it's fixed after all.

@jeremyevans
Copy link
Author

I've built and installed ruby-head locally and checked and confirmed the problem is fixed. Thank you very much for fixing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants