Problem with "Skipped" #1304
Comments
|
Maybe @sanmai can help here? |
|
You need to raise the timeout if you want Infection to run these mutations. Mutations do run in timeouts still, but since we filter out know predictable timeouts (which do not affect the score), you shall only see actual timeouts, e.g. mutations that cause your program to run endlessly or excessively long. Quoting from https://infection.github.io/2020/08/18/whats-new-in-0.17.0/
|
|
Hello @sanmai, thx for your fast respone. But I don't understand why mutants which did not cause a timeout in version 0.16.4 should cause a timeout in version 0.17.2? |
|
There were some new mutations added, note you have 3950 mutations in the older version, and 4095 mutations in the newer. They might be hitting some over-covered code. It is hard to tell precisely without looking at the code. Try increasing the timeout still (it's in Or you can find these mutations on the log to figure out which lines they are hitting. Is there an increasingly long integration test?.. Tagging it as |
|
I had checked that the skipped mutations were not triggered by a new mutator. I had already set the timeout to 300 and none of these mutations went into a timeout - but the mutations were no longer skipped. For infection only small unit tests and no long running integration tests are used in my setup. |
|
If I can have a look at your project, we may figure something out. What do you think? It doesn't have to be all open source for all I care, but if it is it'll make all things easier. |
|
Since this is a customer project, I can unfortunately not give you access to the project. I understand that it is so naturally difficult to reproduce the behaviour. I will try to create a minimal test case to reproduce the behaviour. |
|
Sure. You can add debugging output right about here. At very least you could find out at what timeout you should be aiming at. |
|
I might be having a similar problem.... My project's complete test suite runs in 5 seconds (see output below using this commit). I believe the infection timeout is defaulted to 10 seconds. If I understand this functionality correctly then surely no test takes more than 10 seconds to run, and therefore none should be marked as skipped? Output: Increasing timeout to 90 seconds does decrease the number of mutants marked as skipped. My best guess is that this functionality is working but the timing is wrong. |
|
OK, does this recreate the problem: https://infection-php.dev/r/608 Assuming playground has a timeout of 10s. This test should only run 5s. So no reason for any to be skipped? |
|
Thank you for the demo. There's indeed something fishy here. |
|
@sanmai I was looking at PR that added functionality, but can't see issue. I added this comment: #1171 (comment) assuming it's a timing issue then adding in VO for time intervals would help find issues sooner. |
|
This issue might have been here for a long time. There isn't a problem in #1171 per se, but rather there are some broken assumptions down the call stack. Can you see if #1310 improves the situation for you? @ingowalther you're welcome to try the new workaround too. |
TestLocator returns non-unique tests, and JUnitTestCaseSorter works around that; we have to do that too.
Fixes infection#1304 TestLocator returns non-unique tests, and JUnitTestCaseSorter works around that; we have to do that too.
|
@sanmai Can I check my assumption here.... I am assuming that E.g. Having a timeout of >7 would produce 0 skipped mutants? If my assumption is correct then this PR does not appear to have fixed the issue. |
|
Do you say that you still have this issue even with #1310? |
|
For my testsuite I can confirm that this pull-request eliminates the unwanted "skipped" cases. |
|
@DaveLiddament for SARB with #1310 I'm getting the following report with 0 skipped for timeout of 5 seconds: I wonder if you can confirm this. Should I use a shorter timeout value? |
Great. Looks like you've solved it. Thanks for getting this fixed. I can't recreate your results locally (I'm still getting skipped mutants). As it works for you and the originally bug submitter this must be me not setting up something correctly. Can you see anything I'm obviously doing wrong? If not just put this down to user error on my part.... PHP version Infection output Config: Git sha of infection: |
|
Try Or, even better, do (With |
|
I've created a new VM. Built a PHAR for infection and run it... If I leave default timeout of 10s, then I still get mutants marked as So yes, your fix has made it work. That said I still don't understand why the timeout needs to be so high. If the entire test suite runs in 6 seconds (as reported by infection and I've independently timed it as well), then why does a timeout of 10 still mean mutants are skipped? |
This is how it should be, but you're correct it is not yet so. Still looking into this. I think this should output the full time needed to run all tests: So timeout of 1 second should work, but it does not. |
|
Infection tells me that these tests took this much: Although looking at JUnit report I do not see exactly these numbers ( It appears these lines have something to do with these slightly inflated times. What I can say that we've dealt with the blown-up-out-of-proportion numbers, and what we have here is a slightly different issue. |
|
@DaveLiddament Could you try with the latest from #1310? So far I'm getting 0 skipped with timeout of 1 second. |
|
@sanmai yes that works! Fantastic work, both on this new feature and getting this fix in. Thanks very much for all you effort today. |
|
Yeah! Thanks for your work @sanmai! |
…1310) * TestLocator returns non-unique tests, but Mutation didn't know that Fixes #1304 TestLocator returns non-unique tests, and JUnitTestCaseSorter works around that; we have to do that too. * Extend test case to include summation * Timings are per test suite, not per test, therefore we have to unique by test suite name * Fix PHPStan warning * Extract adding logic into JUnitTestCaseTimeAdder
…nfection#1310) * TestLocator returns non-unique tests, but Mutation didn't know that Fixes infection#1304 TestLocator returns non-unique tests, and JUnitTestCaseSorter works around that; we have to do that too. * Extend test case to include summation * Timings are per test suite, not per test, therefore we have to unique by test suite name * Fix PHPStan warning * Extract adding logic into JUnitTestCaseTimeAdder
|
Thank you guys for providing examples, especially nice to see Playground example ;) Thank you @sanmai for such a quick fix. Released: https://github.com/infection/infection/releases/tag/0.17.3 |
…1310) * TestLocator returns non-unique tests, but Mutation didn't know that Fixes #1304 TestLocator returns non-unique tests, and JUnitTestCaseSorter works around that; we have to do that too. * Extend test case to include summation * Timings are per test suite, not per test, therefore we have to unique by test suite name * Fix PHPStan warning * Extract adding logic into JUnitTestCaseTimeAdder
|
Thanks @maks-rafalko! |
|
Sure, it's automatically deployed after successful build for tags, so give it some time please |
|
I didn't mean to push, I just didn't know how the process worked. Thanks for the explanation. Thanks for the great work on Infection! I wish you a nice day |
|
@ingowalther should be there. |
|
Hi everyone, I think I'm experiencing the same issue (or a very similar one). |
|
Well, in the playground, there is a It's clearly visible in this example: https://infection-php.dev/r/4yv When I set timeout to 1, there are no Are you sure you experiencing the very same issue with timeout properly set? Do you have any example? |
|
Ah ok, let me work on small reproduction case then |
In version 0.16.4 most mutations do not run in timeouts. However, after updating to version 0.17.2, some of these mutants will be "skipped".
I also checked that the mutants with "skipped" are not generated by the new "instanceof" mutator.
I don't understand exactly why this happens? Is this a bug or intentional behavior?
Output Infection 0.17.2:
Output Infection 0.16.4:
The text was updated successfully, but these errors were encountered: