Skip to content

Commit 9d5f507

Browse files
sebastianbergmannsebastianbergmann
committed
Run a repeated or retried PHPT test as one PHPT unit rather than as a standalone unit at its suite index, so that --parallel does not serialize every PHPT test when --repeat or --retry is used
1 parent b43f4c0 commit 9d5f507

16 files changed

Lines changed: 533 additions & 24 deletions

‎src/Event/CollectingEmitter.php‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
*
2424
* @internal This class is not covered by the backward compatibility promise for PHPUnit
2525
*/
26-
final readonly class CollectingEmitter
26+
final readonly class CollectingEmitter implements EventCollector
2727
{
2828
private Emitter $emitter;
2929
private CollectingDispatcher $dispatcher;
@@ -43,4 +43,32 @@ public function flush(): EventCollection
4343
{
4444
return $this->dispatcher->flush();
4545
}
46+
47+
/**
48+
* @throws EventsAreAlreadyBeingCollectedException
49+
*/
50+
public function startCollectingEvents(): void
51+
{
52+
$this->dispatcher->startCollectingEvents();
53+
}
54+
55+
/**
56+
* @throws EventsAreNotBeingCollectedException
57+
*/
58+
public function stopCollectingEvents(): EventCollection
59+
{
60+
return $this->dispatcher->stopCollectingEvents();
61+
}
62+
63+
/**
64+
* Add the given events to the collection, as if they had been emitted
65+
* through this emitter: this is how the events of a run that was diverted
66+
* through a collection window become part of the collection after all.
67+
*/
68+
public function forward(EventCollection $events): void
69+
{
70+
foreach ($events as $event) {
71+
$this->dispatcher->dispatch($event);
72+
}
73+
}
4674
}

‎src/Runner/Parallel/PhptRunner.php‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
use PHPUnit\Event\CollectingEmitter;
1919
use PHPUnit\Event\EventCollection;
2020
use PHPUnit\Event\Facade as EventFacade;
21+
use PHPUnit\Framework\PhptRepeatTestSuite;
22+
use PHPUnit\Framework\PhptRetryTestSuite;
2123
use PHPUnit\Runner\Phpt\Interruption;
2224
use PHPUnit\Runner\Phpt\TestCase as PhptTestCase;
2325
use PHPUnit\Util\PHP\Job;
@@ -337,7 +339,7 @@ private function startRunnable(): bool
337339

338340
$collector = EventFacade::instance()->collectingEmitter();
339341
$interruption = new Interruption;
340-
$generator = new PhptTestCase($unit->file())->execute($collector->emitter(), $interruption);
342+
$generator = $this->generatorFor($unit, $collector, $interruption);
341343

342344
$generator->rewind();
343345

@@ -367,6 +369,35 @@ private function startRunnable(): bool
367369
return $progressed;
368370
}
369371

372+
/**
373+
* The generator that advances the unit: the sections of a single PHPT
374+
* test, or the repetitions or attempts of a repeated or retried one, which
375+
* the suite that aggregates them orchestrates as one generator so that
376+
* they run one after another within the unit.
377+
*
378+
* @return Generator<int, Job, Result, void>
379+
*/
380+
private function generatorFor(PhptWorkUnit $unit, CollectingEmitter $collector, Interruption $interruption): Generator
381+
{
382+
if ($unit->numberOfRuns() > 1) {
383+
return PhptRepeatTestSuite::for($unit->file(), $unit->numberOfRuns())->executeInterleaved(
384+
$collector->emitter(),
385+
$collector,
386+
$interruption,
387+
);
388+
}
389+
390+
if ($unit->maxAttempts() > 1) {
391+
return PhptRetryTestSuite::for($unit->file(), $unit->maxAttempts())->executeInterleaved(
392+
$collector->emitter(),
393+
$collector,
394+
$interruption,
395+
);
396+
}
397+
398+
return new PhptTestCase($unit->file())->execute($collector->emitter(), $interruption);
399+
}
400+
370401
/**
371402
* Whether the unit may be started right now: a unit that conflicts with
372403
* "all" may start only when nothing else is running, and any other unit may

‎src/Runner/Parallel/PhptWorkUnit.php‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@
1818
* file. A PHPT test is not a PHPUnit\Framework\TestCase and carries no test
1919
* data, so the worker reconstructs it from nothing more than this file path.
2020
*
21+
* A repeated or retried PHPT test is one unit as well: its repetitions and its
22+
* attempts must run one after another and are orchestrated by the suite that
23+
* aggregates them, which the unit carries the shape of so that the runner can
24+
* reconstruct that suite.
25+
*
2126
* @no-named-arguments Parameter names are not covered by the backward compatibility promise for PHPUnit
2227
*
2328
* @internal This class is not covered by the backward compatibility promise for PHPUnit
@@ -39,16 +44,52 @@
3944
*/
4045
private array $conflicts;
4146

