JRuby

test/ruby/test_settracefunc failures

Details

  • Type: Sub-task Sub-task
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Won't Fix
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Number of attachments :
    5

Description

We have other bugs for trace problems, but I am recording this one here since it's a subtask of the "MRI tests" task.

  1) Failure:
test_event(TestSetTraceFunc) [test_settracefunc.rb:43]:
<["call", 4, :foo, #<Class:0x4bb523>]> expected but was
<["call", 20, :foo, #<Class:0x4bb523>]>.
  1. diff5
    22/May/07 6:59 PM
    7 kB
    Marcin Mielzynski
  2. method_equality_test.rb
    04/Oct/07 2:07 PM
    3 kB
    Robin Salkeld
  3. set_trace_func_fixes_3.patch
    22/May/07 10:40 PM
    32 kB
    Charles Oliver Nutter
  4. set_trace_func_fixes.patch
    21/May/07 4:57 PM
    5 kB
    Charles Oliver Nutter
  5. set_trace_func_fixes2.patch
    22/May/07 6:59 PM
    27 kB
    Marcin Mielzynski

Issue Links

Activity

Hide
Charles Oliver Nutter added a comment -

Here's a few fixes to get us farther in this file. See also JRUBY-971, which is the cause of the failure it gets to now (basically, the default === impl needs to always invoke ==, and Fixnum needs to map to the right methods).

Show
Charles Oliver Nutter added a comment - Here's a few fixes to get us farther in this file. See also JRUBY-971, which is the cause of the failure it gets to now (basically, the default === impl needs to always invoke ==, and Fixnum needs to map to the right methods).
Hide
Marcin Mielzynski added a comment -

with the patch combined and eql/==/=== fixes half of the test_settracefunc.rb tests pass:

1) Failure:
test_event(TestSetTraceFunc) [test_settracefunc.rb:89]:
<["c-call", 32, :new, Class]> expected but was
<["c-call", 32, :initialize, Exception]>.

the new patch doesn't contain other equality fixes yet:

missing Method: Proc#==
missing Method: Method#==
missing Method: UnboundMethod#==
extra Method: Time#===

maybe the failed test relies on Method comparison (but guessing now)

attached new patch (not final one) and current JRuby/MRI api diff

there's more work left for tomorrow though, any further thoughts ?.

Show
Marcin Mielzynski added a comment - with the patch combined and eql/==/=== fixes half of the test_settracefunc.rb tests pass: 1) Failure: test_event(TestSetTraceFunc) [test_settracefunc.rb:89]: <["c-call", 32, :new, Class]> expected but was <["c-call", 32, :initialize, Exception]>. the new patch doesn't contain other equality fixes yet: missing Method: Proc#== missing Method: Method#== missing Method: UnboundMethod#== extra Method: Time#=== maybe the failed test relies on Method comparison (but guessing now) attached new patch (not final one) and current JRuby/MRI api diff there's more work left for tomorrow though, any further thoughts ?.
Hide
Charles Oliver Nutter added a comment -

Additional fixes on top of my earlier ones and Marcin's, that allows test_settracefunc to run to completion. However the changes are a big questionable, and they prevent ant test from running successfully. The additional changes were:

  • RubyKernel#eval needs to pass "0" as the starting default line number to eval, not 1
  • RubyKernel#raise appears to be responsible for a call to backtrace and another to set_backtrace when constructing a simple RuntimeError. This could indicate that we're missing some call steps in the exception construction sequence that Ruby has.
  • RaiseException currently does a "return" trace on the current frame's information which should actually be a "raise" trace on the previous frame's.
  • interpreted methods can result in only their return event firing if a trace is set while they're running and still exists when they return. Previously in DefaultMethod the return event would only fire if the call event fired previously.
  • Exceptions should be constructed by actually invoking "new" on the appropriate class, not by just constructing a RubyException manually. Ruby expects "new" to be called along with "initialize".

Since the patch breaks ant test, it still needs work. All these changes except the stuff inside RubyKernel#raise seem mostly reasonable...though they have some small potential to add a bit of overhead. The backtrace/set_backtrace stuff needs a better explanation...my hacked way of making it pass is not sufficient.

