-
Notifications
You must be signed in to change notification settings - Fork 75
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
Disallow surviving mutants in strict mode #1017
Disallow surviving mutants in strict mode #1017
Conversation
The warnings from In general, the idea of failing in case of surviving mutants makes sense and is a great addition. |
Although, enabling it by default would likely break many integration tests, so I guess it's better to not turn it on by default just yet - unless you also want to update all the affected tests 😄 |
I see. Then it's just my getting tricked by the wording :)
A separate argument just as well. I'll have a look at it. If the number of failing tests is managable, I'll enable it by default. As for the naming, perhaps something along the lines of --allow-surviving (or --disallow-surviving if not enabled by default)? It's a little verbose but at least it should be obvious what it does. |
Or, perhaps, we could simply enable the option by default but explicitly disable it for the integration tests. That would save me quite a bit of work I'd imagine |
|
The release CI stage of the provisioning fails due to mull_version not being passed as a variable to ansible. Check the environment and, if no version is set, fall back on the latest git tag with potential -dirty suffix
When running provisioning through vagrant, pip installs its packages to /home/vagrant/.local which is not in PATH when the lit tests are run. Add it manually. While this is not required for the github workflows (and probably not macos either), it shouldn't do any harm
519ca8e
to
14174b7
Compare
As discussed, mutants surviving is now treated as an error by default but can be ignored by passing the Also made some minor changes to the infrastructure to make the CI workflow succeed when run via vagrant. Tested with the ubuntu20.04 box. |
Surviving mutants would typically indicate insufficient test coverage. As such, it makes sense for Mull to treat it as an error and exit with a non-zero status. This changes the default behavior of mull-runner which up until this point has exited with zero regardless of how many mutants survived. This behavior can be "restored" by passing the --allow-surviving CLI argument to mull-runner. Since the mutation scores for both fmtlib and openssl are very low, the --allow-surviving flag is passed when running the integration tests This does not treat the absence of mutants (i.e. infinite mutation score) as an error
Thank you so much @vengaer! |
No worries! You guys have something special here, happy to help in whatever little way I can |
The --help for strict mode states that "all warning messages are treated as fatal errors". While this is indeed true for anything that goes through Diagnostics::warning, there are a few messages presented as warnings that are not written via said function. The one (and at least so far only) concrete example I have come across are warnings written in printMutant in IDEReporter.cpp which emits messages similar to
Given the description for the --strict flag, it would seem as though the above should be considered a fatal error too.
The warnings written in printMutant are currently written through fprintf. While it would be simple to change this this to use Diagnostics::warning instead, that doesn't seem to be the best approach. Seeing as all the mutants have already run, all that would achieve is cut parts of the report.
Instead of omitting useful information, this checks the number of surviving mutants at the very end of mull-runner's main. If any survived, this is logged this via Diagnostics::warning. This way, mull-runner will exit with a non-zero status if there are surviving mutants in strict mode. If not in strict mode, mull-runner always exits with status 0.
This change would, of course, alter the behavior of the --strict flag which I could see being a deal breaker. If that is the case, perhaps it'd be better to add another command line argument to achieve the intended results. Either way, I'd personally find it very useful to have a way of forcing Mull to exit with an error when there are surviving mutants.
Please let me know if you'd prefer this to be handled differently