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

Fix usage of tracer gem and add tests #857

Merged
merged 1 commit into from
Feb 6, 2024
Merged

Conversation

nunosilva800
Copy link
Contributor

@nunosilva800 nunosilva800 commented Feb 2, 2024

Fixes support of option --tracer when gem tracer >= 0.2.0 is installed.

The API for tracer changed a lot from 0.1.x to 0.2.x. For reference, here is the old API: https://github.com/ruby/tracer/blob/v0.1.1/lib/tracer.rb

This PR also adds some tests for this integration:

  • WITH_TRACER=true bundle exec rake test
    The new tests are executed. There is one test that verifies behaviour when --tracer is set, but gem is not bundled:
    Skipping test_use_tracer_when_gem_is_unavailable because 'tracer' gem is available.

  • WITH_TRACER=false bundle exec rake test
    The new tests will be skipped with message Skipping ContextWithTracerTest because 'tracer' gem is not available. Enable with WITH_TRACER=true.

Closes #752
Closes #694

Comment on lines +6 to +7
# Loading the gem "tracer" will cause it to extend IRB commands with:
# https://github.com/ruby/tracer/blob/v0.2.2/lib/tracer/irb.rb
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It took me some time to figure this out. Is this helpful?

Copy link
Member

Choose a reason for hiding this comment

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

I added that because IRB currently doesn't have integration with it. Do you think dropping it will make things easier here?

Comment on lines -25 to -30
Tracer.verbose = false
Tracer.add_filter {
|event, file, line, id, binding, *rests|
/^#{Regexp.quote(@CONF[:IRB_LIB_PATH])}/ !~ file and
File::basename(file) != "irb.rb"
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tracer.verbose and Tracer.add_filter were available in v0.1.x and I could not find an equivalent in the 0.2.x API.

Comment on lines -45 to -52
if opt
Tracer.set_get_line_procs(@irb_path) {
|line_no, *rests|
@io.line(line_no)
}
elsif !opt && @use_tracer
Tracer.off
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here, set_get_line_procs, off / on are all method of the v0.1.x API, without an equivalent in v0.2.x

lib/irb/ext/tracer.rb Outdated Show resolved Hide resolved
Gemfile Show resolved Hide resolved
@nunosilva800
Copy link
Contributor Author

Rebased.

@nunosilva800
Copy link
Contributor Author

Hmm, just before the rebase we were all green: https://github.com/ruby/irb/actions/runs/7758254084

Is there a way to retry? The current error does not seem related.

lib/irb/ext/tracer.rb Outdated Show resolved Hide resolved
lib/irb/ext/tracer.rb Outdated Show resolved Hide resolved
lib/irb/ext/tracer.rb Outdated Show resolved Hide resolved
lib/irb/ext/tracer.rb Outdated Show resolved Hide resolved
lib/irb/ext/tracer.rb Outdated Show resolved Hide resolved
test/irb/test_tracer.rb Outdated Show resolved Hide resolved
test/irb/test_tracer.rb Show resolved Hide resolved
test/irb/test_tracer.rb Outdated Show resolved Hide resolved
test/irb/test_tracer.rb Show resolved Hide resolved
test/irb/test_tracer.rb Outdated Show resolved Hide resolved
The new tests are skipped when ruby below 3.1, as it was a default gem on it, and in a version we do not support.

This also move definition of `use_tracer` to module Context instead of monkey patch.
@nunosilva800
Copy link
Contributor Author

Updated with feedback and rebased.

@adam12
Copy link
Contributor

adam12 commented Feb 6, 2024

I've tested this PR and it works great, thanks!

One observation, and I am not sure if it's even possible, would be to strip off the calls from IRB::WorkSpace#evaluate and Kernel#eval.

irb(main):001> EID.generate_bytes
#depth:16>                  IRB::WorkSpace#evaluate at /Users/adam/code/github.com/ruby/irb/lib/irb/ext/tracer.rb:25:in `block in evaluate'
#depth:17>                   Kernel#eval at /Users/adam/code/github.com/ruby/irb/lib/irb/ext/tracer.rb:25:in `block in evaluate'
#depth:19>                     EID.generate_bytes at (irb):1:in `<top (required)>'

@nunosilva800
Copy link
Contributor Author

One observation, and I am not sure if it's even possible, would be to strip off the calls from IRB::WorkSpace#evaluate and Kernel#eval.

The older version of Tracer supported add_filter , which we could passRegexp.quote(@CONF[:IRB_LIB_PATH]) to exclude these from output.

The current version of tracer only supports an inclusive pattern as far as I can tell: https://github.com/ruby/tracer/blob/dc71573705638463ea2c50f5e9ff10dd275dc193/lib/tracer/base.rb#L78

@st0012 st0012 added the bug Something isn't working label Feb 6, 2024
Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!
I think some polishing can still be made, but I'll that in a follow up PR 👍

@st0012 st0012 merged commit 08834fb into ruby:master Feb 6, 2024
43 checks passed
matzbot pushed a commit to ruby/ruby that referenced this pull request Feb 6, 2024
(ruby/irb#857)

The new tests are skipped when ruby below 3.1, as it was a default gem on it, and in a version we do not support.

This also move definition of `use_tracer` to module Context instead of monkey patch.

ruby/irb@08834fbd5f
@nunosilva800 nunosilva800 deleted the 694-tracer branch February 6, 2024 17:09
@st0012
Copy link
Member

st0012 commented Feb 6, 2024

@adam12 I filtered those traces out in #864

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
3 participants