Fix JsonProfilePrinter #2187

Merged
merged 3 commits into from Nov 9, 2015

Conversation

Projects
None yet
3 participants
@dmarcotte
Contributor

dmarcotte commented Nov 12, 2014

first flag was incorrectly set, resulting in a leading comma for json profile output.

Also remove some calls to rspec's have, which has been removed from rspec since the profiler specs were last run.

@headius, with this the spec/profiler specs are green for 1.7. Do you want to re-enable on Travis?

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 12, 2014

Member

Great to hear you got them all working! Yes, we will reenable on travis. Add that to PR and we'll pull it in.

Member

headius commented Nov 12, 2014

Great to hear you got them all working! Yes, we will reenable on travis. Add that to PR and we'll pull it in.

@dmarcotte

This comment has been minimized.

Show comment
Hide comment
@dmarcotte

dmarcotte Nov 13, 2014

Contributor

Ready to go! Specs are enabled and verified (also fixed a problem where it was messing with rake's final outcome in spite of all passing tests).

Note: I only flipped the switch for 1.9 since it turns out there's still one failure for 1.8:

1) JRuby's profiling mode reports unimplemneted methods like fork as unimplemented
     Failure/Error: Kernel.respond_to?(:fork).should == false
       expected: false
            got: true (using ==)
     # ./spec/profiler/profiler_basics_spec.rb:8:in `(root)'

Let me know if there's a strong desire to see 1.8 profile specs running in Travis and I'll try and send a follow-up pull addressing this.

Contributor

dmarcotte commented Nov 13, 2014

Ready to go! Specs are enabled and verified (also fixed a problem where it was messing with rake's final outcome in spite of all passing tests).

Note: I only flipped the switch for 1.9 since it turns out there's still one failure for 1.8:

1) JRuby's profiling mode reports unimplemneted methods like fork as unimplemented
     Failure/Error: Kernel.respond_to?(:fork).should == false
       expected: false
            got: true (using ==)
     # ./spec/profiler/profiler_basics_spec.rb:8:in `(root)'

Let me know if there's a strong desire to see 1.8 profile specs running in Travis and I'll try and send a follow-up pull addressing this.

dmarcotte added some commits Nov 12, 2014

Fix JsonProfilePrinter
`first` flag was incorrectly set, resulting in a leading comma for json
profile output.

Also remove some calls to rspec's `have`, which has been removed from
rspec since the profiler specs were last run.

This gets the spec/profiler specs green.
Update profile test exit check
This was incorrectly reporting a failure because RSpec::Core::Runner now
returns an exit status rather than true/false (it's *has* been a while
since this was run...)
@dmarcotte

This comment has been minimized.

Show comment
Hide comment
@dmarcotte

dmarcotte Nov 6, 2015

Contributor

@headius just noticed this guy was unmerged... just slipped off your radar too? (No worries if that's the case!) Or is there something we want to address here?

Also: I just rebased to the latest jruby-1_7, so Travis should soon (hopefully...) confirm everything is still cool here.

Contributor

dmarcotte commented Nov 6, 2015

@headius just noticed this guy was unmerged... just slipped off your radar too? (No worries if that's the case!) Or is there something we want to address here?

Also: I just rebased to the latest jruby-1_7, so Travis should soon (hopefully...) confirm everything is still cool here.

@enebo enebo added this to the JRuby 1.7.23 milestone Nov 9, 2015

enebo added a commit that referenced this pull request Nov 9, 2015

@enebo enebo merged commit 553cf61 into jruby:jruby-1_7 Nov 9, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment