"undefined method on nilClass" rather than "for nilClass"

[steveklabnik]

Working on getting the rails tests to pass, and found this:

  5) Failure:
RenderStreaming::StreamingTest#test_rendering_with_template_exception_logs_the_exception [/home/steveklabnik/src/rails/actionpack/test/controller/new_base/render_streaming_test.rb:93]:
Expected /\(undefined\ method\ `invalid!'\ for\ nil:NilClass\)/ to match "\nActionView::Template::Error (undefined method `invalid!' on nil:NilClass.):\n    

not the 'on' rather than the expected 'for'.

I did a bit of grep-ping, but couldn't figure out where this error would be defined in the rbx source. Any idea what's causing this issue?

[dbussink]

Actually the exact wording of exception messages is not something that is defined and should be relied on. Of course we can discuss which wording is better, but in general tests should not depend on such exact wording

Perhaps it's better to change the test to an exemption that is thrown explicitly with a message that the test itself defines?

[kachick]

@steveklabnik be defined looks here:

rubinius/kernel/delta/kernel.rb

Line 74 in 7058a0c

msg << " on nil:NilClass."

[brixen]

RubySpec does not specify the text of exceptions because they are for human consumption and MRI often has poorly worded exceptions. Even if MRI's exception messages were stellar, expecting that exceptions are in eg English eliminates the possibility of localization.

The class of the exception is what has code semantics. RubySpec does specify the expected exception class in a multitude of specs.

Applications and frameworks like Rails have understandably been written from an MRI-centric perspective, but there are presently Rubinius, JRuby, MagLev, IronRuby, Topaz, Opal as significant Ruby implementations. The sooner we address improper reliance on MRI-specific aspects, the better.

[razielgn]

Just for the record, I've found many gems in the past month which have some functionality depending exclusively upon MRI's specific printing of backtraces, with a few offering compatibility with JRuby in clunky and awkward ways.

[jc00ke]

@razielgn I'm curious, which ones?

[brixen]

@razielgn let's start compiling a list. We can send PRs to fix them. That's an easy thing to ask people to do to help out.

[razielgn]

@brixen absolutely, it was on my todo list :)
Thor: https://github.com/erikhuda/thor/blob/master/lib/thor/command.rb#L93
Naught: https://github.com/avdi/naught I have to look up where exactly are the critical points.
edit: Might be this one https://github.com/avdi/naught/blob/master/lib/naught/null_class_builder.rb#L121
Rake: https://github.com/jimweirich/rake Again, I have to look up where exactly are the critical points.

[steveklabnik]

Yup, would love some advice for how to fix this test The Right Way.

[brixen]

The right way is for exception instances to have more information in the form of attributes rather than just a message. That requires fixing Ruby.

In this case, looking just at the text you pasted in the original message of this ticket, it looks like something is rescuing an exception and re-raising an ActionView::Template::Error using the original exception's message. Since the code isn't using its own message, the test shouldn't make assertions about the content of that message.

The code could use it's own exception message. In that case, spec'ing the message would be reasonable. Of course, the argument against this is that ActionView::Template::Error doesn't really know anything about what's happening. And we're back to having exceptions like NoMethodError provide more information in the form of proper OO attributes.