Closed simPod closed 4 months ago
Maybe it's not a coincidence that webonyx/graphql-php is involved, too?
I'm seeing similar errors over at https://github.com/rebing/graphql-laravel when I run composer update and run the test suite.
Is https://github.com/sebastianbergmann/phpunit/issues/5866 a regression of this?
We had to lock phpunit to 10.5.20
due to this.
Thank you for your report.
Please provide a minimal, self-contained, reproducing test case that shows the problem you are reporting.
Without such a minimal, self-contained, reproducing test case I will not be able to investigate this issue.
Sorry meant: possible regression of https://github.com/sebastianbergmann/phpunit/pull/5861 ?
Destroy TestCase object after its test was run
Because the error is triggered in \PHPUnit\Framework\TestSuiteIterator::current
Yes, #5861 would be the only change in PHPUnit 10.5.21 that could explain this.
I just noticed also a difference:
vendor/bin/phpunit
-> fails as described abovevendor/bin/phpunit path/to/test/file/which/justfailed/above.php
-> error not triggered, test is greenLuckily, I can reproduce it when I provide a custom @group foo
on the test and run with --group foo
filter.
I set a breakpoint on the condition !isset($this->tests[$this->position])
There's no match on that position although there are elements in that array.
It seems there's something in webonyx/graphql-php specifically, which triggers this state.
I think I mostly see this when a test fails, the webonyx/graphql-php catches the error and, as part of the processing of that error, from the screenshot:
Eventally, printVar()
receives a TestSuite
object and the generic code therein detects it is of type \Countable
and calls count()
on it:
if (\is_object($var)) {
return 'instance of ' . \get_class($var) . ($var instanceof \Countable ? '(' . \count($var) . ')' : '');
}
When I remove that count()
call -> no problems.
I think I mostly see this when a test fails, the webonyx/graphql-php catches the error and, as part of the processing of that error, from the screenshot
If that that is indeed the root cause here then I will close this as "won't fix". An error handler that is not PHPUnit's must not mess with exceptions raised by PHPUnit that indicate test failure.
Hmm, but how is count()
on a \Countable
messing around with anything? 🤔
That is besides the point: the TestSuite
object in question, as well as the associated iterator, are internal to PHPUnit's test runner and must not be used from outsite of PHPUnit's test runner. As of #5861, TestCase
objects aggregated in TestSuite
are delete after their test was run.
Then maybe the \Countable
should be removed, as it's not fulfilling the contract?
@mfn you've indeed found the issue, it is not directly related to phpunit. ~For some reason~ the count()
call on Testcase object triggers the sideeffect via https://github.com/sebastianbergmann/phpunit/blob/1d08c5292edf73998a83a5119e3ecc77e3e8c1ef/src/Framework/TestSuite.php#L265-L274 that invokes FilterIterator and messes with tests indexing.
https://github.com/sebastianbergmann/phpunit/blob/1d08c5292edf73998a83a5119e3ecc77e3e8c1ef/src/Framework/TestSuite.php#L416-L424
Summary
v10.5.20 works fine. With 10.5.21 getting the error when I run the whole test suite.
https://github.com/webonyx/graphql-php/actions/runs/9533107528/job/26275958530
Current behavior
How to reproduce
checkout https://github.com/webonyx/graphql-php/commit/21bc031e4b0c2035e4d4ddb88ffef98e2f11c97f
composer install
vendor/bin/phpunit
Expected behavior
No error.