47+
/**
48+
* @var positive-int
49+
*/
50+
private int $numberOfRuns;
51+
52+
/**
53+
* @var positive-int
54+
*/
55+
private int $maxAttempts;
56+
4257
/**
4358
* @param non-negative-int $index
4459
* @param non-empty-string $file
4560
* @param list<non-empty-string> $conflicts
61+
* @param positive-int $numberOfRuns
62+
* @param positive-int $maxAttempts
63+
*/
64+
public function __construct(int $index, string $file, array $conflicts = [], int $numberOfRuns = 1, int $maxAttempts = 1)
65+
{
66+
$this->index = $index;
67+
$this->file = $file;
68+
$this->conflicts = $conflicts;
69+
$this->numberOfRuns = $numberOfRuns;
70+
$this->maxAttempts = $maxAttempts;
71+
}
72+
73+
/**
74+
* How often the test is run in a row, each run reported on its own; more
75+
* than once when --repeat was used.
76+
*
77+
* @return positive-int
78+
*/
79+
public function numberOfRuns(): int
80+
{
81+
return $this->numberOfRuns;
82+
}
83+
84+
/**
85+
* How often the test may be attempted before a failure is reported as its
86+
* result; more than once when --retry was used.
87+
*
88+
* @return positive-int
4689
*/
47-
public function __construct(int $index, string $file, array $conflicts = [])
90+
public function maxAttempts(): int
4891
{
49-
$this->index = $index;
50-
$this->file = $file;
51-
$this->conflicts = $conflicts;
92+
return $this->maxAttempts;
5293
}
5394

5495
/**

‎src/TextUI/ParallelTestRunner.php‎

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
use PHPUnit\Framework\DataProviderTestSuite;
2727
use PHPUnit\Framework\IterativeTestSuite;
2828
use PHPUnit\Framework\PhptIterativeTestSuite;
29+
use PHPUnit\Framework\PhptRepeatTestSuite;
30+
use PHPUnit\Framework\PhptRetryTestSuite;
2931
use PHPUnit\Framework\Test;
3032
use PHPUnit\Framework\TestCase;
3133
use PHPUnit\Framework\TestRunner\ChildProcessResultProcessor;
@@ -896,7 +898,7 @@ private function collectUnits(TestSuite $suite, int &$index): array
896898
/** @var array<class-string<TestCase>, array{index: non-negative-int, tests: list<DataProviderTestSuite|IterativeTestSuite|TestCase>}> $byClass */
897899
$byClass = [];
898900

899-
/** @var list<array{index: non-negative-int, file: non-empty-string, conflicts: list<non-empty-string>}> $phpt */
901+
/** @var list<array{index: non-negative-int, file: non-empty-string, conflicts: list<non-empty-string>, numberOfRuns: positive-int, maxAttempts: positive-int}> $phpt */
900902
$phpt = [];
901903

902904
/** @var list<array{index: non-negative-int, test: Test}> $standalone */
@@ -913,7 +915,13 @@ private function collectUnits(TestSuite $suite, int &$index): array
913915
$phptUnits = [];
914916

915917
foreach ($phpt as $item) {
916-
$phptUnits[] = new PhptWorkUnit($item['index'], $item['file'], $item['conflicts']);
918+
$phptUnits[] = new PhptWorkUnit(
919+
$item['index'],
920+
$item['file'],
921+
$item['conflicts'],
922+
$item['numberOfRuns'],
923+
$item['maxAttempts'],
924+
);
917925
}
918926

919927
return [
@@ -924,10 +932,10 @@ private function collectUnits(TestSuite $suite, int &$index): array
924932
}
925933

