hcoles / pitest

State of the art mutation testing system for the JVM
Apache License 2.0
1.69k stars 358 forks source link

Allow tests failing during initial coverage analysis #514

Open nrainer opened 6 years ago

nrainer commented 6 years ago

Sometimes a few tests exist that fail during the initial coverage analysis for whatever reason (e.g., caused by the class loading), although, they succeed when executed with mvn verify. However, the mutation analysis aborts as soon as one test case fails during the initial analysis. I suggest to add a parameter <allowedFailingTests> (or similar) indicating how many tests may at most fail during the initial analysis and continue the analysis without these tests. @hcoles If you consider this as sensible, I will prepare a pull request. Let me know :)

hcoles commented 6 years ago

This has been requested a few times, although it sounds reasonable my concern is that it will create an additional support burden.

If failing tests are allowed then they must also be automatically excluded from the analysis (otherwise they will appear to kill any mutant that they exercise). This ought to be simple enough, but as tests may be represented in an unbounded number of ways (due to custom junit runners etc), my concern is that some of these might end up being inadvertently run if the exclusion is attempted at a more fine grained level than a test class.

This possibility would have to be excluded everytime someone reported an erroneously killed mutant.

The alternative to auto allowing failing tests is for the user to add the ones that did fail to the exclude list. Pitest doesn't do a brilliant job of reporting which ones these are at the moment, so I think improving the reporting of failing tests would be a more valuable pull request.

nrainer commented 6 years ago

Thanks for your response. :)

I understand your concerns. However, these are the downsides I see with the exclude list: it takes some effort to create and maintain the list, tests no longer failing with PIT will not be detected and removed from the list, frustration if you run an analysis on a large project (using the pitmp extension) and the whole build fails because of a test in one of the last modules, flickering (integration) tests causing even more frustration (which of course should not be flickering, but are unfortunately very common in practice).

I am not sure if I understand why the exclusion of failing tests in this experimental branch might not behave as expected: https://github.com/hcoles/pitest/compare/master...nrainer:features/allow_failing_tests Can you point out what could go wrong here?

Reporting the failing tests at a glance would still be nice and should be easy to implement. https://github.com/hcoles/pitest/pull/507 was probably already a step towards that. However, I think that the improved reporting does not compensate for this often requested feature.

nrainer commented 5 years ago

I think this has been addressed with #528. Shall we close this issue?