Skip to content

Test against ruby-head #9

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Test against ruby-head #9

wants to merge 1 commit into from

Conversation

casperisfine
Copy link
Contributor

      STACK_FRAMES_BUFFER.find do |frame|
        path_match = frame.path.match(path_regexp)
      end

This crash on ruby-head with NoMethodError: undefined method match' for nil:NilClass`.

Trying to see if there's something wrong in this gem or if it's normal.

@casperisfine
Copy link
Contributor Author

Ok, I think it's simply https://bugs.ruby-lang.org/issues/17018 ruby/ruby#3299

It now return cfunc frame which don't have a .path. Not sure if this gem should filter them or not, but regardless, It's easy to work around.

@dylanahsmith
Copy link
Contributor

This library allows the user of it to specify the size of the buffer to capture the frames, so it seems like a leaky abstraction to try to hide C frames. That could confuse the code using it into behaving as it had captured the entire stack because less frames are exposed than the buffer can capture.

Ideally, we would skip the StackFrames::Buffer#capture method, but I think that is blocked on https://bugs.ruby-lang.org/issues/14607 to do that without the above mentioned leaky abstraction.

I'll open a PR to fix the tests on ruby 3.

@@ -5,4 +5,5 @@ cache: bundler
rvm:
- 2.4.9 # minimum supported
- 2.7.0 # latest released
before_install: gem install bundler -v 1.17.3
- ruby-head
Copy link
Contributor

Choose a reason for hiding this comment

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

travis-ci.org no longer works, so I'll switch to Github Actions for CI

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.

3 participants