926934
/**
927-
* @param array<class-string<TestCase>, array{index: non-negative-int, tests: list<DataProviderTestSuite|IterativeTestSuite|TestCase>}> $byClass
928-
* @param list<array{index: non-negative-int, file: non-empty-string, conflicts: list<non-empty-string>}> $phpt
929-
* @param list<array{index: non-negative-int, test: Test}> $standalone
930-
* @param non-negative-int $index
935+
* @param array<class-string<TestCase>, array{index: non-negative-int, tests: list<DataProviderTestSuite|IterativeTestSuite|TestCase>}> $byClass
936+
* @param list<array{index: non-negative-int, file: non-empty-string, conflicts: list<non-empty-string>, numberOfRuns: positive-int, maxAttempts: positive-int}> $phpt
937+
* @param list<array{index: non-negative-int, test: Test}> $standalone
938+
* @param non-negative-int $index
931939
*/
932940
private function collect(TestSuite $suite, array &$byClass, array &$phpt, array &$standalone, int &$index): void
933941
{
@@ -941,13 +949,32 @@ private function collect(TestSuite $suite, array &$byClass, array &$phpt, array
941949
}
942950

943951
// The repetitions of a repeated PHPT test and the attempts of a
944-
// retried PHPT test are orchestrated by their suite's runTests()
945-
// method and must run sequentially, so the suite runs as one unit
946-
// in the main process at its suite index.
952+
// retried PHPT test must run one after another, so the suite that
953+
// orchestrates them becomes one PHPT unit rather than one unit per
954+
// run: the runner rebuilds the suite from the unit and advances it
955+
// as a whole, alongside the other PHPT tests.
947956
if ($test instanceof PhptIterativeTestSuite) {
948-
$standalone[] = [
949-
'index' => $index,
950-
'test' => $test,
957+
$file = $test->name();
958+
959+
assert($file !== '');
960+
961+
$numberOfRuns = 1;
962+
$maxAttempts = 1;
963+
964+
if ($test instanceof PhptRepeatTestSuite) {
965+
$numberOfRuns = $test->numberOfRuns();
966+
} else {
967+
assert($test instanceof PhptRetryTestSuite);
968+
969+
$maxAttempts = $test->maxAttempts();
970+
}
971+
972+
$phpt[] = [
973+
'index' => $index,
974+
'file' => $file,
975+
'conflicts' => $this->phptConflicts($file),
976+
'numberOfRuns' => $numberOfRuns,
977+
'maxAttempts' => $maxAttempts,
951978
];
952979

953980
$index++;
@@ -1008,9 +1035,11 @@ private function collect(TestSuite $suite, array &$byClass, array &$phpt, array
10081035
// --CONFLICTS-- section. The runner honours those conflict keys
10091036
// while running the PHPT tests concurrently in the main process.
10101037
$phpt[] = [
1011-
'index' => $index,
1012-
'file' => $file,
1013-
'conflicts' => $this->phptConflicts($file),
1038+
'index' => $index,
1039+
'file' => $file,
1040+
'conflicts' => $this->phptConflicts($file),
1041+
'numberOfRuns' => 1,
1042+
'maxAttempts' => 1,
10141043
];
10151044

10161045
$index++;

‎tests/_files/parallel-worker/worker-failing.phpt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
--TEST--
2+
A PHPT test that always fails, for the tests of the repeated and retried PHPT units
3+
--FILE--
4+
<?php declare(strict_types=1);
5+
print 'the phpt test failed in a worker';
6+
--EXPECT--
7+
the phpt test ran in a worker

‎tests/_files/parallel-worker/worker-flaky.phpt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
A PHPT test that fails on its first attempt and passes on its second, for the tests of the retried PHPT unit
3+
--FILE--
4+
<?php declare(strict_types=1);
5+
$marker = sys_get_temp_dir() . '/phpunit-parallel-phpt-retry.marker';
6+
7+
if (@file_get_contents($marker) === false) {
8+
file_put_contents($marker, '1');
9+
10+
print 'the phpt test failed in a worker';
11+
} else {
12+
@unlink($marker);
13+
14+
print 'the phpt test ran in a worker';
15+
}
16+
--EXPECT--
17+
the phpt test ran in a worker

‎tests/end-to-end/parallel/repeat-retry/_files/RetryFlakyPhpt.phpt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
PHPT that fails on the first attempt and passes on the second
3+
--FILE--
4+
<?php declare(strict_types=1);
5+
$marker = sys_get_temp_dir() . '/phpunit-parallel-phpt-retry.marker';
6+
7+
if (@file_get_contents($marker) === false) {
8+
file_put_contents($marker, '1');
9+
10+
print 'FAIL';
11+
} else {
12+
@unlink($marker);
13+
14+
print 'OK';
15+
}
16+
--EXPECT--
17+
OK

‎tests/end-to-end/parallel/repeat-retry/_files/repeat-interval-a.phpt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
PHPT fixture that records the interval of each of its runs, for the concurrency test of the repeated PHPT tests (a)
3+
--FILE--
4+
<?php declare(strict_types=1);
5+
$start = microtime(true);
6+
7+
usleep(300000);
8+
9+
file_put_contents(
10+
sys_get_temp_dir() . '/phpunit-parallel-repeat-interval-a.intervals',
11+
$start . ' ' . microtime(true) . PHP_EOL,
12+
FILE_APPEND,
13+
);
14+
15+
print 'ok';
16+
--EXPECT--
17+
ok

‎tests/end-to-end/parallel/repeat-retry/_files/repeat-interval-b.phpt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
PHPT fixture that records the interval of each of its runs, for the concurrency test of the repeated PHPT tests (b)
3+
--FILE--
4+
<?php declare(strict_types=1);
5+
$start = microtime(true);
6+
7+
usleep(300000);
8+
9+
file_put_contents(
10+
sys_get_temp_dir() . '/phpunit-parallel-repeat-interval-b.intervals',
11+
$start . ' ' . microtime(true) . PHP_EOL,
12+
FILE_APPEND,
13+
);
14+
15+
print 'ok';
16+
--EXPECT--
17+
ok

0 commit comments

Comments
 (0)