Be strict about data providers having redundant data#6213
Merged
sebastianbergmann merged 2 commits intosebastianbergmann:mainfrom May 20, 2025
Conversation
Owner
No. |
Owner
|
Thank you! |
Contributor
Author
|
@sebastianbergmann you are so fast 👍🏼 Even too fast: #6215 |
Owner
|
I have "downgraded" this from an error to a warning in 7f1969a. |
Contributor
Author
|
Cool. The warning will be on the test case level, not at the test method level, as the error was. |
Owner
|
The warning can be seen here: https://github.com/sebastianbergmann/phpunit/blob/main/tests/end-to-end/data-provider/too-many-arguments.phpt#L23 |
|
This might have a small regression for ... /**
* tests failure messages for assertions
*
* @param string $assertion Assertion method
* @param string $message Expected failure message
* @param string $url URL to test
* @param mixed ...$rest
*/
#[DataProvider('assertionFailureMessagesProvider')]
public function testAssertionFailureMessages($assertion, $message, $url, ...$rest): void
{
$this->expectException(AssertionFailedError::class);
$this->expectExceptionMessage($message);
Security::setSalt($this->key);
$this->get($url);
call_user_func_array($this->$assertion(...), $rest);
}If there is a ...$var maybe the warning should be skipped as it is intentional? |
|
Maybe a And if true we just skip? |
Owner
|
@dereuromark Please open a new issue if you think you found a bug/regression. Thank you. |
Contributor
Author
|
It is regression, I've raised the PR with the fix: #6228 |
This was referenced Jun 11, 2025
theofidry
added a commit
to theofidry/console-parallelization
that referenced
this pull request
Aug 19, 2025
It is an issue since sebastianbergmann/phpunit#6213 and those cases were legit.
theofidry
added a commit
to webmozarts/console-parallelization
that referenced
this pull request
Aug 19, 2025
) It is an issue since sebastianbergmann/phpunit#6213 and those cases were legit.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Closes #6211
Should this be hidden behind a config option?
Should it make the test risky/with a warning rather than producing error?