From 411b9954b53c4e8225802826af75ff78721ba2ab Mon Sep 17 00:00:00 2001 From: nuno maduro Date: Wed, 5 Aug 2026 16:18:03 +0100 Subject: [PATCH] wip --- .../Parallel/Paratest/WrapperRunner.php | 24 +++ src/Plugins/Tia.php | 170 +++++++++++++++--- src/Plugins/Tia/Graph.php | 46 ++++- 3 files changed, 218 insertions(+), 22 deletions(-) diff --git a/src/Plugins/Parallel/Paratest/WrapperRunner.php b/src/Plugins/Parallel/Paratest/WrapperRunner.php index 4638fefe..283cce8c 100644 --- a/src/Plugins/Parallel/Paratest/WrapperRunner.php +++ b/src/Plugins/Parallel/Paratest/WrapperRunner.php @@ -14,6 +14,7 @@ use ParaTest\RunnerInterface; use ParaTest\WrapperRunner\MissingResultsException; use ParaTest\WrapperRunner\SuiteLoader; use ParaTest\WrapperRunner\WrapperWorker; +use Pest\Plugins\Tia; use Pest\Result; use Pest\TestSuite; use PHPUnit\Event\Facade as EventFacade; @@ -155,6 +156,7 @@ final class WrapperRunner implements RunnerInterface /** @var array $parameters */ $parameters = $this->handleLaravelHerd($parameters); + $parameters = $this->handleTia($parameters); $parameters[] = $wrapper; $parameters[] = '--test-directory='.TestSuite::getInstance()->testPath; @@ -202,6 +204,28 @@ final class WrapperRunner implements RunnerInterface return $parameters; } + /** + * Widens pcov's instrumentation scope to the whole project for workers that + * record TIA edges. + * + * pcov's default scope is a single source directory it auto-detects, so + * `config/`, `routes/`, `bootstrap/` and every test's own file never reach + * the recorder — a worker-recorded graph selects a fraction of what a + * sequential one does. `pcov.directory` is only settable at startup, hence + * the command line rather than an `ini_set()` inside the worker. + * + * @param array $parameters + * @return array + */ + private function handleTia(array $parameters): array + { + if (! Tia::recordsEdgesInWorkers()) { + return $parameters; + } + + return array_merge($parameters, ['-d', 'pcov.directory='.TestSuite::getInstance()->rootPath]); + } + private function startWorkers(): void { for ($token = 1; $token <= $this->options->processes; $token++) { diff --git a/src/Plugins/Tia.php b/src/Plugins/Tia.php index 694fd06b..23b5c44d 100644 --- a/src/Plugins/Tia.php +++ b/src/Plugins/Tia.php @@ -111,12 +111,35 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument '--include-path', '--whitelist', '--log-junit', '--log-teamcity', '--testdox-html', '--testdox-text', '--coverage-clover', '--coverage-cobertura', '--coverage-crap4j', - '--coverage-html', '--coverage-php', '--coverage-text', '--coverage-xml', + '--coverage-html', '--coverage-openclover', '--coverage-php', + '--coverage-text', '--coverage-xml', '--coverage-filter', '--path-coverage', '--repeat', '--retry-times', '--memory-limit', '--seed', '--compact', '--ci-build-id', '--min', ]; + /** + * PHPUnit flags that make this run produce a coverage report. + * + * Pest's own `--coverage` is tracked by the Coverage plugin, but a raw + * PHPUnit report flag never reaches it. A run that reports coverage must + * not be narrowed to the affected tests — the report would then describe a + * subset of the suite — and must let PHPUnit own the coverage driver rather + * than have the TIA recorder clear it mid-collection. + * + * Flags that only shape collection or an existing report — `--coverage-filter`, + * `--path-coverage`, `--warm-coverage-cache`, `--only-summary-for-coverage-text`, + * `--show-uncovered-for-coverage-text`, `--disable-coverage-ignore` — produce no + * report on their own, so they are deliberately absent. + * + * @var list + */ + private const array COVERAGE_REPORT_FLAGS = [ + '--coverage-clover', '--coverage-cobertura', '--coverage-crap4j', + '--coverage-html', '--coverage-openclover', '--coverage-php', + '--coverage-text', '--coverage-xml', + ]; + /** * Flags that narrow this run to a subset of the suite. * @@ -158,6 +181,17 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument /** @var array */ private array $cachedAssertionsByTestId = []; + /** + * Recorded durations of the tests this run replayed rather than executed. + * + * A replayed test never runs, so the duration PHPUnit reports for it is the + * cost of replaying it — near zero. Writing that back would decay every + * cached timing toward zero one run at a time. + * + * @var array + */ + private array $cachedTimeByTestId = []; + private ?Graph $replayGraph = null; /** @@ -297,6 +331,25 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument return ! self::argumentPresent('--ci', $arguments); } + /** + * Whether the workers of this run record their own coverage edges. + * + * Stamped by the parent before paratest spawns anything, because a worker + * cannot tell on its own: its argv carries no `--tia`, and the restarters + * run before `tests/Pest.php` is loaded, so {@see self::isEnabledForRun()} + * sees an empty {@see WatchPatterns} too. Left unanswered, pcov keeps its + * default scope — a single auto-detected source directory — and every edge + * outside it, test self-edges included, is silently dropped. + * + * Piggyback runs are excluded: their edges come from PHPUnit's own coverage + * session, so widening pcov there costs time and buys nothing. + */ + public static function recordsEdgesInWorkers(): bool + { + return (string) Parallel::getGlobal(self::RECORDING_GLOBAL) === '1' + && (string) Parallel::getGlobal(self::PIGGYBACK_COVERAGE_GLOBAL) !== '1'; + } + /** * @param array $arguments */ @@ -375,6 +428,12 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument $this->replayedCount++; $assertions = $this->replayGraph->getAssertions($this->branch, $testId); $this->cachedAssertionsByTestId[$testId] = $assertions ?? 0; + + $time = $this->replayGraph->getTime($this->branch, $testId); + + if ($time !== null) { + $this->cachedTimeByTestId[$testId] = $time; + } } else { $this->executedCount++; } @@ -574,7 +633,7 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument $this->branch, $changedFiles->snapshotTree($changedFiles->since($currentSha) ?? []), ); - $graph->replaceEdges($perTest); + $graph->replaceEdges($perTest, keepExisting: $this->piggybackCoverage); $graph->replaceTestTables($perTestTables); $graph->replaceTestInertiaComponents($perTestInertia); $graph->replaceJsFileToComponents(JsModuleGraph::build($projectRoot)); @@ -690,7 +749,7 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument return $exitCode; } - $graph->replaceEdges($finalised); + $graph->replaceEdges($finalised, keepExisting: $this->piggybackCoverage); $graph->replaceTestTables($finalisedTables); $graph->replaceTestInertiaComponents($finalisedInertia); $graph->replaceJsFileToComponents(JsModuleGraph::build($projectRoot)); @@ -816,13 +875,20 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument } } - if ($this->piggybackCoverage) { + // Both of these belong to the coverage cache, which only Pest's own + // `--coverage` ever writes or merges. A raw PHPUnit report flag takes + // the piggyback path — it must not drive the driver itself — but must + // not leave a marker behind, nor force a recording run to prime a cache + // that nothing on its path will fill. + $coverageCacheOwned = $this->piggybackCoverage && $this->pestCoverageActive(); + + if ($coverageCacheOwned) { $this->state->write(self::KEY_COVERAGE_MARKER, ''); } - if ($this->piggybackCoverage && ! $this->state->exists(self::KEY_COVERAGE_CACHE)) { + if ($coverageCacheOwned && ! $this->state->exists(self::KEY_COVERAGE_CACHE)) { if ($graph instanceof Graph && $this->driftLabel === null) { - $this->freshGraphReason = 'recording coverage baseline'; + $this->freshGraphReason = 'recording a coverage baseline'; } return $this->enterRecordMode($arguments); @@ -1018,7 +1084,15 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument if (! Parallel::isEnabled()) { if ($canRefreshReplayEdges) { - $this->recorder->activate(); + // Piggyback runs read PHPUnit's own coverage session. Driving + // the driver alongside it would clear the data PHPUnit is about + // to read, so only link tracking may run here. + if ($this->piggybackCoverage) { + $this->recorder->activateLinkTracking(); + } else { + $this->recorder->activate(); + } + $this->recordingActive = true; } @@ -1037,6 +1111,10 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument if ($canRefreshReplayEdges) { Parallel::setGlobal(self::RECORDING_GLOBAL, '1'); + + if ($this->piggybackCoverage) { + Parallel::setGlobal(self::PIGGYBACK_COVERAGE_GLOBAL, '1'); + } } if ($this->filteredMode) { @@ -1178,6 +1256,16 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument $recorder->activate(); $this->recordingActive = true; + // Why this run is rebuilding is worth saying whenever there is a reason + // for it — the parallel and piggyback branches above already do. Runs + // that are simply recording for the first time have nothing to explain. + if ($this->driftLabel !== null || $this->freshGraphReason !== null) { + $this->output->writeln(''); + $this->renderFreshGraph(); + + return $arguments; + } + $this->renderChild('Running in TIA mode.'); return $arguments; @@ -1185,14 +1273,18 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument private function renderFreshGraph(): void { - $headline = 'Experimental TIA mode enabled / fresh graph'; - - if ($this->driftLabel !== null) { - $headline .= sprintf(' (%s changed)', $this->driftLabel); - } elseif ($this->freshGraphReason !== null) { - $headline .= sprintf(' (%s)', $this->freshGraphReason); + if ($this->driftLabel === null && $this->freshGraphReason !== null) { + // The reason is only ever set for a run that keeps its graph and + // records alongside it, so "fresh graph" would be a lie here. + $headline = sprintf('Experimental TIA mode enabled / %s.', $this->freshGraphReason); } else { - $headline .= '.'; + $headline = 'Experimental TIA mode enabled / fresh graph'; + + if ($this->driftLabel !== null) { + $headline .= sprintf(' (%s changed)', $this->driftLabel); + } else { + $headline .= '.'; + } } $this->renderChild($headline); @@ -1553,6 +1645,15 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument return $coverage; } + /** + * The duration to record for a test: its own, unless it was replayed rather + * than executed, in which case the duration it was recorded with stands. + */ + private function resultTime(string $testId, float $time): float + { + return $this->cachedTimeByTestId[$testId] ?? $time; + } + private function seedResultsInto(Graph $graph): void { /** @var ResultCollector $collector */ @@ -1577,7 +1678,7 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument $testId, $result['status'], $result['message'], - $result['time'], + $this->resultTime($testId, $result['time']), $result['assertions'], $file, ); @@ -1639,10 +1740,11 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument // A result is only ever invalidated through the edges of the test // that produced it, so one recorded for a test the graph has no // edges for could never be invalidated again — it would be replayed - // as settled however far the code around it moved. Only a complete - // run records the edges that would close that gap, so until one - // does, the test stays unknown. - if (! $complete && (! is_string($file) || ! $graph->knowsTest($file))) { + // as settled however far the code around it moved. Only a run that + // records edges closes that gap, and marking known test files is + // what says this run did; a complete run that recorded none leaves + // the test just as unknown as a partial one does. + if ((! $complete || ! $markKnownTestFiles) && (! is_string($file) || ! $graph->knowsTest($file))) { continue; } @@ -1651,7 +1753,7 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument $testId, $result['status'], $result['message'], - $result['time'], + $this->resultTime($testId, $result['time']), $result['assertions'], $file, ); @@ -1702,7 +1804,35 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument return null; } + /** + * Whether this run produces a coverage report, however it was asked for. + * + * The original argv, not the handled arguments: Pest's own Coverage plugin + * appends `--coverage-php ` to those and runs before this one, and a + * paratest worker's arguments always carry it too. `bin/worker.php` never + * hands over the original argv, so a worker sees `[]` here and keeps taking + * this from {@see self::PIGGYBACK_COVERAGE_GLOBAL} instead. + */ private function coverageReportActive(): bool + { + if ($this->pestCoverageActive()) { + return true; + } + + foreach (self::COVERAGE_REPORT_FLAGS as $flag) { + if ($this->hasArgument($flag, $this->originalArguments)) { + return true; + } + } + + return false; + } + + /** + * Whether Pest's own `--coverage` was given — the only entry point that + * writes the coverage cache these two flags read and clean up. + */ + private function pestCoverageActive(): bool { $coverage = Container::getInstance()->get(Coverage::class); assert($coverage instanceof Coverage); diff --git a/src/Plugins/Tia/Graph.php b/src/Plugins/Tia/Graph.php index 420fe54a..14d077d8 100644 --- a/src/Plugins/Tia/Graph.php +++ b/src/Plugins/Tia/Graph.php @@ -622,6 +622,17 @@ final class Graph return $baseline['results'][$testId]['assertions']; } + public function getTime(string $branch, string $testId, string $fallbackBranch = 'main'): ?float + { + $baseline = $this->baselineFor($branch, $fallbackBranch); + + if (! isset($baseline['results'][$testId]['time'])) { + return null; + } + + return $baseline['results'][$testId]['time']; + } + public function getResult(string $branch, string $testId, string $fallbackBranch = 'main'): ?TestStatus { $baseline = $this->baselineFor($branch, $fallbackBranch); @@ -677,6 +688,15 @@ final class Graph return array_keys($files); } + /** + * Whether any cached result due a re-run points at a test file that is not + * on disk — deleted, or never locatable in the first place (`eval()`'d code, + * a path outside the project). + * + * A filtered run cannot honour such an entry: it would select a file that + * collects no tests, so the run reports green without ever re-running the + * failure — and does so again on every subsequent invocation. + */ public function hasUnlocatedTestsToRerun(string $branch, string $fallbackBranch = 'main'): bool { $baseline = $this->baselineFor($branch, $fallbackBranch); @@ -688,7 +708,16 @@ final class Graph $file = $result['file'] ?? null; - if ($file === null || $file === '' || $this->relative($file) === null) { + if ($file === null || $file === '') { + return true; + } + + $rel = $this->relative($file); + + // Results are stored relative, so `relative()` answers "is this + // inside the project" without ever touching the filesystem. The + // stat is what tells a deleted test file apart from a live one. + if ($rel === null || ! is_file($this->projectRoot.'/'.$rel)) { return true; } } @@ -809,8 +838,15 @@ final class Graph /** * @param array> $testToFiles + * @param bool $keepExisting Leave already-recorded edge sets alone. For runs + * whose edges are piggybacked off a PHPUnit coverage + * session: that data is scoped by ``, so it + * can only ever be narrower than what the TIA + * recorder sees — it never contains the test's own + * file, for one — and a narrower edge set silently + * stops selecting the tests it used to select. */ - public function replaceEdges(array $testToFiles): void + public function replaceEdges(array $testToFiles, bool $keepExisting = false): void { foreach ($testToFiles as $testFile => $sources) { $testRel = $this->relative($testFile); @@ -819,6 +855,12 @@ final class Graph continue; } + // An empty set means "known, covers nothing", so piggyback data is + // still an improvement there — only a populated set is protected. + if ($keepExisting && ($this->edges[$testRel] ?? []) !== []) { + continue; + } + $this->edges[$testRel] = []; foreach ($sources as $source) {