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

Expect coverage for .simplecov when running with JRuby #479

Closed
wants to merge 1 commit into from

Conversation

donv
Copy link
Contributor

@donv donv commented Mar 24, 2016

...when running code in .simplecov after SimpleCov.start has finished.

Downside is we get a skipped scenario indicated with dashes.

...when running code in `.simplecov` after `SimpleCov.start` has finished
@donv
Copy link
Contributor Author

donv commented Mar 24, 2016

JRuby tests are green in travis! 🍰

@donv
Copy link
Contributor Author

donv commented Mar 24, 2016

@colszowka Good to merge?

@donv
Copy link
Contributor Author

donv commented Mar 24, 2016

@colszowka @xaviershay @bf4 Anything more I need to do before merge?

@xaviershay
Copy link
Collaborator

Thanks for looking at this!

Could you add some context for why we expect these to be different? As written the scenarios are pretty confusing - why do we "expect" JRuby to have different results!?

The answer is likely - coz that's just how it works on JRuby and we don't care enough to fix it? If true, we should either a) document that, b) not specify the behaviour (i.e. existing example is "over specified" since we don't care about number of files, or c) exclude JRuby with a comment that this behaviour is undefined on JRuby.

(Option b makes the most sense to me, but I haven't thought about the problem a lot)

@donv
Copy link
Contributor Author

donv commented Mar 24, 2016

Thanks for responding!

There is some discussion in #478 . As you suggested, we expect this because that's how it is.

I confronted the JRuby team, and we feel we are doing the right thing. The natural next step is filing an issue with https://github.com/ruby/spec with a test case to clear up the expected behavior. I can do that (hopefully with some help from you?).

Before that, the behavior does differ across Ruby implementations. Too bad ruby-head, ruby-2.3, and rbx don't have green tests for simplecov. That could shed some light. Anyway, I really want to have green simplecov tests for JRuby before going further. When the verdict from ruby/spec comes back, we can make changes where appropriate.

@xaviershay
Copy link
Collaborator

seems legit - can we put that comment in the spec (or at least in the commit message) so it's easy for future spelunkers to find?

@donv
Copy link
Contributor Author

donv commented Mar 25, 2016

OK, started writing some specs for coverage in ruby/spec, and it will lead to changes in JRuby at least. So we can hold off this PR a little while longer.

@PragTob
Copy link
Collaborator

PragTob commented Jan 25, 2017

JRuby 9.1 seems to have resolved failures and runs cleanly now and supporting this through #547 I think this one can get closed as we don't need conditional tests then.

If anyone ( ping @donv ) disagrees please reopen/let me know :)

edit: still thanks everyone involved for your work! 🎉

@PragTob PragTob closed this Jan 25, 2017
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.

4 participants