Skip to content

Commit 93ff20a

Browse files
ondrejmirtesclaude
andcommitted
Install the right container before every test and data provider on PHPUnit 9 too
PHPStanPHPUnitExtension implements the PHPUnit >= 10 extension interface and is registered through <extensions><bootstrap>, which PHPUnit 9 rejects outright ("Element 'bootstrap': This element is not expected. Expected is ( extension )"). The "Tests with old PHPUnit" jobs therefore ran without its two subscribers, so nothing made a data provider or a test see its own test class's container - and because ParaTest reuses one worker process for many test files, whatever container the previous file installed stayed installed. That is what flaked IntersectionTypeTest::testIsAcceptedBy on the 8.1 jobs: a bleeding edge test file left BleedingEdgeToggle on, dataIsAcceptedBy() built its ConstantArrayTypes sealed (the constructor reads the toggle), dataGetEnumCases() then called createReflectionProvider() and switched the toggle back off, and every data set expecting Maybe came out No. Reproduced deterministically with paratest --runner WrapperRunner -p 1 -c <config without the extension> over a bleeding edge test file followed by IntersectionTypeTest. The same gap made VerbosityLevelTest and 29 other files fail with MissingStaticAccessorInstanceException whenever they were the first file a worker picked up. setUp() now always installs the test class's own container, tearDownAfterClass() restores the base one for the next file's data providers, and the bootstrap installs it for the first file's - which makes $reinitializeContainerBeforeEachTest redundant - and InitContainerBeforeTestSubscriber too, so setUp() is now the one mechanism for tests on every PHPUnit version. InitContainerBeforeDataProviderSubscriber stays: data providers run while the suite is being built, and a single-process PHPUnit >= 10 run calls every class's providers before the first test, so a provider that installs its own container would otherwise leak it into the next class's providers. PHPUnit 9 has no such hook, so a data provider of a test class whose container is not the base one still has to ask for it itself there, the way TypeGetFiniteTypesTest now does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019isZYNh3Zms6xGKGGXqxgp
1 parent 51b4d48 commit 93ff20a

12 files changed

Lines changed: 120 additions & 67 deletions

‎src/Testing/PHPStanTestCase.php‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use PHPStan\Reflection\ReflectionProvider;
2323
use PHPStan\Reflection\ReflectionProvider\DirectReflectionProviderProvider;
2424
use PHPStan\Rules\Properties\PropertyReflectionFinder;
25+
use PHPStan\Testing\PHPUnit\ContainerInitializer;
2526
use PHPStan\Type\Constant\OversizedArrayBuilder;
2627
use PHPStan\Type\ExpressionTypeResolverExtension;
2728
use PHPStan\Type\OperatorTypeSpecifyingExtensionRegistry;
@@ -43,21 +44,28 @@ abstract class PHPStanTestCase extends TestCase
4344
use PHPStanTestCaseTrait;
4445

4546
/**
46-
* Re-register the runtime container as the global static reflection provider before
47-
* every test. Enable this in tests that construct Type objects directly and assert
48-
* PHP-version-dependent reflection results, so a foreign PhpVersion leaked by another
49-
* test can't flake them. See https://github.com/phpstan/phpstan/issues/14860
47+
* Re-register this test class's container as the owner of the global static state
48+
* (the reflection provider, the PhpVersion, the bleeding edge toggle) before every
49+
* test, so a container another test class installed can't flake tests that build
50+
* Type objects directly. See https://github.com/phpstan/phpstan/issues/14860
51+
*
52+
* This is the only hook that does it, on every PHPUnit version. Data providers
53+
* are covered separately - see PHPStanTestCaseTrait::restoreBaseContainer().
5054
*/
51-
protected bool $reinitializeContainerBeforeEachTest = false;
52-
5355
#[Override]
5456
protected function setUp(): void
5557
{
56-
if (!$this->reinitializeContainerBeforeEachTest) {
57-
return;
58-
}
58+
ContainerInitializer::initialize(static::class);
59+
}
60+
61+
#[Override]
62+
public static function tearDownAfterClass(): void
63+
{
64+
parent::tearDownAfterClass();
5965

60-
self::getContainer();
66+
// The next test class's data providers run before any hook this test case has
67+
// on PHPUnit 9, in the same ParaTest worker process - see restoreBaseContainer().
68+
self::restoreBaseContainer();
6169
}
6270

6371
public static function getParser(): Parser

‎src/Testing/PHPStanTestCaseTrait.php‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,43 @@ trait PHPStanTestCaseTrait
2222