Show
Charles Oliver Nutter added a comment - Additional fixes on top of my earlier ones and Marcin's, that allows test_settracefunc to run to completion. However the changes are a big questionable, and they prevent ant test from running successfully. The additional changes were:
  • RubyKernel#eval needs to pass "0" as the starting default line number to eval, not 1
  • RubyKernel#raise appears to be responsible for a call to backtrace and another to set_backtrace when constructing a simple RuntimeError. This could indicate that we're missing some call steps in the exception construction sequence that Ruby has.
  • RaiseException currently does a "return" trace on the current frame's information which should actually be a "raise" trace on the previous frame's.
  • interpreted methods can result in only their return event firing if a trace is set while they're running and still exists when they return. Previously in DefaultMethod the return event would only fire if the call event fired previously.
  • Exceptions should be constructed by actually invoking "new" on the appropriate class, not by just constructing a RubyException manually. Ruby expects "new" to be called along with "initialize".
Since the patch breaks ant test, it still needs work. All these changes except the stuff inside RubyKernel#raise seem mostly reasonable...though they have some small potential to add a bit of overhead. The backtrace/set_backtrace stuff needs a better explanation...my hacked way of making it pass is not sufficient.
Hide
Charles Oliver Nutter added a comment -

This has been reported again and should be fixed for 1.1. More information is coming.

Show
Charles Oliver Nutter added a comment - This has been reported again and should be fixed for 1.1. More information is coming.
Hide
Charles Oliver Nutter added a comment -

Er, sorry, to be more specific, the missing == methods above must be fixed.

Show
Charles Oliver Nutter added a comment - Er, sorry, to be more specific, the missing == methods above must be fixed.
Hide
Robin Salkeld added a comment -

Tests asserting the correct behaviour of Method#==, UnboundMethod#==, and Proc#==.

Note three tests are marked as failing under Ruby 1.8.5, in spite of documentation stating that they should pass.

Show
Robin Salkeld added a comment - Tests asserting the correct behaviour of Method#==, UnboundMethod#==, and Proc#==. Note three tests are marked as failing under Ruby 1.8.5, in spite of documentation stating that they should pass.
Hide
Robin Salkeld added a comment -

Whoops, didn't realize my file description would be attached as a comment.

I've added method_equality_test.rb to define the expected behaviour of the missing == methods. I hit the issue when switching a Rails project from Ruby 1.8.5 to JRuby 1.0.1. There is no way to implement these in user code as the internals of all three classes are hidden in both implementations.

Also, I found it frustrating in Ruby that == is defined on these three classes in terms of having the same body (which is preserved under method aliasing), and yet eql? and hash are not defined in a similar way so none of them can be used effectively as Hash keys, which is my target usage.

I am currently working around this in Ruby by defining UnboundMethod#=hash to be a fixed number, which works but means my Hashes degenerate to associative lists. I will log this as a feature request on the appropriate Ruby issue tracker, but I just wanted to mention it here for interest's sake.

Show
Robin Salkeld added a comment - Whoops, didn't realize my file description would be attached as a comment. I've added method_equality_test.rb to define the expected behaviour of the missing == methods. I hit the issue when switching a Rails project from Ruby 1.8.5 to JRuby 1.0.1. There is no way to implement these in user code as the internals of all three classes are hidden in both implementations. Also, I found it frustrating in Ruby that == is defined on these three classes in terms of having the same body (which is preserved under method aliasing), and yet eql? and hash are not defined in a similar way so none of them can be used effectively as Hash keys, which is my target usage. I am currently working around this in Ruby by defining UnboundMethod#=hash to be a fixed number, which works but means my Hashes degenerate to associative lists. I will log this as a feature request on the appropriate Ruby issue tracker, but I just wanted to mention it here for interest's sake.
Hide
Charles Oliver Nutter added a comment -

Invalid, these tests no longer exist and have been replaced by others.

Show
Charles Oliver Nutter added a comment - Invalid, these tests no longer exist and have been replaced by others.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: