PHPUnit 11 compatibility #1766
PHPUnit 11 compatibility #1766
Conversation
src/Test/AbstractMakerTestCase.php
Outdated
| self::assertEquals(1, substr_count($haystack, $needle), \sprintf('Found more than %d occurrences of "%s" in "%s"', $count, $needle, $haystack)); | ||
| } | ||
|
|
||
| private static function getMakerInstance(string $makerClass): MakerInterface |
There was a problem hiding this comment.
this should not be static. The maker instance should be created inside the test, so that it is coming from the containing booted for that test.
Data providers are executed before all tests, so all your cases are currently using the same container instance, breaking test isolation.
I think the retrieval of the maker instance from the service locator (introduced in #1771) should be moved to the test itself.
There was a problem hiding this comment.
That's an excellent point. By delaying the retrieval of the maker in the test body, there are far fewer methods that need to be made static. It's no longer necessary to create a new class for MakerTestCase.
GromNaN
force-pushed
the
phpunit-11
branch
2 times, most recently
from
3688847 to
02dbbfd
Compare
| use Symfony\Component\Process\Process; | ||
|
|
||
| /** | ||
| * @method static iterable<array{0: MakerTestDetails}> getTestDetails() |
There was a problem hiding this comment.
By removing the abstract method declaration, moved to a PHPDoc, I avoid the breaking change.
We have have to be careful to not add the abstract static method declaration in 1.x, but only in 2.x.
| */ | ||
| protected function createMakerTest(): MakerTestDetails | ||
| { | ||
| trigger_deprecation('symfony/maker-bundle', '1.66.0', 'The "%s()" method is deprecated. Use "self::newMakerTest()" instead.', __METHOD__, self::class); |
There was a problem hiding this comment.
This method is renamed because it needs to be static. We don't want to introduce a breaking change in case this method is overridden.
| * @return void | ||
| */ | ||
| protected function assertContainsCount(string $needle, string $haystack, int $count) | ||
| protected static function assertContainsCount(string $needle, string $haystack, int $count) |
There was a problem hiding this comment.
This is a breaking change, but there is no reason to override this method. And all assertion methods are usually static.
src/Test/AbstractMakerTestCase.php
Outdated
| self::assertEquals(1, substr_count($haystack, $needle), \sprintf('Found more than %d occurrences of "%s" in "%s"', $count, $needle, $haystack)); | ||
| } | ||
|
|
||
| private static function getMakerInstance(string $makerClass): MakerInterface |
There was a problem hiding this comment.
That's an excellent point. By delaying the retrieval of the maker in the test body, there are far fewer methods that need to be made static. It's no longer necessary to create a new class for MakerTestCase.
2 participants
Making all the data providers static require to make many other methods static in the
testsdirectory.The main point is the
MakerTestCasethat is part of the API and is extended in other projects. Creating a new class is the only way to change abstract methods from non-static to static in a BC way.