2323
public static function getContainer(): Container
2424
{
25-
$additionalConfigFiles = [__DIR__ . '/TestCase.neon'];
25+
$additionalConfigFiles = [self::getBaseConfigFile()];
2626
foreach (static::getAdditionalConfigFiles() as $configFile) {
2727
$additionalConfigFiles[] = $configFile;
2828
}
29-
$composerAutoloaderProjectPaths = static::getComposerAutoloaderProjectPaths();
29+
30+
return self::getContainerForConfigFiles($additionalConfigFiles, static::getComposerAutoloaderProjectPaths());
31+
}
32+
33+
/**
34+
* Installs the global state of the container built from just the base config -
35+
* the bleeding edge toggle, the static reflection provider, the PhpVersion and
36+
* the type caches all live in static properties that the last initialized
37+
* container owns.
38+
*
39+
* PHPStanTestCase::setUp() installs the right container before every test. Data
40+
* providers are called while the test suite is being built, so on PHPUnit >= 10
41+
* InitContainerBeforeDataProviderSubscriber installs it before each of them.
42+
* PHPUnit 9 - which the "Tests with old PHPUnit" jobs run - rejects that extension
43+
* and has no hook that fires before a data provider, while ParaTest reuses one
44+
* worker process for many test files. A
45+
* class whose container turns on e.g. bleeding edge would therefore leak that
46+
* state into the data providers of the next test file, which build Type objects
47+
* that read the toggle in their constructor. Restoring the base state when a test
48+
* class is done keeps those data providers deterministic on every supported
49+
* PHPUnit version.
50+
*/
51+
public static function restoreBaseContainer(): void
52+
{
53+
self::getContainerForConfigFiles([self::getBaseConfigFile()], []);
54+
}
55+
56+
/**
57+
* @param string[] $additionalConfigFiles
58+
* @param string[] $composerAutoloaderProjectPaths
59+
*/
60+
private static function getContainerForConfigFiles(array $additionalConfigFiles, array $composerAutoloaderProjectPaths): Container
61+
{
3062
$cacheKey = hash('sha256', implode("\n", $additionalConfigFiles) . "\0" . implode("\n", $composerAutoloaderProjectPaths));
3163

3264
if (!isset(self::$containers[$cacheKey])) {
@@ -66,6 +98,11 @@ public static function getContainer(): Container
6698
return self::$containers[$cacheKey];
6799
}
68100

101+
private static function getBaseConfigFile(): string
102+
{
103+
return __DIR__ . '/TestCase.neon';
104+
}
105+
69106
/**
70107
* @return string[]
71108
*/

‎src/Testing/PHPUnit/InitContainerBeforeTestSubscriber.php‎

Lines changed: 0 additions & 32 deletions
This file was deleted.

‎src/Testing/PHPUnit/PHPStanPHPUnitExtension.php‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,6 @@ public function bootstrap(
2121
$facade->registerSubscriber(
2222
new InitContainerBeforeDataProviderSubscriber(),
2323
);
24-
$facade->registerSubscriber(
25-
new InitContainerBeforeTestSubscriber(),
26-
);
2724
}
2825

2926
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Testing;
4+
5+
use Override;
6+
use PHPStan\DependencyInjection\BleedingEdgeToggle;
7+
8+
/**
9+
* The global state a container installs - here the bleeding edge toggle, which
10+
* ConstantArrayType reads in its constructor - has to belong to the test class
11+
* that is running, not to whichever class ran before it in the same ParaTest
12+
* worker process. PHPStanTestCase::setUp() guarantees that on every PHPUnit
13+
* version.
14+
*/
15+
class BleedingEdgeContainerStateTest extends PHPStanTestCase
16+
{
17+
18+
#[Override]
19+
public static function getAdditionalConfigFiles(): array
20+
{
21+
return [
22+
__DIR__ . '/../../../conf/bleedingEdge.neon',
23+
];
24+
}
25+
26+
public function testThisClassesContainerOwnsTheGlobalState(): void
27+
{
28+
$this->assertTrue(BleedingEdgeToggle::isBleedingEdge());
29+
}
30+
31+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Testing;
4+
5+
use PHPStan\DependencyInjection\BleedingEdgeToggle;
6+
7+
/**
8+
* @see BleedingEdgeContainerStateTest for the same guarantee with a non-base container
9+
*/
10+
class ContainerStateTest extends PHPStanTestCase
11+
{
12+
13+
public function testThisClassesContainerOwnsTheGlobalState(): void
14+
{
15+
$this->assertFalse(BleedingEdgeToggle::isBleedingEdge());
16+
}
17+
18+
}

‎tests/PHPStan/Type/Accessory/HasPropertyTypeTest.php‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,6 @@
2323
class HasPropertyTypeTest extends PHPStanTestCase
2424
{
2525

26-
// Pin the runtime container so a foreign PhpVersion leaked by another test
27-
// can't flake the version-dependent Closure data set below.
28-
// See https://github.com/phpstan/phpstan/issues/14860
29-
protected bool $reinitializeContainerBeforeEachTest = true;
30-
3126
public static function dataIsSuperTypeOf(): array
3227
{
3328
return [

‎tests/PHPStan/Type/Generic/GenericObjectTypeTest.php‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,6 @@
3232
class GenericObjectTypeTest extends PHPStanTestCase
3333
{
3434

35-
// Pin the runtime container so a foreign PhpVersion leaked by another test
36-
// can't flake the version-dependent data sets (reflected variance of built-in
37-
// generics). See https://github.com/phpstan/phpstan/issues/14860
38-
protected bool $reinitializeContainerBeforeEachTest = true;
39-
4035
public static function dataIsSuperTypeOf(): array
4136
{
4237
return [

‎tests/PHPStan/Type/ObjectTypeTest.php‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,6 @@
5353
class ObjectTypeTest extends PHPStanTestCase
5454
{
5555

56-
// Pin the runtime container so a foreign PhpVersion leaked by another test
57-
// can't flake the version-dependent Closure data sets (dynamic-property
58-
// handling). See https://github.com/phpstan/phpstan/issues/14860
59-
protected bool $reinitializeContainerBeforeEachTest = true;
60-
6156
public static function dataIsIterable(): array
6257
{
6358
return [

‎tests/PHPStan/Type/TypeCombinatorTest.php‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,6 @@
7474
class TypeCombinatorTest extends PHPStanTestCase
7575
{
7676

77-
// Pin the runtime container so a foreign PhpVersion leaked by another test
78-
// can't flake the version-dependent data sets (dynamic-property handling of
79-
// final classes). See https://github.com/phpstan/phpstan/issues/14860
80-
protected bool $reinitializeContainerBeforeEachTest = true;
81-
8277
public static function dataAddNull(): array
8378
{
8479
return [

0 commit comments

Comments
 (0)