From 4d3d0105b73173f57241647d994b0e127a0c5191 Mon Sep 17 00:00:00 2001 From: nuno maduro Date: Thu, 6 Aug 2026 16:02:36 +0100 Subject: [PATCH] chore: style --- rector.php | 2 - src/Concerns/Testable.php | 3 - .../Parallel/Paratest/WrapperRunner.php | 9 - src/Plugins/Tia.php | 317 ++---------------- src/Plugins/Tia/ChangedFiles.php | 20 -- src/Plugins/Tia/CiDefaultBranch.php | 18 - src/Plugins/Tia/Configuration.php | 6 - src/Plugins/Tia/Graph.php | 38 +-- tests/Features/Tia.php | 2 +- tests/Features/Tia/DefaultBranchReplay.php | 17 - .../Features/Tia/DefaultBranchResolution.php | 32 -- tests/Features/Tia/DefaultBranchWriteTier.php | 18 - tests/Fixtures/Suites/TiaReplayHooks.xml | 17 + tests/Fixtures/Tia/GitRepo.php | 34 -- tests/Fixtures/Tia/GraphDelta.php | 44 --- tests/Fixtures/Tia/PestResult.php | 31 +- tests/Fixtures/Tia/Project.php | 155 --------- .../tests/Feature/CoversCalculatorTest.php | 2 - tests/Fixtures/Tia/app/tests/Pest.php | 3 - .../Tia/app/tests/Unit/CalculatorTest.php | 3 - .../configured-default-branch/tests/Pest.php | 3 - .../unknown-default-branch/tests/Pest.php | 2 - tests/Pest.php | 2 - tests/Unit/Plugins/Tia/Graph.php | 2 - 24 files changed, 52 insertions(+), 728 deletions(-) create mode 100644 tests/Fixtures/Suites/TiaReplayHooks.xml diff --git a/rector.php b/rector.php index d3fc637a..90c46ea2 100644 --- a/rector.php +++ b/rector.php @@ -28,8 +28,6 @@ return RectorConfig::configure() ->withSkip([ __DIR__.'/src/Plugins/Parallel/Paratest/WrapperRunner.php', __DIR__.'/tests/Fixtures/Arch', - // Fixture suites are pinned by the TeamCity / JUnit snapshots, down to - // the line numbers — rewriting their source would break them. __DIR__.'/tests/Fixtures/Suites', ReturnNeverTypeRector::class, ArrowFunctionDelegatingCallToFirstClassCallableRector::class, diff --git a/src/Concerns/Testable.php b/src/Concerns/Testable.php index 93ae3af6..b34e7a19 100644 --- a/src/Concerns/Testable.php +++ b/src/Concerns/Testable.php @@ -286,9 +286,6 @@ trait Testable if ($replay !== ReplayType::None) { assert($status !== null); - // Marks the replay before the branches below throw, so `tearDown` - // short-circuits for every replayed result — the throwing branches - // never reach `parent::setUp`, so no user hook may run after them. $this->__replay = $replay; match ($replay) { diff --git a/src/Plugins/Parallel/Paratest/WrapperRunner.php b/src/Plugins/Parallel/Paratest/WrapperRunner.php index 283cce8c..db76ea00 100644 --- a/src/Plugins/Parallel/Paratest/WrapperRunner.php +++ b/src/Plugins/Parallel/Paratest/WrapperRunner.php @@ -205,15 +205,6 @@ final class WrapperRunner implements RunnerInterface } /** - * 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 */ diff --git a/src/Plugins/Tia.php b/src/Plugins/Tia.php index 3032be6c..bf632551 100644 --- a/src/Plugins/Tia.php +++ b/src/Plugins/Tia.php @@ -63,11 +63,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument private const string BASELINE_PATH_OPTION = '--baseline'; - /** - * Set by the mutation plugin on the subprocess running a single mutant, - * and nowhere else. Its own `--mutate` flag is popped before the argv is - * handed to that subprocess, so the flag cannot be matched instead. - */ private const string ENV_MUTATION_TESTING = 'PEST_MUTATION_TESTING'; private const string ENV_TIA = 'PEST_TIA'; @@ -102,19 +97,8 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument private const string PIGGYBACK_COVERAGE_GLOBAL = 'TIA_PIGGYBACK_COVERAGE'; - /** - * The parent's resolved fallback branch, handed to the workers. - * - * A worker cannot resolve it for itself: the restarters run before - * `tests/Pest.php` is loaded, so a `defaultBranch()` declared there is - * invisible to it — and autodetecting again would spend a git call per - * worker to reach the answer the parent already has. - */ private const string FALLBACK_BRANCH_GLOBAL = 'TIA_FALLBACK_BRANCH'; - /** - * The branch assumed when a repository cannot name its own default. - */ private const string DEFAULT_BRANCH = 'main'; /** @@ -138,19 +122,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument ]; /** - * 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 = [ @@ -160,24 +131,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument ]; /** - * Flags that narrow this run to a subset of the suite. - * - * Only user-supplied, per-run narrowing belongs here. A filter that is - * always in force — `` in phpunit.xml, or a plugin registering a - * test case filter from `boot()` — applies equally to the runs that build - * the baseline, so it does not make this run narrower than the baseline - * and must not disable baseline writes. - * - * `--shard` is rewritten to `--filter` before this plugin sees the - * arguments, so it is covered here too. The `bin/pest`-only flags are - * stripped from the handled arguments, so they are matched against the - * original argv instead. - * - * Flags that cut a run short instead of narrowing it — `--bail`, `--retry`, - * `--stop-on-*` — do not belong here either. They only narrow the run when - * something actually fails, and that is not known until it is over, so they - * are handled by stoppedEarly() from addOutput(). - * * @var list */ private const array PARTIAL_SELECTION_FLAGS = [ @@ -187,15 +140,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument '--assignee', '--issue', '--ticket', '--pr', '--pull-request', ]; - /** - * Options that cannot be combined with Tia mode. - * - * `--covers` and `--uses` select on coverage metadata Tia does not model, - * so they resolve to no tests at all rather than to the ones the user meant. - * `--random-order-seed` exits non-zero on its own, with or without Tia. - * Either way the run cannot honour both things it was asked for, so it says - * so instead of silently dropping Tia and running something else. - */ private const array UNSUPPORTED_OPTIONS = [ '--covers', '--uses', '--random-order-seed', ]; @@ -214,46 +158,16 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument 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; - /** - * The baseline this run reads from and writes to. - * - * The repository's default branch is only the fallback for a checkout whose - * branch cannot be read — a detached HEAD. It is also the branch every - * other baseline falls back to reading, so writing there by accident - * corrupts the shared baseline. Resolved through resolveBranch() rather - * than at every use site, because the git call it needs is not free. - */ private string $branch = self::DEFAULT_BRANCH; - /** - * The baseline branches with none of their own read from. - * - * Read-only, and the whole point of the exercise: without it the first run - * on every new branch re-runs a suite whose results the default branch - * already holds. - */ private string $fallbackBranch = self::DEFAULT_BRANCH; - /** - * Whether anything actually named the branch above. - * - * When nothing did, the value is a guess, and a guess is what the TIA path - * refuses to run on: an unresolved fallback reads no baseline at all, which - * looks exactly like a hit in the output. Runs that never asked for TIA - * still have to write somewhere, so the guess stands for them. - */ private bool $fallbackBranchResolved = false; private bool $branchResolved = false; @@ -276,22 +190,8 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument private bool $filteredMode = false; - /** - * Bars this run from touching the graph at all, results included. - * - * Reserved for runs whose results describe something other than the code - * in the working tree, which is nothing the baseline can ever use. - */ private bool $writesSuppressed = false; - /** - * Narrows this run's writes to the results of the tests it actually ran. - * - * A run that covered only part of the suite still learns something true - * about the tests it did reach. What it cannot do is speak for the rest: - * pruning results, advancing the recorded sha and replacing the edge map - * all claim the whole suite reported, so they stay behind a complete run. - */ private bool $resultsOnlyWrites = false; /** @var array */ @@ -342,8 +242,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument $graph = Graph::decode($json, $projectRoot); - // Every read of a baseline goes through a graph loaded here, so this is - // the one place the resolved fallback has to reach. $graph?->setFallbackBranch($this->fallbackBranch); return $graph; @@ -388,19 +286,6 @@ 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' @@ -541,11 +426,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument $partial = ! $isWorker && ($hasExplicitPath || $this->hasPartialSelection($arguments)); $disabled = $disabled || $partial; - // A mutation subprocess runs the suite against source the mutation - // plugin has deliberately broken. Its failures describe the mutant, not - // the working tree, so unlike every other narrowed run there is nothing - // in its results worth keeping. The parent `--mutate` run is untouched - // by this: it runs the whole suite against real source. if (getenv(self::ENV_MUTATION_TESTING) !== false) { $this->writesSuppressed = true; } @@ -566,18 +446,8 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument if ($disabled) { if ($partial) { - // TIA cannot choose what runs here — the user already did — but - // the tests they picked still report honestly, so their results - // are kept and everything that would speak for the excluded ones - // is not. `--no-tia` needs none of this: it still runs the whole - // suite, so it remains a complete run. $this->resultsOnlyWrites = true; - // `$this->filteredMode` counts as asking for it: reaching here - // means the narrowing came from the command line while filtered - // mode came from the environment or the config, and a run that - // silently declines what the config asked for is the one most - // in need of the explanation. if ($cliEnabled || $freshRequested || $this->forceRefetch || $this->filteredMode) { $this->output->writeln(''); $this->renderChild('TIA does not apply to partial runs — running the selected tests directly.'); @@ -621,11 +491,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument $this->flushWorkerReplay(); } - // Both only ever set for the parent — addOutput() returns early in - // workers, whose partials are ephemeral and only reach the baseline if - // the parent consumes them. Everything this method goes on to write is - // whole-suite by nature — the edge map above all — so a narrowed run - // stops here too, its results already persisted by addOutput(). if ($this->writesSuppressed || $this->resultsOnlyWrites) { $this->recorder->reset(); $this->coverageCollector->reset(); @@ -722,20 +587,12 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument return $exitCode; } - // `->only()` narrows the executed set exactly like `--filter` does, but - // is only knowable once the suite has been collected — too late to turn - // TIA off up front. Sampled in addOutput() because Only's lock file is - // already gone by the time terminate() runs (its plugin terminates - // first). Whether the run was cut short is likewise only knowable now. if (Only::isEnabled() || $this->stoppedEarly()) { $this->resultsOnlyWrites = true; } $this->reportMissingWorkerDrivers(); - // Runs before the checks below: it is what fills the parent's result - // collector in parallel, and a worker that stopped early narrows the - // whole run. if (Parallel::isEnabled()) { $this->mergeWorkerReplayPartials(); } @@ -900,12 +757,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument $this->resolveBranch($projectRoot); - // After resolveBranch(), so a directory that is no repository at all - // still reports the missing git dependency rather than an unresolved - // default branch. Nothing named the branch every other baseline reads - // through, so every new branch would re-run the whole suite while the - // output called it a hit. A repository with no remote is the likeliest - // reason and gets said out loud. if (! $this->fallbackBranchResolved) { Panic::with(new ChangedFiles($projectRoot)->hasRemote() ? new TiaRequiresDefaultBranch @@ -948,11 +799,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument } } - // 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) { @@ -1157,9 +1003,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument if (! Parallel::isEnabled()) { if ($canRefreshReplayEdges) { - // 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 { @@ -1329,9 +1172,6 @@ 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(); @@ -1347,8 +1187,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument private function renderFreshGraph(): void { 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 = 'Experimental TIA mode enabled / fresh graph'; @@ -1444,12 +1282,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument return; } - // A replayed result carries the duration PHPUnit measured for a test - // that never ran — near zero. Unlike the assertion count, which the - // replay injects into the result itself, the cached duration lives only - // in this process: the parent replayed nothing of its own, so once the - // partial is written the real value is unrecoverable. Launder it here - // and the parent's verbatim read is correct by construction. foreach ($results as $testId => $result) { $results[$testId]['time'] = $this->resultTime($testId, $result['time']); } @@ -1459,8 +1291,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument 'replayed' => $this->replayedCount, 'affected' => $this->affectedCount, 'executed' => $this->executedCount, - // Only the worker knows it stopped early — the parent runs no tests - // of its own, so its own check would always come back clean. 'truncated' => $this->stoppedEarly(), ], JSON_UNESCAPED_SLASHES); @@ -1498,9 +1328,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument continue; } - // One worker stopping early leaves the whole suite incomplete: the - // tests it never reached are missing from the merged result set just - // as if they had been filtered out. if (($decoded['truncated'] ?? false) === true) { $this->resultsOnlyWrites = true; } @@ -1728,10 +1555,6 @@ 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; @@ -1773,13 +1596,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument $collector->reset(); } - /** - * Folds the run's results into the existing graph. - * - * An incomplete run passes `$complete: false`, which keeps the additive - * half — the results of the tests it did run — and drops the half that - * speaks for the suite as a whole. - */ private function snapshotTestResults(bool $markKnownTestFiles = false, bool $complete = true): void { /** @var ResultCollector $collector */ @@ -1802,23 +1618,12 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument try { $this->resolveBranch($projectRoot); } catch (MissingDependency) { - // This run never asked for TIA, so a missing git must not turn it - // into a failure the way it does on the TIA path. Writing to the - // fallback baseline is the lesser of the two evils. } - // The graph above was loaded before the branch was known — this path - // only writes, but a graph carrying an unresolved fallback is the exact - // bug this whole change is about. $graph->setFallbackBranch($this->fallbackBranch); $touchedFiles = []; - // Whether this run is the one that records the edges its results will be - // invalidated through. A recording run's edges are written after this - // (terminate() runs last), and a parallel one's arrive with the worker - // partials that ask for $markKnownTestFiles — either way the graph on - // disk cannot be asked yet, so the run is taken at its word. $recordsEdges = $complete && ($markKnownTestFiles || $this->recordingActive); foreach ($results as $testId => $result) { @@ -1832,12 +1637,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument $touchedFiles[$file] = true; } - // 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. A run that - // records no edges leaves such a test exactly as unknown as it - // found it, whether or not it ran the whole suite. if (! $recordsEdges && (! is_string($file) || ! $graph->knowsTest($file))) { continue; } @@ -1857,8 +1656,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument $graph->markKnownTestFiles(array_keys($touchedFiles)); } - // Pruning reads the absence of a test from this run as the test being - // gone. That only holds if every test was invited to report. if ($complete) { $graph->pruneStaleResults($this->branch, array_keys($touchedFiles), array_keys($results)); } @@ -1898,15 +1695,6 @@ 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()) { @@ -1916,10 +1704,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument return array_any(self::COVERAGE_REPORT_FLAGS, fn (string $flag): bool => $this->hasArgument($flag, $this->originalArguments)); } - /** - * 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); @@ -1929,11 +1713,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument } /** - * Panics when the run asks for Tia alongside an option Tia cannot honour. - * - * Checked against the original argv as well, because `bin/pest` consumes - * some of these itself before PHPUnit ever sees them. - * * @param array $arguments */ private function guardUnsupportedOptions(array $arguments): void @@ -1952,11 +1731,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument } /** - * Whether a selection-narrowing flag was given, either among the arguments - * PHPUnit receives or — for the flags `bin/pest` consumes itself — among - * the original argv. Explicit path arguments and `->only()` are detected - * separately. - * * @param array $arguments */ private function hasPartialSelection(array $arguments): bool @@ -1974,36 +1748,11 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument return false; } - /** - * Whether the run stopped before reaching every test it had queued. - * - * Covers `--bail`, `--retry` and every `--stop-on-*` flag, the equivalent - * `phpunit.xml` attributes, and an interrupted run — none of which narrow - * the selection up front, so hasPartialSelection() cannot see them. The - * tests queued behind the defect that halted the run never reported, and - * folding what did report into the baseline prunes the cached results of - * their siblings in every file the run had already entered. - * - * Deliberately unguarded. Both callers run only once PHPUnit's - * configuration is registered — the kernel reads it unguarded itself just - * before dispatching addOutput(), and flushWorkerReplay() bails out unless - * the worker actually executed something. Swallowing a failure here would - * report every truncated run as complete, which is the corruption this - * guards against in the first place. - */ private function stoppedEarly(): bool { return TestResultFacade::shouldStop(); } - /** - * Resolves the baselines this run reads from and writes to, once. - * - * Results are written on runs where TIA itself took no part, and those - * never reach handleParent(). Without this the default would stand and - * every such run would write its results to `main`, whatever branch it - * actually ran on. - */ private function resolveBranch(string $projectRoot): void { if ($this->branchResolved) { @@ -2014,9 +1763,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument $changedFiles = new ChangedFiles($projectRoot); - // Resolved before the current branch, which throws where git is - // missing: the fallback is advisory, so a run that cannot name its - // branch at all should still carry the best answer available. $resolved = $this->resolveFallbackBranch($changedFiles); $this->fallbackBranchResolved = $resolved !== null; @@ -2024,24 +1770,9 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument Parallel::setGlobal(self::FALLBACK_BRANCH_GLOBAL, $this->fallbackBranch); - // A detached HEAD has no branch of its own to write to. The default - // branch is the honest key there — it is the commit the checkout most - // likely sits on, and it keeps a phantom baseline from being minted - // under a branch name the repository never had. $this->branch = $changedFiles->currentBranch() ?? $this->fallbackBranch; } - /** - * The branch every other baseline falls back to reading, or null when - * nothing in the checkout can name it. - * - * Ordered by how much the source actually knows. Configuration first: it is - * the escape hatch for a repository whose git-side answers disagree with its - * branches. Then the CI provider, which states the answer outright where git - * is at its least informed. Then git itself. Then the recorded graph, whose - * single baseline can only have come from the branch this repository - * integrates on. - */ private function resolveFallbackBranch(ChangedFiles $changedFiles): ?string { $inherited = Parallel::getGlobal(self::FALLBACK_BRANCH_GLOBAL); @@ -2056,14 +1787,6 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument ?? $this->soleRecordedBranch(); } - /** - * The one branch a recorded graph holds a baseline for. - * - * Last in the chain and deliberately narrow: with a single baseline on disk - * there is only one branch whose results can be read at all, so naming it is - * strictly better than resolving to a branch that holds nothing. Two or more - * baselines carry no such implication and are left alone. - */ private function soleRecordedBranch(): ?string { $json = $this->state->read(self::KEY_GRAPH); @@ -2098,11 +1821,15 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument if (str_starts_with($arg, '-')) { continue; } - if ($index > 0) { - $previous = $arguments[$index - 1] ?? ''; - if (in_array($previous, self::VALUE_TAKING_FLAGS, true)) { - continue; - } + + if ($index === 0) { + continue; + } + + $previous = $arguments[$index - 1] ?? ''; + + if (in_array($previous, self::VALUE_TAKING_FLAGS, true)) { + continue; } $candidate = $this->resolveArgumentPath($arg, $projectRoot); @@ -2111,16 +1838,34 @@ final class Tia implements AddsOutput, HandlesArguments, HandlesOriginalArgument continue; } - foreach ($testPaths as $testPath) { - if ($candidate === $testPath || str_starts_with($candidate, $testPath.DIRECTORY_SEPARATOR)) { - return true; - } + if ($this->narrowsSuite($candidate, $testPaths)) { + return true; } } return false; } + /** + * @param array $testPaths + */ + private function narrowsSuite(string $candidate, array $testPaths): bool + { + foreach ($testPaths as $testPath) { + if ($candidate === $testPath || str_starts_with($candidate, $testPath.DIRECTORY_SEPARATOR)) { + return true; + } + } + + foreach ($testPaths as $testPath) { + if (str_starts_with($testPath, $candidate.DIRECTORY_SEPARATOR)) { + return false; + } + } + + return true; + } + private function resolveArgumentPath(string $arg, string $projectRoot): ?string { $candidates = [$arg, rtrim($projectRoot, DIRECTORY_SEPARATOR).DIRECTORY_SEPARATOR.ltrim($arg, DIRECTORY_SEPARATOR)]; diff --git a/src/Plugins/Tia/ChangedFiles.php b/src/Plugins/Tia/ChangedFiles.php index 9207bd03..01fd90c1 100644 --- a/src/Plugins/Tia/ChangedFiles.php +++ b/src/Plugins/Tia/ChangedFiles.php @@ -219,16 +219,6 @@ final readonly class ChangedFiles return $branch === '' || $branch === 'HEAD' ? null : $branch; } - /** - * The repository's default branch — the one every other branch's baseline - * falls back to reading. - * - * Advisory, unlike {@see self::currentBranch()}: a repository that cannot - * answer the question is not a broken repository. A remote-less checkout - * has no `origin/HEAD`, and plenty of CI checkouts never run - * `git remote set-head`, so every step here fails soft and the caller is - * left to pick its own default. - */ public function defaultBranch(): ?string { $head = $this->gitOutput(['git', 'symbolic-ref', '--short', 'refs/remotes/origin/HEAD']); @@ -241,10 +231,6 @@ final readonly class ChangedFiles } } - // `init.defaultBranch` is a setting of the machine, not of the - // repository — it names what `git init` would have called the first - // branch here, which is worth nothing once the repository disagrees. - // Taken only when a branch by that name actually exists. $configured = $this->gitOutput(['git', 'config', '--get', 'init.defaultBranch']); if ($configured === null) { @@ -257,12 +243,6 @@ final readonly class ChangedFiles return $exists ? $configured : null; } - /** - * Whether the repository has any remote configured. - * - * Advisory like {@see self::defaultBranch()} — a `git` that cannot answer - * is reported as "no remote", and the caller decides what that means. - */ public function hasRemote(): bool { return $this->gitOutput(['git', 'remote']) !== null; diff --git a/src/Plugins/Tia/CiDefaultBranch.php b/src/Plugins/Tia/CiDefaultBranch.php index 07450ae3..3db30f5f 100644 --- a/src/Plugins/Tia/CiDefaultBranch.php +++ b/src/Plugins/Tia/CiDefaultBranch.php @@ -5,23 +5,10 @@ declare(strict_types=1); namespace Pest\Plugins\Tia; /** - * The default branch as the CI provider itself reports it. - * - * Worth asking before git: a CI checkout is the one place where git knows the - * least. `actions/checkout` builds the working copy with `git init` plus a - * single-ref `fetch` rather than a `clone`, so `origin/HEAD` is never set and - * `init.defaultBranch` — a setting of the runner image, not of the repository — - * is all git has left to offer. The provider, meanwhile, states the answer - * outright in the environment it handed us. - * * @internal */ final class CiDefaultBranch { - /** - * Advisory, like every other source in the chain: anything unreadable, - * unparsable, or simply absent means "no answer", never a failure. - */ public static function detect(): ?string { return self::fromGitLab() ?? self::fromGitHubEvent(); @@ -32,11 +19,6 @@ final class CiDefaultBranch return self::environment('CI_DEFAULT_BRANCH'); } - /** - * GitHub publishes no default-branch variable, but every repository-scoped - * event payload carries `repository.default_branch`, and the path to that - * payload is in the environment. - */ private static function fromGitHubEvent(): ?string { $path = self::environment('GITHUB_EVENT_PATH'); diff --git a/src/Plugins/Tia/Configuration.php b/src/Plugins/Tia/Configuration.php index 0850b2e6..fc7a42bd 100644 --- a/src/Plugins/Tia/Configuration.php +++ b/src/Plugins/Tia/Configuration.php @@ -61,12 +61,6 @@ final class Configuration } /** - * The branch whose baseline every other branch falls back to reading. - * - * Autodetected from the repository when left unset; declare it here when - * the repository cannot answer for itself — no `origin/HEAD`, or an - * `init.defaultBranch` that disagrees with reality. - * * @return $this */ public function defaultBranch(string $branch): self diff --git a/src/Plugins/Tia/Graph.php b/src/Plugins/Tia/Graph.php index 22f42f7d..1ab7cc61 100644 --- a/src/Plugins/Tia/Graph.php +++ b/src/Plugins/Tia/Graph.php @@ -48,15 +48,6 @@ final class Graph */ private array $baselines = []; - /** - * The baseline a branch with none of its own reads from. - * - * Only ever read from: a branch writes to its own key, so a fallback that - * leaked into the write path would corrupt the baseline every other branch - * depends on. Resolved once per run by the plugin — see - * {@see self::setFallbackBranch()} — because the git calls it takes are not - * free and the read path runs per test. - */ private string $fallbackBranch = 'main'; private readonly string $projectRoot; @@ -704,15 +695,6 @@ 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 = null): bool { $baseline = $this->baselineFor($branch, $fallbackBranch); @@ -730,9 +712,6 @@ final class Graph $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; } @@ -856,13 +835,7 @@ 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. + * @param bool $keepExisting Leave already-recorded edge sets alone. */ public function replaceEdges(array $testToFiles, bool $keepExisting = false): void { @@ -873,8 +846,6 @@ 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; } @@ -1483,13 +1454,6 @@ final class Graph } /** - * The branches a recorded graph holds baselines for, read straight from the - * encoded form. - * - * Answerable before the graph is hydrated because the default branch has to - * be resolved first: the fallback is what every hydrated graph reads its - * baselines through. - * * @return list */ public static function branchesIn(string $json): array diff --git a/tests/Features/Tia.php b/tests/Features/Tia.php index c8acb6fd..30df425d 100644 --- a/tests/Features/Tia.php +++ b/tests/Features/Tia.php @@ -49,7 +49,7 @@ it('does not run user hooks when replaying cached skipped and incomplete results expect($storage->write(Tia::KEY_GRAPH, (string) $json))->toBeTrue(); $process = new Process( - ['php', 'bin/pest', $fixture, '--tia'], + ['php', 'bin/pest', '--configuration', 'tests/Fixtures/Suites/TiaReplayHooks.xml', '--tia'], $projectRoot, [ 'COLLISION_PRINTER' => 'DefaultPrinter', diff --git a/tests/Features/Tia/DefaultBranchReplay.php b/tests/Features/Tia/DefaultBranchReplay.php index e31447a1..e6c5efb4 100644 --- a/tests/Features/Tia/DefaultBranchReplay.php +++ b/tests/Features/Tia/DefaultBranchReplay.php @@ -4,15 +4,6 @@ declare(strict_types=1); use Tests\Fixtures\Tia\Project; -/** - * Reading a baseline recorded on another branch. - * - * Without the default-branch fallback, the first `--tia` run on every new branch - * re-runs a whole suite whose results the default branch already holds — once - * per branch, forever, on any repository not named `main`. - * - * @see https://github.com/pestphp/pest/issues/1823 - */ afterEach(function (): void { Project::destroyAll(); }); @@ -54,8 +45,6 @@ test('replays on a second new branch too', function (): void { $result = $project->pest('--tia'); - // The toll is one full run per new branch. It must not come back for the - // second branch either. expect($result->replayed())->toBe(Project::TOTAL_TESTS, $result->describe()) ->and($result->uncached())->toBe(0, $result->describe()); })->skipOnWindows(); @@ -93,12 +82,6 @@ test('replays inside a worktree on a new branch', function (): void { $project = Project::make('master'); $worktree = $project->worktree('feature-worktree'); - // Seeded against the worktree rather than the main checkout: a worktree's - // `.git` is a file, so `Storage::originIdentity()` cannot read the remote - // from it and the worktree resolves a storage key of its own. That gap is - // separate from the branch fallback, and it is the fallback this row is - // about — the worktree is checked out on a branch the baseline does not - // name, which is the scenario from the issue. $project->seedFor($worktree, 'master'); $result = $project->pestIn($worktree, '--tia'); diff --git a/tests/Features/Tia/DefaultBranchResolution.php b/tests/Features/Tia/DefaultBranchResolution.php index a79a350f..7a17aa8a 100644 --- a/tests/Features/Tia/DefaultBranchResolution.php +++ b/tests/Features/Tia/DefaultBranchResolution.php @@ -5,17 +5,11 @@ declare(strict_types=1); use Symfony\Component\Process\ExecutableFinder; use Tests\Fixtures\Tia\Project; -/** - * How the default branch gets named: declared in `tests/Pest.php`, autodetected - * from the repository, or not answerable at all. - */ afterEach(function (): void { Project::destroyAll(); }); test('a declared default branch beats autodetection', function (): void { - // The repository autodetects `develop`, which holds no baseline. Only the - // declaration in `tests/Pest.php` can reach the `master` one. $project = Project::make('develop', overlay: 'configured-default-branch'); $project->seed('master'); @@ -35,17 +29,12 @@ test('a declared default branch that does not exist degrades to a full run', fun $result = $project->pest('--tia'); - // Nothing to fall back to, so everything runs — and no baseline is minted - // under the name that resolved to nothing. expect($result->uncached())->toBe(Project::TOTAL_TESTS, $result->describe()) ->and($result->exitCode)->toBe(0, $result->describe()) ->and($project->branchKeys())->toBe(['master', 'feature-x']); })->skipOnWindows(); test('the CI provider names the default branch where the checkout cannot', function (): void { - // A CI checkout: `actions/checkout` fetches a single ref instead of cloning, - // so there is no `origin/HEAD` for git to read the default branch from. The - // event payload GitHub hands the job says it outright. $project = Project::make('master'); $project->git()->unsetOriginHead(); $project->seed('master'); @@ -80,10 +69,6 @@ test('GitLab names the default branch through its own variable', function (): vo })->skipOnWindows(); test('a lone recorded baseline names the default branch', function (): void { - // Nothing left to ask: no `origin/HEAD`, no CI provider, and an - // `init.defaultBranch` that names a branch this repository does not have. - // The graph holds exactly one baseline, and it is the only one any branch - // could read — so it is the answer. $project = Project::make('master'); $project->git()->unsetOriginHead(); $project->git()->config('init.defaultBranch', 'main'); @@ -98,9 +83,6 @@ test('a lone recorded baseline names the default branch', function (): void { })->skipOnWindows(); test('a default branch nothing can name is refused rather than guessed', function (): void { - // Same checkout as above, without the graph that answered it. Guessing here - // is what made this bug expensive: the guess reads no baseline at all, and - // the output calls that a hit. $project = Project::make('master'); $project->git()->unsetOriginHead(); $project->git()->config('init.defaultBranch', 'main'); @@ -116,9 +98,6 @@ test('a default branch nothing can name is refused rather than guessed', functio })->skipOnWindows(); test('an init.defaultBranch naming a branch that exists is still trusted', function (): void { - // The setting is the machine's, not the repository's — worth taking only - // where the repository has a branch by that name. It does here, and with no - // graph on disk it is the only source left, so the run must not be refused. $project = Project::make('master'); $project->git()->unsetOriginHead(); $project->git()->config('init.defaultBranch', 'master'); @@ -141,16 +120,11 @@ test('a repository with no remote is refused rather than silently re-run', funct $result = $project->pest('--tia'); - // Nothing can name the default branch, so every new branch would re-run the - // whole suite with no explanation. Saying so beats doing that quietly. The - // missing remote is the likeliest reason and gets named as such. expect($result->output)->toContain('Tia mode requires a repository with a remote.') ->and($result->exitCode)->toBe(1, $result->describe()); })->skipOnWindows(); test('a remote-less repository holding one baseline is not refused', function (): void { - // The refusal above exists to stop a guess, not to demand a remote for its - // own sake. With a baseline on disk there is nothing left to guess at. $project = Project::make('master'); $project->git()->removeOrigin(); @@ -165,8 +139,6 @@ test('a remote-less repository holding one baseline is not refused', function () })->skipOnWindows(); test('a declared default branch stands in for a missing remote', function (): void { - // The escape hatch the refusal above points at: with the branch named by - // hand there is nothing left for a remote to answer. $project = Project::make('master', overlay: 'configured-default-branch'); $project->git()->removeOrigin(); @@ -185,8 +157,6 @@ test('tia still requires git', function (): void { $result = $project->pest('--tia'); - // The soft default-branch resolver runs before the branch is named, and it - // must not swallow this. expect($result->output)->toContain('The feature "Tia mode" requires "git".') ->and($result->exitCode)->not->toBe(0); })->skipOnWindows(); @@ -212,8 +182,6 @@ test('the default branch is resolved once per run, not once per test', function expect($git)->not->toBeNull(); - // A `git` first on `PATH` that records what it was asked before handing over - // to the real one. $log = $project->path('git-calls.log'); $project->write('shim/git', implode("\n", [ '#!/bin/sh', diff --git a/tests/Features/Tia/DefaultBranchWriteTier.php b/tests/Features/Tia/DefaultBranchWriteTier.php index caa4c825..28f56020 100644 --- a/tests/Features/Tia/DefaultBranchWriteTier.php +++ b/tests/Features/Tia/DefaultBranchWriteTier.php @@ -4,12 +4,6 @@ declare(strict_types=1); use Tests\Fixtures\Tia\Project; -/** - * What the fallback is allowed to touch. - * - * Reading another branch's baseline must stay a read: writes belong to the - * branch that ran, and a branch that has no name of its own must not mint one. - */ afterEach(function (): void { Project::destroyAll(); }); @@ -20,8 +14,6 @@ test('narrows to the affected tests on a new branch', function (): void { $project->git()->switchTo('feature-x', new: true); - // Semantic, not cosmetic: PHP is hashed at the AST level, so a comment - // would not register as a change at all. $project->write('app/Calculator.php', <<<'PHP' pest('--tia'); - // Two of the three test files cover `Calculator`; the third replays from the - // default branch's baseline. expect($result->affected())->toBe(4, $result->describe()) ->and($result->replayed())->toBe(2, $result->describe()) ->and($result->exitCode)->toBe(0, $result->describe()); @@ -60,18 +50,12 @@ test('narrows to the affected tests on a new branch', function (): void { test('filtered mode reads the fallback too', function (): void { $project = Project::make('master'); - // A failure cached on the default branch. Filtered mode asks the graph which - // test files are due a re-run, and that read has to reach the fallback as - // well: without it a new branch sees nothing to do, reports green, and never - // re-runs the failure — on every subsequent invocation. $project->seed('master', failing: ['adds two numbers']); $project->git()->switchTo('feature-x', new: true); $result = $project->pest('--tia', '--filtered'); - // Selection is by file, so the failure's two siblings in `CalculatorTest` - // come along; the other two test files stay out of the run entirely. expect($result->output)->toContain('from 1 previously unsuccessful test') ->and($result->output)->not->toContain('No affected tests found') ->and($result->tally())->toContain('2 passed'); @@ -99,8 +83,6 @@ test('a detached HEAD replays without minting a branch key', function (): void { $result = $project->pest('--tia'); - // A detached HEAD has no branch of its own. The default branch is the - // honest key, and no phantom one appears beside it. expect($result->replayed())->toBe(Project::TOTAL_TESTS, $result->describe()) ->and($result->uncached())->toBe(0, $result->describe()) ->and($project->branchKeys())->toBe(['master']); diff --git a/tests/Fixtures/Suites/TiaReplayHooks.xml b/tests/Fixtures/Suites/TiaReplayHooks.xml new file mode 100644 index 00000000..c7d6ef52 --- /dev/null +++ b/tests/Fixtures/Suites/TiaReplayHooks.xml @@ -0,0 +1,17 @@ + + + + + + TiaReplayHooks.php + + + diff --git a/tests/Fixtures/Tia/GitRepo.php b/tests/Fixtures/Tia/GitRepo.php index e828fd04..0969ac3a 100644 --- a/tests/Fixtures/Tia/GitRepo.php +++ b/tests/Fixtures/Tia/GitRepo.php @@ -4,19 +4,10 @@ declare(strict_types=1); namespace Tests\Fixtures\Tia; -use Pest\Plugins\Tia\Storage; use RuntimeException; use Symfony\Component\Process\Process; /** - * A git repository the scenario tests drive. - * - * Hermetic by construction: every invocation neutralises the machine's global - * and system config and carries its own committer identity. Without that, an - * ambient `init.defaultBranch = main` would answer questions the scenario means - * to leave unanswered, and rows like "resolves to nothing, degrades safely" - * would pass by accident. - * * @internal */ final readonly class GitRepo @@ -27,9 +18,6 @@ final readonly class GitRepo public const array ENV = [ 'GIT_CONFIG_GLOBAL' => '/dev/null', 'GIT_CONFIG_SYSTEM' => '/dev/null', - // `GIT_CONFIG_SYSTEM` does not cover every system-level file git reads: - // Apple's git also loads one from inside Xcode, and it sets - // `init.defaultBranch`. Only this suppresses all of them. 'GIT_CONFIG_NOSYSTEM' => '1', 'GIT_AUTHOR_NAME' => 'Pest Fixture', 'GIT_AUTHOR_EMAIL' => 'fixture@pestphp.io', @@ -39,9 +27,6 @@ final readonly class GitRepo public function __construct(public string $path) {} - /** - * Initialises the repository on `$branch` and commits everything in it. - */ public function init(string $branch = 'master'): void { $this->run(['init', '--quiet']); @@ -70,12 +55,6 @@ final readonly class GitRepo $this->run(['checkout', '--quiet', '--detach']); } - /** - * Registers an `origin`, which also decides the graph's storage key: - * {@see Storage::projectKey()} prefers the remote's - * identity over the path, so two checkouts of one repository — a worktree, - * say — share a single graph. - */ public function addOrigin(string $url = 'git@github.com:pestphp/tia-fixture.git'): void { $this->run(['remote', 'add', 'origin', $url]); @@ -86,22 +65,12 @@ final readonly class GitRepo $this->run(['remote', 'remove', 'origin']); } - /** - * Points `refs/remotes/origin/HEAD` at a local branch — what - * `git remote set-head` would write, without a remote to talk to. - */ public function setOriginHead(string $branch): void { $this->run(['update-ref', 'refs/remotes/origin/'.$branch, 'HEAD']); $this->run(['symbolic-ref', 'refs/remotes/origin/HEAD', 'refs/remotes/origin/'.$branch]); } - /** - * Drops `refs/remotes/origin/HEAD` while keeping the remote-tracking branch - * — what a CI checkout looks like. `actions/checkout` builds its working - * copy with `git init` plus a single-ref `fetch` rather than a `clone`, and - * only a `clone` writes that symbolic ref. - */ public function unsetOriginHead(): void { $this->run(['symbolic-ref', '--delete', 'refs/remotes/origin/HEAD']); @@ -112,9 +81,6 @@ final readonly class GitRepo $this->run(['config', '--local', $key, $value]); } - /** - * Adds a worktree for a new branch and returns its path. - */ public function worktree(string $path, string $branch): string { $this->run(['worktree', 'add', '--quiet', '-b', $branch, $path]); diff --git a/tests/Fixtures/Tia/GraphDelta.php b/tests/Fixtures/Tia/GraphDelta.php index 54ba5abd..7d930e78 100644 --- a/tests/Fixtures/Tia/GraphDelta.php +++ b/tests/Fixtures/Tia/GraphDelta.php @@ -5,20 +5,6 @@ declare(strict_types=1); namespace Tests\Fixtures\Tia; /** - * What one run did to the graph. - * - * The three tiers a run may respect, from the conformance matrix: - * - * - COMPLETE — may change everything. - * - RESULTS-ONLY — may change only `baselines[].results` for tests that - * actually ran. It may never remove an entry, nor move `sha`, `tree`, - * `edges`, `files` or `fingerprint`. - * - HARD-SUPPRESSED — may change nothing at all. - * - * {@see self::writtenCount()} is the load-bearing measurement, and the reason - * {@see Project::sentinel()} exists: without falsified cached values there is no - * way to tell "wrote the same values back" from "wrote nothing". - * * @internal */ final readonly class GraphDelta @@ -42,9 +28,6 @@ final readonly class GraphDelta return $this->before !== null && $this->after === null; } - /** - * Result entries whose stored values actually moved. - */ public function writtenCount(): int { $written = 0; @@ -71,9 +54,6 @@ final readonly class GraphDelta return $written; } - /** - * Result entries that appeared. - */ public function added(): int { $added = 0; @@ -88,9 +68,6 @@ final readonly class GraphDelta return $added; } - /** - * Result entries that were pruned. - */ public function removed(): int { $removed = 0; @@ -106,9 +83,6 @@ final readonly class GraphDelta } /** - * The baseline keys after the run — the headline signal for the - * default-branch rows, where a phantom key is the defect. - * * @return array */ public function branchKeys(): array @@ -129,10 +103,6 @@ final readonly class GraphDelta return $this->branchKeysBefore() !== $this->branchKeys(); } - /** - * Whether the named branch's baseline is byte-identical — how a row proves - * the fallback is read-only. - */ public function baselineUntouched(string $branch): bool { return ($this->baselines($this->before)[$branch] ?? null) @@ -149,11 +119,6 @@ final readonly class GraphDelta return array_any($this->branchKeys(), fn (string $branch) => $this->baselineField($branch, 'tree', $this->before) !== $this->baselineField($branch, 'tree', $this->after)); } - /** - * Compares edges by the file paths they resolve to, not by file id: ids are - * an implementation detail that shifts whenever `files` is rebuilt in a - * different order. - */ public function edgesMoved(): bool { return $this->edgeSets($this->before) !== $this->edgeSets($this->after); @@ -184,17 +149,11 @@ final readonly class GraphDelta return $this->branchKeysMoved(); } - /** - * Nothing moved at all. - */ public function isHardSuppressed(): bool { return $this->before === $this->after; } - /** - * Results may have moved for tests that ran; nothing structural did. - */ public function isResultsOnly(): bool { return ! $this->graphWasCreated() @@ -206,9 +165,6 @@ final readonly class GraphDelta && $this->added() === 0; } - /** - * A one-line verdict, for failure messages. - */ public function summary(): string { if ($this->graphWasCreated()) { diff --git a/tests/Fixtures/Tia/PestResult.php b/tests/Fixtures/Tia/PestResult.php index 16533be7..de9a339e 100644 --- a/tests/Fixtures/Tia/PestResult.php +++ b/tests/Fixtures/Tia/PestResult.php @@ -5,15 +5,10 @@ declare(strict_types=1); namespace Tests\Fixtures\Tia; /** - * The outcome of one `pest` invocation against a fixture project. - * * @internal */ final readonly class PestResult { - /** - * The run's output, with the terminal's escape sequences taken back out. - */ public string $output; /** @@ -25,39 +20,26 @@ final readonly class PestResult public int $exitCode, ) { $this->output = (string) preg_replace([ - '#\x1b[[][^A-Za-z]*[A-Za-z]#', // colours, cursor moves - '#\x1b\]8;[^\x1b\x07]*(?:\x1b\\\\|\x07)#', // hyperlinks + '#\x1b[[][^A-Za-z]*[A-Za-z]#', + '#\x1b\]8;[^\x1b\x07]*(?:\x1b\\\\|\x07)#', ], '', $output); } - /** - * Tests whose cached result was replayed instead of executed. - */ public function replayed(): int { return $this->recapFragment('replayed'); } - /** - * Tests that ran because the graph held nothing for them — the count that - * betrays a fallback which never resolved. - */ public function uncached(): int { return $this->recapFragment('uncached'); } - /** - * Tests that ran because a file they depend on changed. - */ public function affected(): int { return $this->recapFragment('affected'); } - /** - * The `Tests:` summary line, without its label or leading whitespace. - */ public function tally(): string { if (preg_match('/^\s*Tests:\s+(.+)$/m', $this->output, $matches) !== 1) { @@ -72,10 +54,6 @@ final readonly class PestResult return str_contains($this->output, $needle); } - /** - * A description of the run, for failure messages that would otherwise say - * only that 0 !== 6. - */ public function describe(): string { return sprintf( @@ -86,11 +64,6 @@ final readonly class PestResult ); } - /** - * Read off the `Tests:` line rather than the whole output: the TIA headline - * counts affected *files*, and matching that instead would be a quietly - * wrong number. - */ private function recapFragment(string $label): int { if (preg_match('/(\d+) '.preg_quote($label, '/').'/', $this->tally(), $matches) !== 1) { diff --git a/tests/Fixtures/Tia/Project.php b/tests/Fixtures/Tia/Project.php index ee6b42ff..04b5ff3b 100644 --- a/tests/Fixtures/Tia/Project.php +++ b/tests/Fixtures/Tia/Project.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace Tests\Fixtures\Tia; use FilesystemIterator; -use Pest\Factories\TestCaseFactory; use Pest\Plugins\Tia; use Pest\Plugins\Tia\ChangedFiles; use Pest\Plugins\Tia\FileState; @@ -19,28 +18,11 @@ use RuntimeException; use Symfony\Component\Process\Process; /** - * A throwaway Pest project the TIA scenario tests drive. - * - * Two facts about Pest shape everything here: - * - * 1. `bin/pest` derives the project root from **the autoloader it finds**, not - * from the working directory — `dirname($autoloadPath, 2)`. So the project - * owns a real `vendor/autoload.php` and a real copy of `bin/pest` at the path - * a composer install would have put them. A symlinked `vendor` would resolve - * `__DIR__` straight back to the Pest repository, and every scenario would - * silently measure the wrong project. - * 2. TIA cannot *record* without pcov or Xdebug, and CI has neither. So a - * scenario never records: {@see self::seed()} writes the graph a recording - * run would have written, and the run under test exercises the read path. - * * @internal */ final class Project { /** - * Test file → the source files a recording run would have linked it to. The - * self-edge every test file gets is added on top of these. - * * @var array> */ public const array EDGES = [ @@ -50,8 +32,6 @@ final class Project ]; /** - * Test file → the descriptions it declares, in declaration order. - * * @var array> */ public const array TESTS = [ @@ -60,15 +40,9 @@ final class Project 'tests/Feature/CoversCalculatorTest.php' => ['adds within a feature test', 'subtracts within a feature test'], ]; - /** - * Every test in the fixture suite. - */ public const int TOTAL_TESTS = 6; /** - * Every project scaffolded so far, so a row cannot leak one by failing - * before its own cleanup. - * * @var array */ private static array $created = []; @@ -85,13 +59,6 @@ final class Project */ private array $extraPaths = []; - /** - * The root whose graph this project reads and writes. - * - * Its own, except where a row seeds a worktree: a worktree's `.git` is a - * file rather than a directory, so {@see Storage} cannot read the remote - * from it and resolves a storage key of its own. - */ private string $graphRoot; private function __construct(public readonly string $path) @@ -99,14 +66,6 @@ final class Project $this->graphRoot = $path; } - /** - * Scaffolds a project whose default branch is `$branch`, and hands it back - * checked out there. - * - * The repository is realistic on purpose: it has an `origin`, and an - * `origin/HEAD` naming `$branch`, which is what a checkout of a real project - * looks like and what the default branch is autodetected from. - */ public static function make(string $branch = 'master', ?string $overlay = null): self { $project = self::scaffold($overlay); @@ -118,9 +77,6 @@ final class Project return $project; } - /** - * Destroys every project scaffolded so far. Belongs in an `afterEach`. - */ public static function destroyAll(): void { while (self::$created !== []) { @@ -128,14 +84,6 @@ final class Project } } - /** - * A project that is not a git repository at all — for the rows that assert - * TIA still demands git, and that a plain run does not care. - * - * Lives in the system temp directory, so it is outside any enclosing - * repository, and owns a real `vendor` rather than a symlinked one, so its - * baseline key cannot collide with another fixture's. - */ public static function withoutGit(?string $overlay = null): self { return self::scaffold($overlay); @@ -151,12 +99,6 @@ final class Project return $relative === '' ? $this->path : $this->path.DIRECTORY_SEPARATOR.$relative; } - /** - * Overwrites a file in the project. - * - * Edits must be semantic: TIA hashes PHP at the AST level, so a - * comment-only change is not a change at all. - */ public function write(string $relative, string $contents): void { $path = $this->path($relative); @@ -171,14 +113,6 @@ final class Project } } - /** - * Adds a worktree for a new branch, scaffolded so `pest` can run in it, and - * returns its path. - * - * The graph is shared with the main checkout, which is the whole point: both - * resolve the same storage key, because {@see Storage} prefers the `origin` - * identity over the path. - */ public function worktree(string $branch): string { $path = $this->path.'-worktree-'.preg_replace('/[^a-z0-9]+/i', '-', $branch); @@ -190,18 +124,11 @@ final class Project return $path; } - /** - * Runs `pest` in the project and returns what happened. - */ public function pest(string ...$arguments): PestResult { return $this->pestIn($this->path, ...$arguments); } - /** - * Runs `pest` in `$directory` — a worktree, say — against this project's - * graph. - */ public function pestIn(string $directory, string ...$arguments): PestResult { return $this->pestWithEnvironment($directory, [], ...$arguments); @@ -222,11 +149,6 @@ final class Project 'PARATEST' => '0', 'PAO_DISABLE' => '1', 'HOME' => $this->home(), - // Blanked for the same reason `GitRepo::ENV` blanks git's own - // config: the default branch a CI provider reports is the one - // *its* build is for. Pest's suite runs on GitHub Actions, so - // without this every scenario would autodetect Pest's default - // branch instead of the fixture's. 'GITHUB_EVENT_PATH' => '', 'CI_DEFAULT_BRANCH' => '', ...$environment, @@ -244,17 +166,6 @@ final class Project } /** - * Writes the graph a clean, green recording run on `$branch` would have - * written, and remembers it as the snapshot the next {@see self::delta()} - * compares against. - * - * Sentinelled by default: a row almost always wants to know which entries - * were written, and only a real recording run's values can answer that. - * - * `$failing` names descriptions to record as failures — a clean green run - * can never cache one, so a row that needs a cached failure to re-run has to - * be handed it. - * * @param array $failing */ public function seed(string $branch, bool $sentinel = true, array $failing = []): void @@ -263,9 +174,6 @@ final class Project } /** - * Seeds the graph belonging to `$root` — a worktree, say, which resolves a - * storage key of its own. - * * @param array $failing */ public function seedFor(string $root, string $branch, bool $sentinel = true, array $failing = []): void @@ -279,8 +187,6 @@ final class Project $graph->setFingerprint(Fingerprint::compute($root)); $graph->setRecordedAtSha($branch, $sha); - // Hashes the tree as it stands, so the run under test sees nothing as - // changed — the same call the recording path makes. $graph->setLastRunTree($branch, $changedFiles->snapshotTree($changedFiles->since($sha) ?? [])); $graph->markKnownTestFiles(array_keys(self::EDGES)); @@ -322,13 +228,6 @@ final class Project $sentinel ? $this->sentinel() : $this->snapshot(); } - /** - * The id PHPUnit reports for a test in the fixture suite. - * - * Mirrors how Pest names generated test classes - * ({@see TestCaseFactory}): a wrong id here shows up as - * `0 replayed`, which every scenario asserts against. - */ public static function testId(string $testFile, string $description): string { $basename = basename($testFile, '.php'); @@ -343,15 +242,6 @@ final class Project return 'P\\'.str_replace(DIRECTORY_SEPARATOR, '\\', $relative).'::'.Str::evaluable($description); } - /** - * Falsifies every cached value, so the next {@see self::delta()} can tell - * "wrote the same values back" from "wrote nothing". - * - * `assertions` is only ever falsified where it is already non-zero: risky, - * skipped and incomplete statuses are *derived* from "performed no - * assertions", so patching those would rewrite the status on replay and - * destroy the very discriminator this exists to provide. - */ public function sentinel(): void { $graph = $this->graph(); @@ -376,8 +266,6 @@ final class Project } /** - * The decoded graph, or `null` when there is none. - * * @return array|null */ public function graph(): ?array @@ -413,43 +301,21 @@ final class Project return is_file($this->graphDir().DIRECTORY_SEPARATOR.Tia::KEY_GRAPH); } - /** - * Remembers the graph as it stands now. - */ public function snapshot(): void { $this->snapshot = $this->graph(); } - /** - * What has happened to the graph since the last snapshot. - */ public function delta(): GraphDelta { return new GraphDelta($this->snapshot, $this->graph()); } - /** - * Writes the `vendor` a composer install would have produced. - * - * Pest is mirrored in at `vendor/pestphp/pest` rather than pointed at, - * because Pest locates things from where its own files sit: - * `bin/pest` finds the project root by walking up from the autoloader it - * loads, and a parallel run picks its worker binary — and with it the - * worker's project root — from the directory the runner class was loaded - * from. Deferring to the repository's copy would resolve both back to the - * Pest repository, and every scenario would quietly measure that instead. - * - * Hardlinked where the filesystem allows it, so the mirror costs almost - * nothing and can never drift from the working tree. - */ public function scaffoldVendor(string $directory): void { $pestRoot = dirname(__DIR__, 3); $pest = $directory.'/vendor/pestphp/pest'; - // `overrides`, `resources` and `stubs` come along because Pest loads - // them relative to `src` — the same list `BootExcludeList` walks. foreach (['src', 'overrides', 'resources', 'stubs'] as $tree) { self::mirror($pestRoot.'/'.$tree, $pest.'/'.$tree); } @@ -460,21 +326,11 @@ final class Project self::mirror($pestRoot.'/composer.json', $pest.'/composer.json'); - // Pest's own autoloader, with the mirrored copy taking precedence: the - // repository's `vendor` supplies PHPUnit, Symfony and the plugin - // packages, none of which care where they are loaded from. file_put_contents($directory.'/vendor/autoload.php', sprintf( "addPsr4('Pest\\\\', __DIR__.'/pestphp/pest/src', true);\n\nreturn \$loader;\n", var_export($pestRoot.'/vendor/autoload.php', true), )); - // Invoking the binary directly skips composer's bin proxy, which is - // what would otherwise define `$GLOBALS['_composer_bin_dir']`. Without - // it `Pest\Plugin\Loader` looks for `vendor/bin/../pest-plugins.json` - // relative to the working directory — so that is where the plugin list - // goes, and mirroring the repository's keeps it in step with - // composer.json. `vendor/bin` has to exist for the `..` in that path to - // resolve, empty though it is. if (! is_dir($directory.'/vendor/bin') && ! @mkdir($directory.'/vendor/bin', 0755, true)) { throw new RuntimeException(sprintf('Unable to create [%s].', $directory.'/vendor/bin')); } @@ -500,9 +356,6 @@ final class Project } /** - * `Storage` reads `HOME` from the environment, so the graph lands inside the - * throwaway project instead of the developer's real `~/.pest`. - * * @template TReturn * * @param callable(): TReturn $callback @@ -529,8 +382,6 @@ final class Project throw new RuntimeException(sprintf('Unable to create [%s].', $path)); } - // Realpathed because Pest realpaths its own project root, and macOS - // hands out a symlinked temp directory. $real = realpath($path); $project = new self($real === false ? $path : $real); @@ -543,8 +394,6 @@ final class Project self::copy(__DIR__.'/overlays/'.$overlay, $project->path); } - // `ChangedFiles` asks git what changed, so anything the scaffold writes - // but the project does not own has to be invisible to it. $project->write('.gitignore', implode("\n", ['/vendor/', '/.home/', '/.phpunit.cache/', ''])); $project->scaffoldVendor($project->path); @@ -553,10 +402,6 @@ final class Project return $project; } - /** - * Mirrors a file or directory, hardlinking where the filesystem allows it - * and copying where it does not. - */ private static function mirror(string $from, string $to): void { if (is_dir($from)) { diff --git a/tests/Fixtures/Tia/app/tests/Feature/CoversCalculatorTest.php b/tests/Fixtures/Tia/app/tests/Feature/CoversCalculatorTest.php index bebd43ac..1b6dfc8a 100644 --- a/tests/Fixtures/Tia/app/tests/Feature/CoversCalculatorTest.php +++ b/tests/Fixtures/Tia/app/tests/Feature/CoversCalculatorTest.php @@ -4,8 +4,6 @@ declare(strict_types=1); use Fixture\App\Calculator; -// A second test file over the same source file, so a `Calculator` edit narrows -// to two of the three test files rather than to one. test('adds within a feature test', function (): void { expect((new Calculator)->add(10, 5))->toBe(15); }); diff --git a/tests/Fixtures/Tia/app/tests/Pest.php b/tests/Fixtures/Tia/app/tests/Pest.php index 7a4c6ed1..88080aa1 100644 --- a/tests/Fixtures/Tia/app/tests/Pest.php +++ b/tests/Fixtures/Tia/app/tests/Pest.php @@ -2,8 +2,5 @@ declare(strict_types=1); -// The fixture project has no composer autoloader of its own — its `vendor/` -// holds nothing but a shim pointing back at Pest's. Requiring the two classes -// here is enough for every test file in the suite. require_once __DIR__.'/../app/Calculator.php'; require_once __DIR__.'/../app/Greeter.php'; diff --git a/tests/Fixtures/Tia/app/tests/Unit/CalculatorTest.php b/tests/Fixtures/Tia/app/tests/Unit/CalculatorTest.php index 3640900c..bcfcc66a 100644 --- a/tests/Fixtures/Tia/app/tests/Unit/CalculatorTest.php +++ b/tests/Fixtures/Tia/app/tests/Unit/CalculatorTest.php @@ -4,9 +4,6 @@ declare(strict_types=1); use Fixture\App\Calculator; -// `test()` rather than `it()`: the harness seeds results by test id, and `it()` -// would prefix every description with `it `, leaving Project::TESTS a step away -// from what is written here. test('adds two numbers', function (): void { expect((new Calculator)->add(1, 2))->toBe(3); }); diff --git a/tests/Fixtures/Tia/overlays/configured-default-branch/tests/Pest.php b/tests/Fixtures/Tia/overlays/configured-default-branch/tests/Pest.php index 37d0447c..ec15f3c7 100644 --- a/tests/Fixtures/Tia/overlays/configured-default-branch/tests/Pest.php +++ b/tests/Fixtures/Tia/overlays/configured-default-branch/tests/Pest.php @@ -5,7 +5,4 @@ declare(strict_types=1); require_once __DIR__.'/../app/Calculator.php'; require_once __DIR__.'/../app/Greeter.php'; -// Declared default branch. Wins over whatever the repository autodetects — the -// escape hatch for a checkout with no `origin/HEAD` and a misleading -// `init.defaultBranch`. pest()->tia()->defaultBranch('master'); diff --git a/tests/Fixtures/Tia/overlays/unknown-default-branch/tests/Pest.php b/tests/Fixtures/Tia/overlays/unknown-default-branch/tests/Pest.php index 55c99b94..8fc7ecea 100644 --- a/tests/Fixtures/Tia/overlays/unknown-default-branch/tests/Pest.php +++ b/tests/Fixtures/Tia/overlays/unknown-default-branch/tests/Pest.php @@ -5,6 +5,4 @@ declare(strict_types=1); require_once __DIR__.'/../app/Calculator.php'; require_once __DIR__.'/../app/Greeter.php'; -// A branch the repository does not have. Accepted as configured — a name that -// resolves to no baseline degrades to a full run, which is safe. pest()->tia()->defaultBranch('nope'); diff --git a/tests/Pest.php b/tests/Pest.php index 6a0988ff..6247c703 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -19,8 +19,6 @@ pest()->in('PHPUnit/GlobPatternTests/SubFolder2/*AsPattern.php')->use(CustomTest pest()->in('Visual')->group('integration'); -// Every row scaffolds a throwaway project and runs `pest` in it as a -// subprocess, which is far too slow for the unit suite. pest()->in('Features/Tia')->group('integration'); // NOTE: global test value container to be mutated and checked across files, as needed diff --git a/tests/Unit/Plugins/Tia/Graph.php b/tests/Unit/Plugins/Tia/Graph.php index db18f612..1d82e4ac 100644 --- a/tests/Unit/Plugins/Tia/Graph.php +++ b/tests/Unit/Plugins/Tia/Graph.php @@ -67,8 +67,6 @@ describe('applyMigrationChanges()', function (): void { describe('rerun tracking', function (): void { beforeEach(function (): void { - // `hasUnlocatedTestsToRerun()` stats each recorded file to tell a - // deleted test apart from a live one, so the files have to exist. $this->projectRoot = sys_get_temp_dir().'/pest-tia-rerun-'.bin2hex(random_bytes(4)); mkdir($this->projectRoot.'/tests/Feature', 0755, true);