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 JRuby compatibility #36

Closed
wants to merge 1 commit into from
Closed

Fix JRuby compatibility #36

wants to merge 1 commit into from

Conversation

c-lliope
Copy link

See the comments on #27

  • Ran into prolems identifying the line and file of instantiation
    for traceable null objects because of slight differences in
    MRI's and JRuby's backtraces.
    • Create a RubyEngine module to handle slight differences like
      the above between different ruby engines.
  • JRuby's backtrace doesn't give a clear indication of when methods
    are called from inside blocks.
    • It's not vital that this information be tracked by
      pebble objects, so we'll drop the specs that try to verify that.
  • JRuby's backtrace also prints out a different trace for methods:
    "from calling_method" instead of "in calling_method".
    • Drop the preposition and just verify against the method name

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling c2da4d4 on graysonwright:jruby into 5641a7c on avdi:master.

See the comments on #27

- Ran into prolems identifying the line and file of instantiation
for traceable null objects because of slight differences in
MRI's and JRuby's backtraces.
    - Create a RubyEngine module to handle slight differences like
the above between different ruby engines.
- JRuby's backtrace doesn't give a clear indication of when methods
are called from inside blocks.
    - It's not vital that this information be tracked by
pebble objects, so we'll drop the specs that try to verify that.
- JRuby's backtrace also prints out a different trace for methods:
"from calling_method" instead of "in calling_method".
    - Drop the preposition and just verify against the method name
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 9a86eae on graysonwright:jruby into 5641a7c on avdi:master.

@c-lliope
Copy link
Author

I had to delete two specs that just weren't compatible with JRuby:

  context "when is called from a block" do
    it "prints the indication of a block" do
      expect(test_output).to receive(:puts).twice.
        with(/from block/)
      Caller.new.call_method_inside_block(null)
    end
  end

  context "when is called from many levels blocks" do
    it "prints the indication of blocks and its levels" do
      expect(test_output).to receive(:puts).exactly(4).times.
        with(/from block \(2 levels\)/)
      Caller.new.call_method_inside_nested_block(null)
    end
  end

They were failing because JRuby (and RBX) don't give a clear indication of blocks in their stacktrace.

As an example, the following code:

def foo
  2.times { raise 'ohno' }
end

foo

when run in the three different environments, give these stack traces:

MRI:

RuntimeError: ohno
    from (irb):2:in `block in foo'
    from (irb):2:in `times'
    from (irb):2:in `foo'
    from (irb):4

JRuby:

RuntimeError: ohno
    from (irb):2:in `foo'
    from org/jruby/RubyFixnum.java:280:in `times'
    from (irb):2:in `foo'
    from (irb):4:in `evaluate'
    ...

RBX:

RuntimeError: ohno
    from (irb):2:in `foo'
    from kernel/common/integer.rb:83:in `times'
    from (irb):2:in `foo'
    from (irb):4
    ...

They all give similar information, but the specs are looking for a specific indication that the call is coming from inside a block.

I don't think it's really necessary for Pebble to collect detailed information on the blocks, so I decided to scrap those specs. Pebbles will collect slightly less information on JRuby/RBX, but still capture the method name that the pebble was called from, etc.

Let me know if you think those specs were necessary.

@avdi
Copy link
Owner

avdi commented Jan 16, 2014

Looks like @sferik has included JRuby support in his 1.8 compatibility PR. Thanks for getting the ball rolling!

@avdi avdi closed this Jan 16, 2014
@@ -8,7 +9,7 @@ def call
attr_reader :__file__, :__line__

def initialize(options={})
backtrace = options.fetch(:caller) { Kernel.caller(4) }
backtrace = options.fetch(:caller) { Kernel.caller(RubyEngine.backtrace_initialization_offset) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

I came to the same conclusion but with a somewhat different style implementation. If @avdi prefers your version, I’m happy to merge in this part of the pull request.

@c-lliope c-lliope deleted the jruby branch January 17, 2014 23:49
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

4 participants