fix: stop counting a mutant whose subprocess never ran a test as killed - #45
Open
tiagoabsantos wants to merge 1 commit into
Open
tiagoabsantos wants to merge 1 commit into
tiagoabsantos wants to merge 1 commit into
Conversation
A mutant's subprocess inherited two things it cannot use, and both made it exit before running a single test. MutationTest::hasFinished() reads any unsuccessful exit as a kill, so those mutants were reported as tested and the score read 100%. ParaTest-only options such as --processes and --passthru-php were forwarded verbatim. The mutant runs one PHPUnit process, which aborts with 'Unknown option "--processes"' and exit code 2. They are now stripped in MutantArguments, alongside the coverage options that were already dropped, so the original run keeps them for its own parallel execution. The subprocess also used the Pest script as its executable. That works wherever the shebang is honoured, but on Windows cmd.exe reads 'vendor/bin/pest' as the command 'vendor' and fails with exit code 1. The mutant now runs through PHP_BINARY, the same way the original run did. Closes pestphp/pest#1937
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes pestphp/pest#1937.
--mutatecan report a 100% score for mutants whose subprocess never ran a single test.MutationTest::hasFinished()reads any unsuccessful exit as a kill, so a subprocess that dies before reaching the test suite is indistinguishable from a legitimately killed mutant. Two inherited arguments make that happen.ParaTest-only options are forwarded to the mutant (all platforms)
A mutant runs one PHPUnit process, never ParaTest, but the original arguments are passed through as they were.
--parallelis already dropped byParallelOption::remove();--processesand--passthru-phpare not, and PHPUnit rejects them:Exit code 2 in a fraction of a second, counted as killed. This is the Linux CI case in the issue, and it applies to every ParaTest-only option, so
MutantArgumentsstrips them all rather than only the two that were reported. It also takes over the coverage stripping that was inline, so the rules for what a mutant may not receive live in one place.The options are removed from the mutant's arguments only, not from the configuration, so the original run keeps
--processesand--passthru-phpfor its own parallel execution — dropping--passthru-phpthere would take pcov with it and leave the run without coverage.--processes 2spends two arguments, so the value is dropped along with the option; a stray2would otherwise be read as a test path.The Pest script is used as the executable (Windows)
new Process([...$filteredArguments, ...])puts$originalArguments[0], the Pest script, in the executable position. That works wherever the shebang is honoured. On Windowscmd.exereadsvendor/bin/pestas the commandvendor:Exit code 1, counted as killed. The mutant now runs through
PHP_BINARY, the same way the original run reached the same script.Verification
Against
tests/.tests/Untested, whose snapshot expects 3 untested out of 4, on Windows 11 with PHP 8.4.25:Mutations: 4 tested,Score: 100.00%, 0.17sMutations: 3 untested, 1 tested,Score: 25.00%, 2.36s--parallel --processes=2 --passthru-php=...Mutations: 4 tested,Score: 100.00%Mutations: 3 untested, 1 tested,Score: 25.00%Both new tests in
tests/Unit/MutationTestTest.phpfail on the current5.xand pass here. They run a real subprocess against a stand-in for the Pest script that records its own$argv, so they cover the executable and the forwarded arguments the way a mutant actually sees them, on Linux as well as Windows.pint --test,phpstan, andpest --type-coverage --min=100are clean. The 7 failures left inpeston my Windows machine are all present on5.xbefore this branch: they are the path and line-ending issues already covered by #36 and #44.Not included
The issue also suggests not counting a run in which no test was executed as tested, as a backstop for any other environment problem in the subprocess. That needs a signal beyond the exit code — exit code 2 is also what a legitimately killed mutant produces when it makes a test error — so it is a separate change, and #42 is already exploring one shape of it. This PR fixes the two causes that are unambiguous.