Open
Conversation
…uments.
Not making new tests to avoid changes in "Visual" tests.
The following statement should not fail and simply return null:
PHPUnit\Framework\assertEquals((new class {use Pest\Plugins\Concerns\HandleArguments;})->popArgument('anything',['same', 'same']),['same', 'same']);
…gument handling logic - This fixes duplicate argument exclusion with --parallel
Closed
2 tasks
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
1 participant
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.
What:
Description:
This PR fixes issue 1437 where additional --exlude-group options were ignored when using --parallel flag.
It's a much smaller version of PR 1468. This PR doesn't bring the additional tests to avoid changes to Visual test snapshots, but the proof of the problem and fix is available with a single statement given below.
Problem:
In version 3.x and 4.0, running tests with
--parallelusing multiple--exclude-groupoptions, only the first option was used.This probably applies to other multiple-use options such as
--group.This problem is made worse in v4.0 using PHPUnit 12, forcing the use of multiple option vs comma-separated list.
Root Cause:
Some plugin argument handlers remove an argument from the
$argumentsarray byusing array_flip()twice. They wrongly assume the remaining array is unchanged, but any non-unique value is removed from the array.This breaks Parallel because long options (
--option=value) in the argument array appear as two items (['--option','value']) causing potential duplicate keys.Why does it work when not using --parallel?
The non-parallel worker avoids duplicate keys in the argument pipeline by keeping option and value together in the array (
['--option=value']).Solution:
Modified
popArgument()inHandleArgumentsTrait to useunset()instead of the doublearray_flip()Modified
Coverageplugin to use the Trait method instead of its own doublearray_flip()code.Testing:
This PR does not add new tests to avoid changes in "Visual" tests.
The following statement should not fail and simply return null, but it does fail on 4.x branch without this PR
PHPUnit\Framework\assertEquals((new class {use Pest\Plugins\Concerns\HandleArguments;})->popArgument('anything',['same', 'same']),['same', 'same']);Related:
Fixes issue 1437
Better version of PR 1468