rotoscope
rotoscope copied to clipboard
Fix tests on Ruby 3
See CI test failures (https://github.com/Shopify/rotoscope/runs/3954967334?check_suite_focus=true) from draft PR https://github.com/Shopify/rotoscope/pull/86 changing CI to test on ruby 3
It looks like the primary difference is that ruby 3 is actually exposing the C call frames in rb_profile_frames now. E.g. we are now seeing that calls to initialize
are coming from new
instead of the code that calls new
. That is why you see differences like this in the test failures
- :caller_method_name=>"test_start_trace_and_stop_trace",
+ :caller_method_name=>"new",
These C frames are missing a file path. Since the normalization in the tests tries to expand the path as part of turning it into a relative path, it ends up expanding the empty path to a path to the current directory, resulting in differences like
- :filepath=>"/rotoscope_test.rb",
+ :filepath=>"/home/runner/work/rotoscope/rotoscope",
I also noticed that the calls to initialize seem to be coming from a "new" with class_method_level "instance", which seemed strange. The full_label
on that profile frame shows the call is actually from Class#new
, which makes more sense, but it doesn't make sense in the context of the receiver class being a concrete class. For example, we would expect a call to Example#initialize
to come from the class method Example.new
, not the non-existent instance method Example#new
.
We handle C frames different than Ruby ones (link). If Ruby 3 is now including the C frame in #rb_profile_frames
, our logic makes sense to be broken, and our frame_index
is probably looking one level too high in the stack. That might explain Class#new
instead of Example.new
, though I haven't looked closely at the data beyond what you've mentioned here.
There is still a difference in that the C call has the caller at the top of the stack when the event hook is called. Perhaps that difference can be removed upstream, but it probably got introduced because it didn't matter in the context of rb_profile_frames
primary purpose of supporting sample based profiling.
Class#new is the defined method that gets called when it doesn't get overridden, so it is right from that perspective. It just is just wrong from the perspective of knowing that method is called on the receiver class (e.g. Example)
Class#new is the defined method that gets called when it doesn't get overridden, so it is right from that perspective. It just is just wrong from the perspective of knowing that method is called on the receiver class (e.g. Example)
This is also related to the fact that we were pulling together information from rb_profile_frames
and rb_tracearg_*
from the parent call for the caller, since the former didn't expose the receiver or class.
For example, when I run the script
require 'rotoscope'
class Example
end
def format_call(class_name, is_singleton_method, method_name)
class_name ||= "<UNKNOWN>"
method_name ||= "<UNKNOWN>"
singleton_indicator = case is_singleton_method
when nil then "?"
when false then "#"
when true then "."
else
raise
end
"#{class_name}#{singleton_indicator}#{method_name}"
end
rs = Rotoscope.new do |call|
caller_method = format_call(call.caller_class_name, call.caller_singleton_method?, call.caller_method_name)
receiver_method = format_call(call.receiver_class_name, call.singleton_method?, call.method_name)
puts "#{caller_method} called #{receiver_method}"
end
rs.trace do
Example.new
end
it outputs
<UNKNOWN>#<UNKNOWN> called Example.new
Example#new called Example#initialize
where you can see the inconsistency between it logging the Example.new class method being called, but logging it as an instance method when it is the caller.