Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add failing test for define_singleton_method #38

Merged
merged 1 commit into from
Jul 31, 2017

Conversation

jahfer
Copy link
Contributor

@jahfer jahfer commented Jul 31, 2017

It turns out when using dynamically defining methods via define_method and define_singleton_method, the call stack is inconsistent compared with how pre-defined Ruby methods look.


On :call events, the frame points to where the method is received in Ruby, i.e. the block passed to define_method or define_singleton_method. This is the expected behaviour for Ruby invocations:

[TracePoint] has the unfortunate side effect of pointing to the callee of the method (i.e. where the method is received), rather than the caller.

But, when the :return event fires, instead of pointing to the end of the method definition like a normal ruby method, it points one frame up the stack, to where the original caller occurred. Since we assume Ruby methods point one level deeper, Rotoscope pops the frame one level further, and points to where the caller's caller came from. This means the call and return events won't match up.


I tried to figure out how to get around this one (by using Ruby's bitmasks to determine if it was a dynamically defined method, etc.), but I've been unsuccessful thus far. As a compromise, I've added a failing test case with a skip, just to be able to replicate the problem.

This problem is currently avoidable via flatten: true, since unmatched returns are ignored.

  • bin/fmt was successfully run

@jahfer jahfer force-pushed the track-dynamic-methods branch 2 times, most recently from d00b3e8 to 6945bea Compare July 31, 2017 14:14
@@ -403,6 +410,35 @@ def test_dynamic_class_creation
], parse_and_normalize(contents)
end

def test_dynamic_methods_in_blacklist
skip <<-FAILING_TEST_CASE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test passes if I remove this skip

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, indeed it does, because the assertion I'm doing is not the actual result I want to see 😄 I'll push up a failing one so it's more clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -0,0 +1,16 @@
module Monadify

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌯

@jahfer jahfer merged commit e8a02c0 into class-creation-consistency Jul 31, 2017
@jahfer jahfer deleted the track-dynamic-methods branch July 31, 2017 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants