Skip to content

Fix JsonProfilePrinter#2187

Merged
enebo merged 3 commits intojruby:jruby-1_7from
dmarcotte:test-profiler
Nov 9, 2015
Merged

Fix JsonProfilePrinter#2187
enebo merged 3 commits intojruby:jruby-1_7from
dmarcotte:test-profiler

Conversation

@dmarcotte
Copy link
Contributor

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
Copy link
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
Copy link
Contributor Author

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.

@mkristian mkristian force-pushed the jruby-1_7 branch 2 times, most recently from 7e7562b to e355e11 Compare August 10, 2015 20:10
@mkristian mkristian force-pushed the jruby-1_7 branch 2 times, most recently from e801fed to 9d04c55 Compare October 13, 2015 10:11
`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.
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
Copy link
Contributor Author

@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
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