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

Make ResultReporter respect the failure status set by other plugins; add test for coveragerc from config #405

Merged
merged 2 commits into from
Nov 8, 2018

Conversation

sirosen
Copy link
Collaborator

@sirosen sirosen commented Aug 11, 2018

First of all, a goodly chunk of this is just a rebase of #395, and I don't want to pretend otherwise. I renamed the test, as it no longer fails. coverage_config_fail_under2 may not be the most inspired name, but I'm not sure it really matters.

I considered pushing this rebase to #395, but decided to open a fresh PR instead.

More importantly:

  • Tweak ResultSuccessEvent to start at None
  • Tweak ResultReporter to differentiate None vs False on the incoming ResultSuccessEvent
  • Tweak Coverage to ensure that it properly handles the new starting state for ResultSuccessEvent

There's still #397 as a related issue -- nose2.plugins.coverage should still either execute before or after nose2.plugins.result without it mattering how it was enabled -- but this resolves #396 without tackling that work (yet). Once this is done, a solution for #397 becomes easier to work on too.

robertsben and others added 2 commits August 11, 2018 02:44
Test for coverage "fail_under" setting with coverage options loaded from
config. Due to a subtlety of test success/failure reporting, the failure
from the coverage plugin is ignored in this case.
If a plugin executes wasSuccessful() before the ResultReporter plugin,
ResultReporter would overwrite the result. To fix, tweak the
ResultSuccessEvent creation process to start at `None`, rather than
`False`, and treat `None` specially. This lets the ResultReporter
clearly see whether or not a result was explicitly set to `False` by a
prior plugin.

Also, fix the coverage plugin to be aware of this behavior when setting
a result status.

closes nose-devs#395
closes nose-devs#396
@sirosen
Copy link
Collaborator Author

sirosen commented Nov 8, 2018

Self-merging as maintainership of nose2 is in transition.
If you see this and want to contribute to the project in the form of code reviews and feedback, please email me.

@sirosen sirosen merged commit 3380ded into nose-devs:master Nov 8, 2018
@sirosen sirosen deleted the respect-plugin-failure branch November 8, 2018 05:22
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.

Coverage fail_under not being respected
2 participants