From 23efa6dd3601df07bb5c691beef513f20ad7ec1f Mon Sep 17 00:00:00 2001 From: "v.razuvaev" Date: Mon, 28 Sep 2026 09:58:11 +0300 Subject: [PATCH 1/3] Compare the throw site as well as the class when shrinking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A shrink candidate was accepted on the exception class alone, so a body that can fail in more than one place offered the descent failures it could not tell apart: shrinking could leave the failure that was found and minimise another one, and the reported counterexample then belonged to a bug the run never started from. Nothing in the output said so. `FailureIdentity` adds the place to the criterion. The place is the line of the property's OWN file in the failure's stack — the throwable's own line when the body threw it, otherwise the innermost frame in that file — and not the throw itself: PHPUnit raises every constraint failure from `Constraint::fail()` and Testo every comparison from `Assert`, so `getFile()`/`getLine()` alone would tell two assertions of one body apart from nothing. The file comes from the class part of the property id, which both adapters derive from the test class. The issue asks whether to walk to the first frame outside the engine instead: that does not work. Under Testo the body is reached through the framework's interceptor pipeline, so the frames next to the engine belong to Testo, not to the property. The scan is bounded the other way round — it stops at the runner's own file, because beyond it the stack is the engine calling the body, and an adapter that invokes the body from the test file (PHPUnit's `check()` call) would otherwise report that call as the site of every failure while an adapter that invokes it through reflection reported none. Nothing is guessed when the place is unreadable — an id that names no loaded class, a body written in another file, a failure raised by the engine rather than by the body: the class stays the whole criterion. A criterion that guessed, or one that compared whole traces, would refuse candidates that fail exactly as the original did (a failure reached through recursion arrives on a stack of a different height on every input) and stall the descent on the input it started from — the same silence, at a larger counterexample. The corpus stores no failure identity, and a path replay accepts any falsification, so neither is affected. Fixes #155 --- AGENTS.md | 15 +++ CHANGELOG.md | 18 +++ README.md | 17 ++- README.ru.md | 15 ++- llms.txt | 10 +- src/Internal/FailureIdentity.php | 145 ++++++++++++++++++++++ src/Runner/PropertyRunner.php | 39 +++--- tests/Internal/FailureIdentityTest.php | 130 +++++++++++++++++++ tests/Runner/PropertyRunnerShrinkTest.php | 107 ++++++++++++++++ tests/Support/EngineStand.php | 24 ++++ tests/Support/FailingHelper.php | 39 ++++++ 11 files changed, 528 insertions(+), 31 deletions(-) create mode 100644 src/Internal/FailureIdentity.php create mode 100644 tests/Internal/FailureIdentityTest.php create mode 100644 tests/Support/EngineStand.php create mode 100644 tests/Support/FailingHelper.php diff --git a/AGENTS.md b/AGENTS.md index 5fdea76..2d30721 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -137,6 +137,21 @@ make release-check - `Random` uses an object-scoped `\Random\Randomizer` (MT19937), NOT the global `mt_srand`/`mt_rand` — same seed, same sequence, regardless of intervening random calls. +- **A shrink candidate is accepted on failure identity, not on failure.** + `FailureIdentity` (built once per descent, compared against the ORIGINAL + failure and never against the last accepted one) is the exception class plus + the place in the property's own file: the throwable's own line when the body + threw it, otherwise the innermost frame in that file, and the scan stops at + `PropertyRunner`'s own file so frames beyond the engine — an adapter's + `check()` call sits in the test file too — name no place. Never compare the + throwable's `getFile()`/`getLine()` alone: PHPUnit raises every constraint + failure from `Constraint::fail()` and Testo every comparison from `Assert`, + so that criterion tells two assertions of one body apart from nothing. Never + compare traces or frame counts either: a failure reached through recursion + arrives on a stack of a different height on every input, and a criterion that + noticed would refuse every smaller candidate and report the input it started + from. When the place is unknown on either side, the class is the whole + identity, deliberately — the pre-1.1 behaviour, not a stall. - In-body draw shrinking is replay-tape-based and intentionally does NOT re-validate replayed nodes (fast-check `gen()` model). A regrown tape breaks the finite-tree termination argument, so accepted steps are capped by diff --git a/CHANGELOG.md b/CHANGELOG.md index 95f4081..16ed832 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,23 @@ # Changelog +## Unreleased + +- **Changed:** a shrink candidate is accepted only when it fails the same way + the original run did — the same exception class **and** the same place in the + property's own file. A body that can fail in more than one place (two + assertions, or an assertion beside a call that throws the same type) used to + offer the descent failures it could not tell apart, so the reported minimal + counterexample could belong to a bug the run never found, with nothing in the + output saying so. The place is the line of the property's own file in the + failure's stack, never the throw itself: an assertion library raises every + assertion of a body from one line of its own. Where the place cannot be read — + an id that names no loaded class, a body written in another file (a trait, an + included fixture), a failure the engine raised rather than the body — the + exception class stays the whole criterion. Minimised counterexamples may + therefore differ from 1.0.0 on such a body (compatibility policy §2: what a + descent minimises to is not under SemVer); a corpus recorded before this + release replays unchanged. + ## 1.0.0 — 2026-09-20 The stability release. The `@api` surface is that of 0.12.0, unchanged: no diff --git a/README.md b/README.md index 75bd232..44e6264 100644 --- a/README.md +++ b/README.md @@ -106,11 +106,18 @@ The `Changed:` line diffs the original against the shrunk counterexample — arguments the shrinker left untouched are omitted. `trial(s)` counts every candidate the shrinker ran (accepted and rejected); `shrink step(s)` counts only the accepted ones. A candidate is accepted only when it fails the same way -the original run did — the same exception class — so the descent minimises the -bug that was found instead of sliding into a different one (a smaller input that -trips a `TypeError` in the body's setup is not a smaller counterexample of an -assertion failure). Reproduce the exact run by pinning the reported seed in -`PropertyConfig`. +the original run did — the same exception class, raised at the same place in the +property's own file — so the descent minimises the bug that was found instead of +sliding into a different one (a smaller input that trips a `TypeError` in the +body's setup is not a smaller counterexample of an assertion failure, and neither +is one that trips the body's *other* assertion). The place is the line of the +property's file in the failure's stack, not the throw itself: an assertion +library raises every assertion of a body from one line of its own, so that line +would tell two assertions apart from nothing. Where the place cannot be read — +a property id that names no loaded class, a body written in another file (a +trait, an included fixture), a failure the engine raised rather than the body — +the exception class is the whole criterion, as it was before. Reproduce the exact +run by pinning the reported seed in `PropertyConfig`. See [`examples/standalone_runner.php`](examples/standalone_runner.php) for the full runnable script. diff --git a/README.ru.md b/README.ru.md index 5dd19d9..d6ed2e2 100644 --- a/README.ru.md +++ b/README.ru.md @@ -108,10 +108,17 @@ Property falsified after 0 successful run(s); seed=42 аргументы, которые shrinker не тронул, опущены. `trial(s)` — все кандидаты shrinker'а (принятые и отвергнутые); `shrink step(s)` — только принятые. Кандидат принимается, только если падает так же, как исходный прогон — с тем -же классом исключения: спуск минимизирует найденный баг, а не сползает в -другой (меньший вход, роняющий `TypeError` в подготовке тела, — не меньший -контрпример для assertion-падения). Точное воспроизведение прогона — запиньте -seed из отчёта в `PropertyConfig`. +же классом исключения и в том же месте собственного файла property: спуск +минимизирует найденный баг, а не сползает в другой (меньший вход, роняющий +`TypeError` в подготовке тела, — не меньший контрпример для assertion-падения, +и таким же не является вход, роняющий *другой* assert того же тела). Место — +это строка файла property в стеке падения, а не сам throw: assert-библиотека +роняет любой assert тела из одной своей строки, так что она не отличила бы два +assert'а ни от чего. Когда место прочитать нельзя — id property не называет +загруженный класс, тело написано в другом файле (трейт, подключённая фикстура), +падение подняла не body, а движок — критерием остаётся один класс исключения, +как и раньше. Точное воспроизведение прогона — запиньте seed из отчёта в +`PropertyConfig`. Полный исполняемый скрипт — [`examples/standalone_runner.php`](examples/standalone_runner.php). diff --git a/llms.txt b/llms.txt index f1ce8fb..07bc020 100644 --- a/llms.txt +++ b/llms.txt @@ -49,9 +49,13 @@ Runtime deps: PHP `8.3 - 8.5`, `ext-mbstring`, `ext-random`, `ext-tokenizer`. dependency is never told to narrow its generators. - Shrinking is integrated: `generate(Random)` returns a `Shrinkable` (value + lazy tree of smaller candidates). There is NO `shrink(mixed)` method. A shrink - candidate is accepted only when it fails with the SAME exception class as the - original run (a different failure is not a smaller counterexample). Greedy - per-parameter descent; best-effort minimal. + candidate is accepted only when it fails the SAME way as the original run: + same exception class, raised at the same place in the property's own file (the + line of that file in the stack — an assertion library throws every assertion + from one line of its own, so two assertions in one body are two failures). + Unreadable place -> class alone, as before: an id that names no loaded class, + a body in another file (trait, fixture), a failure raised by the engine. + Greedy per-parameter descent; best-effort minimal. - Corpus format is byte-compatible with `rasuvaeff/property-testing` 2.8 (`FORMAT_VERSION` 1): one JSON file per property, max 8 values + 2 seed entries. Seed entries are fenced off by `FilesystemCorpus::SEQUENCE_EPOCH`. diff --git a/src/Internal/FailureIdentity.php b/src/Internal/FailureIdentity.php new file mode 100644 index 0000000..698468c --- /dev/null +++ b/src/Internal/FailureIdentity.php @@ -0,0 +1,145 @@ +class === null || $candidate === null) { + return true; + } + + if ($candidate::class !== $this->class) { + return false; + } + + if ($this->site === null) { + return true; + } + + $site = self::site($candidate, $this->file, $this->boundary); + + return $site === null || $site === $this->site; + } + + /** + * Where in $file the failure was raised: the throwable's own line when the + * body threw it, otherwise the line of the innermost frame that is still in + * the body — an assertion helper is entered from the line the body wrote. + */ + private static function site(\Throwable $failure, ?string $file, string $boundary): ?string + { + if ($file === null) { + return null; + } + + if ($failure->getFile() === $file) { + return $file . ':' . $failure->getLine(); + } + + foreach ($failure->getTrace() as $frame) { + $frameFile = $frame['file'] ?? null; + + if ($frameFile === $boundary) { + return null; + } + + if ($frameFile === $file) { + return $file . ':' . ($frame['line'] ?? 0); + } + } + + return null; + } + + /** + * The file of the class an id names, or null when the id does not name a + * loaded one. Loading is deliberately not attempted: an id is an arbitrary + * string an adapter chose, and autoloading it would run a class map lookup + * — or an autoloader with side effects — for every property that fails. + */ + private static function propertyFile(string $propertyId): ?string + { + $separator = strpos($propertyId, '::'); + $class = $separator === false ? $propertyId : substr($propertyId, 0, $separator); + + if (!class_exists($class, autoload: false)) { + return null; + } + + $file = (new \ReflectionClass($class))->getFileName(); + + return $file === false ? null : $file; + } +} diff --git a/src/Runner/PropertyRunner.php b/src/Runner/PropertyRunner.php index fe583d5..ed76db1 100644 --- a/src/Runner/PropertyRunner.php +++ b/src/Runner/PropertyRunner.php @@ -30,6 +30,7 @@ use Rasuvaeff\PropertyTesting\GenerationExhaustedException; use Rasuvaeff\PropertyTesting\Internal\Domain; use Rasuvaeff\PropertyTesting\Internal\DrawContext; +use Rasuvaeff\PropertyTesting\Internal\FailureIdentity; use Rasuvaeff\PropertyTesting\Internal\PhaseCounters; use Rasuvaeff\PropertyTesting\Internal\ReplayVerdict; use Rasuvaeff\PropertyTesting\Internal\SearchPool; @@ -1029,11 +1030,12 @@ private function corpusCall(array $listeners, string $propertyId, string $operat * {@see ShrinkMode::Bounded}. * @param list $listeners * @param ?\Throwable $failure What the original run failed with. A candidate is accepted only - * when it fails the same way — the same exception class — so the descent minimises - * the bug that was found rather than sliding into a different one (a smaller input - * that trips a `TypeError` in the body's setup is not a smaller counterexample of an - * assertion failure). Null, or a run that failed without an exception, accepts any - * failure. + * when it fails the same way — the same exception class, raised at the same place in + * the property's own file ({@see FailureIdentity}) — so the descent minimises the bug + * that was found rather than sliding into a different one (a smaller input that trips + * a `TypeError` in the body's setup is not a smaller counterexample of an assertion + * failure, and neither is one that trips the body's *other* assertion). Null, or a + * run that failed without an exception, accepts any failure. * @return array{0: array, 1: array, 2: int, 3: ?\Throwable, 4: int, 5: string, 6: array, 7: array, 8: list} The * minimised arguments, the minimised draws (as `draw#N` pseudo-arguments), the number * of accepted shrink steps, the failure of the last accepted candidate (null when @@ -1061,6 +1063,11 @@ private function shrink( return [$this->values($trees), $this->drawArguments($tape), 0, null, 0, '', $notes, $trees, $tape]; } + // Built once: it reads the original failure's throw site, and every + // candidate is measured against that one identity rather than against + // the last accepted failure, so a descent cannot drift a step at a time. + $identity = FailureIdentity::of($propertyId, $failure, __FILE__); + $deadlineNs = $budgetMs === null ? null : $this->clock->nanoseconds() + $budgetMs * 1_000_000; $current = $trees; $currentTape = $tape; @@ -1104,7 +1111,7 @@ private function shrink( [$outcome, $recorded, $trialNotes] = $this->trial($executor, $trial, $currentTape, $random); ++$trials; - $accepted = $this->failsTheSameWay($outcome, $failure); + $accepted = $this->failsTheSameWay($outcome, $identity); $this->emit($listeners, new ShrinkTried($propertyId, $name, $candidate->value, $accepted)); if ($accepted) { @@ -1147,7 +1154,7 @@ private function shrink( [$outcome, $recorded, $trialNotes] = $this->trial($executor, $current, $trialTape, $random); ++$trials; - $accepted = $this->failsTheSameWay($outcome, $failure); + $accepted = $this->failsTheSameWay($outcome, $identity); $this->emit($listeners, new ShrinkTried($propertyId, 'draw#' . ($position + 1), $candidate->value, $accepted)); if ($accepted) { @@ -1199,20 +1206,14 @@ private function replayCounterExample(TrialExecutor $executor, array $trees, arr } /** - * Whether a shrink trial counts as a smaller counterexample: it failed, - * and with the same kind of failure the original run had (see - * {@see shrink()}). Without an original exception to compare against, or - * without one on the trial, any failure counts. + * Whether a shrink trial counts as a smaller counterexample: it failed, and + * with the same failure the original run had — the same exception class, + * raised at the same place in the property's own file. What "the same + * place" means, and when it is unknowable, is {@see FailureIdentity}. */ - private function failsTheSameWay(TrialOutcome $outcome, ?\Throwable $original): bool + private function failsTheSameWay(TrialOutcome $outcome, FailureIdentity $identity): bool { - if (!$outcome->isFailed()) { - return false; - } - - return !$original instanceof \Throwable - || !$outcome->failure instanceof \Throwable - || $outcome->failure::class === $original::class; + return $outcome->isFailed() && $identity->matches($outcome->failure); } /** diff --git a/tests/Internal/FailureIdentityTest.php b/tests/Internal/FailureIdentityTest.php new file mode 100644 index 0000000..fe9db86 --- /dev/null +++ b/tests/Internal/FailureIdentityTest.php @@ -0,0 +1,130 @@ +matches(new \RuntimeException('anything'))); + Assert::true($identity->matches(null)); + } + + public function aCandidateThatFailedWithoutAThrowableMatches(): void + { + $identity = FailureIdentity::of(self::class . '::property', new \RuntimeException('original'), self::ELSEWHERE); + + Assert::true($identity->matches(null)); + } + + public function aDifferentExceptionClassIsADifferentFailure(): void + { + $identity = FailureIdentity::of(self::class . '::property', new \RuntimeException('original'), self::ELSEWHERE); + + Assert::false($identity->matches(new \LogicException('original'))); + } + + public function theSameLineOfThePropertyFileIsTheSameFailure(): void + { + [$original, $candidate] = [new \RuntimeException('a'), new \RuntimeException('b')]; + + Assert::true(FailureIdentity::of(self::class . '::property', $original, self::ELSEWHERE)->matches($candidate)); + } + + public function anotherLineOfThePropertyFileIsAnotherFailure(): void + { + $original = new \RuntimeException('a'); + $candidate = new \RuntimeException('b'); + + Assert::false(FailureIdentity::of(self::class . '::property', $original, self::ELSEWHERE)->matches($candidate)); + } + + public function aFailureRaisedElsewhereIsPlacedAtTheLineThatCalledIn(): void + { + $raise = fn(string $message): \Throwable => $this->caught(static fn(): never => FailingHelper::raise($message)); + $original = $raise('a'); + $sameCall = $raise('b'); + $anotherCall = $this->caught(static fn(): never => FailingHelper::raise('c')); + + $identity = FailureIdentity::of(self::class . '::property', $original, self::ELSEWHERE); + + // The throw is the helper's line either way; the property's line is not. + Assert::true($identity->matches($sameCall)); + Assert::false($identity->matches($anotherCall)); + } + + public function anIdThatNamesNoLoadedClassLeavesTheClassAsTheWholeIdentity(): void + { + $original = new \RuntimeException('a'); + $candidate = new \RuntimeException('b'); + + foreach (['property', 'Rasuvaeff\\PropertyTesting\\Tests\\Internal\\NoSuchTest::property'] as $id) { + $identity = FailureIdentity::of($id, $original, self::ELSEWHERE); + + Assert::true($identity->matches($candidate)); + Assert::false($identity->matches(new \LogicException('b'))); + } + } + + public function theEngineBoundaryEndsTheSearchForThePlace(): void + { + // Beyond the engine's own file the stack is the engine calling the body. + // The line of this file that called in lies there, so it names no place + // in the property: an adapter that invokes the body from the test file + // must not report its own call as the site of every failure. + $original = $this->caught(static fn(): never => EngineStand::raise('a')); + $candidate = $this->caught(static fn(): never => EngineStand::raise('b')); + $boundary = (string) (new \ReflectionClass(EngineStand::class))->getFileName(); + + Assert::true(FailureIdentity::of(self::class . '::property', $original, $boundary)->matches($candidate)); + // Without the bound, the two calls into the stand are two places. + Assert::false(FailureIdentity::of(self::class . '::property', $original, self::ELSEWHERE)->matches($candidate)); + } + + public function aCandidateWhosePlaceIsUnknownMatchesOnTheClass(): void + { + $original = new \RuntimeException('a'); + $candidate = $this->caught(static fn(): never => EngineStand::raise('b')); + $boundary = (string) (new \ReflectionClass(EngineStand::class))->getFileName(); + + $identity = FailureIdentity::of(self::class . '::property', $original, $boundary); + + Assert::true($identity->matches($candidate)); + Assert::false($identity->matches(new \LogicException('b'))); + } + + private function caught(\Closure $body): \Throwable + { + try { + $body(); + } catch (\Throwable $failure) { + return $failure; + } + + throw new \LogicException('The body was expected to throw'); + } +} diff --git a/tests/Runner/PropertyRunnerShrinkTest.php b/tests/Runner/PropertyRunnerShrinkTest.php index 296d25d..66a8a21 100644 --- a/tests/Runner/PropertyRunnerShrinkTest.php +++ b/tests/Runner/PropertyRunnerShrinkTest.php @@ -19,6 +19,7 @@ use Rasuvaeff\PropertyTesting\Runner\TrialOutcome; use Rasuvaeff\PropertyTesting\Tests\Support\ChainArbitrary; use Rasuvaeff\PropertyTesting\Tests\Support\CollectingListener; +use Rasuvaeff\PropertyTesting\Tests\Support\FailingHelper; use Rasuvaeff\PropertyTesting\Tests\Support\ThrowingShrinkArbitrary; use Testo\Assert; use Testo\Codecov\Covers; @@ -456,6 +457,112 @@ public function counterexampleMergesParametersAndDraws(): void * @param array $generators * @param list $parameterNames */ + public function shrinkingKeepsToTheLineThatFailed(): void + { + // Both throws are RuntimeExceptions, so the class alone cannot tell + // them apart: a candidate below 100 fails at the second line, which is + // a different bug. The descent stops at 100 instead of reporting 0 as + // the minimal counterexample of a failure 0 never produced. + $result = (new PropertyRunner())->run( + $this->ownDefinition('shrinkingKeepsToTheLineThatFailed', ['value' => Gen::intBetween(0, 10_000)]), + new CallableTrialExecutor(static function (int $value): void { + if ($value >= 100) { + throw new \RuntimeException('at or above 100'); + } + + throw new \RuntimeException('below 100'); + }), + ); + + Assert::instanceOf($result, Falsified::class); + $example = $result->counterExample(); + Assert::same($example->originalArguments, ['value' => 3989]); + Assert::same($example->shrunkArguments, ['value' => 100]); + Assert::same($example->failure?->getMessage(), 'at or above 100'); + } + + public function shrinkingKeepsToTheCallThatFailed(): void + { + // Both failures are raised from the same line of another file — what an + // assertion library does to every assertion in a body. The line that + // tells them apart is the one in the property's own file that called in. + $result = (new PropertyRunner())->run( + $this->ownDefinition('shrinkingKeepsToTheCallThatFailed', ['value' => Gen::intBetween(0, 10_000)]), + new CallableTrialExecutor(static function (int $value): void { + if ($value >= 100) { + FailingHelper::raise('at or above 100'); + } + + FailingHelper::raise('below 100'); + }), + ); + + Assert::instanceOf($result, Falsified::class); + $example = $result->counterExample(); + Assert::same($example->shrunkArguments, ['value' => 100]); + Assert::same($example->failure?->getMessage(), 'at or above 100'); + } + + public function anIdThatNamesNoClassLeavesTheClassAsTheWholeIdentity(): void + { + // The place a failure came from is only readable when the id names a + // loaded class. Without it the criterion is what it always was, and the + // descent slides into the second line: that is the cost of an + // adapter-supplied id string, not a silent stall. + $result = (new PropertyRunner())->run( + $this->definition(['value' => Gen::intBetween(0, 10_000)], ['value']), + new CallableTrialExecutor(static function (int $value): void { + if ($value >= 100) { + throw new \RuntimeException('at or above 100'); + } + + throw new \RuntimeException('below 100'); + }), + ); + + Assert::instanceOf($result, Falsified::class); + $example = $result->counterExample(); + Assert::same($example->shrunkArguments, ['value' => 0]); + Assert::same($example->failure?->getMessage(), 'below 100'); + } + + public function aFailureReachedThroughStacksOfDifferentHeightStillShrinks(): void + { + // The identity is one place, not a stack: the helper recurses as deep as + // the input is large, and every trial arrives at the same body line. A + // criterion that compared traces would refuse every smaller candidate + // and report the input it started from. + $result = (new PropertyRunner())->run( + $this->ownDefinition('aFailureReachedThroughStacksOfDifferentHeightStillShrinks', ['value' => Gen::intBetween(0, 10_000)]), + new CallableTrialExecutor(static function (int $value): void { + if ($value >= 100) { + FailingHelper::raiseFromDepth('at or above 100', intdiv($value, 250)); + } + }), + ); + + Assert::instanceOf($result, Falsified::class); + $example = $result->counterExample(); + Assert::same($example->shrunkArguments, ['value' => 100]); + } + + /** + * A definition whose id names this very class, so the engine can read the + * file the property bodies above are written in. + * + * @param array $generators + */ + private function ownDefinition(string $method, array $generators): PropertyDefinition + { + return new PropertyDefinition( + id: self::class . '::' . $method, + name: $method, + generators: $generators, + parameterNames: array_keys($generators), + config: new PropertyConfig(runs: 200, seed: 42), + ); + } + private function definition( array $generators, array $parameterNames, diff --git a/tests/Support/EngineStand.php b/tests/Support/EngineStand.php new file mode 100644 index 0000000..44e2e72 --- /dev/null +++ b/tests/Support/EngineStand.php @@ -0,0 +1,24 @@ + $depth + */ + public static function raiseFromDepth(string $message, int $depth): never + { + if ($depth > 0) { + self::raiseFromDepth($message, $depth - 1); + } + + throw new \RuntimeException($message); + } +} From 2d6cc639d63809d99e2ca3fb7568ddd316460240 Mon Sep 17 00:00:00 2001 From: "v.razuvaev" Date: Mon, 28 Sep 2026 10:16:17 +0300 Subject: [PATCH 2/3] Place a failure by line, and kill every mutant of the criterion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The site is a line of the property's own file — the file is fixed for the whole identity, so carrying it in the compared string only made the concat unkillable. Two mutants that no test could reach are gone rather than ignored: the unknown-file guard in the search (an unknown file matches neither the throwable's nor any frame's, so the search already returns nothing) and the autoload flag, now pinned by a test that registers an autoloader and asserts an id never reaches it. A frame with no file of its own — a callback invoked by an internal function — is stepped over, not taken as the end of the search. --- src/Internal/FailureIdentity.php | 42 +++++++++------- tests/Internal/FailureIdentityTest.php | 67 ++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 17 deletions(-) diff --git a/src/Internal/FailureIdentity.php b/src/Internal/FailureIdentity.php index 698468c..2907bfa 100644 --- a/src/Internal/FailureIdentity.php +++ b/src/Internal/FailureIdentity.php @@ -43,7 +43,7 @@ private function __construct( private ?string $class, private ?string $file, private string $boundary, - private ?string $site, + private ?int $site, ) {} /** @@ -64,10 +64,10 @@ public static function of(string $propertyId, ?\Throwable $failure, string $boun $file = self::propertyFile($propertyId); return new self( - class: $failure === null ? null : $failure::class, + class: $failure instanceof \Throwable ? $failure::class : null, file: $file, boundary: $boundary, - site: $failure === null ? null : self::site($failure, $file, $boundary), + site: $failure instanceof \Throwable ? self::site($failure, $file, $boundary) : null, ); } @@ -76,7 +76,7 @@ class: $failure === null ? null : $failure::class, */ public function matches(?\Throwable $candidate): bool { - if ($this->class === null || $candidate === null) { + if ($this->class === null || !$candidate instanceof \Throwable) { return true; } @@ -94,29 +94,35 @@ public function matches(?\Throwable $candidate): bool } /** - * Where in $file the failure was raised: the throwable's own line when the + * Which line of $file raised the failure: the throwable's own line when the * body threw it, otherwise the line of the innermost frame that is still in * the body — an assertion helper is entered from the line the body wrote. + * + * A line is enough to tell two places apart because the file is fixed for + * the whole identity: the frame scan only ever matches $file, and a + * throwable from anywhere else has no place in the property at all. */ - private static function site(\Throwable $failure, ?string $file, string $boundary): ?string + private static function site(\Throwable $failure, ?string $file, string $boundary): ?int { - if ($file === null) { - return null; - } - + // An unknown file needs no guard of its own: it is neither the file the + // throwable names nor any frame's, so the search below finds nothing. if ($failure->getFile() === $file) { - return $file . ':' . $failure->getLine(); + return $failure->getLine(); } foreach ($failure->getTrace() as $frame) { - $frameFile = $frame['file'] ?? null; + if (!isset($frame['file'], $frame['line'])) { + // An internal call — a property method reached through + // `ReflectionMethod::invoke()` — has no place of its own. + continue; + } - if ($frameFile === $boundary) { + if ($frame['file'] === $boundary) { return null; } - if ($frameFile === $file) { - return $file . ':' . ($frame['line'] ?? 0); + if ($frame['file'] === $file) { + return $frame['line']; } } @@ -126,8 +132,10 @@ private static function site(\Throwable $failure, ?string $file, string $boundar /** * The file of the class an id names, or null when the id does not name a * loaded one. Loading is deliberately not attempted: an id is an arbitrary - * string an adapter chose, and autoloading it would run a class map lookup - * — or an autoloader with side effects — for every property that fails. + * string an adapter chose, and autoloading it would put every failing + * property's id through the autoloader chain — a class map lookup at best, + * an autoloader with side effects at worst. A property that runs has its + * class loaded already; anything else is not a class. */ private static function propertyFile(string $propertyId): ?string { diff --git a/tests/Internal/FailureIdentityTest.php b/tests/Internal/FailureIdentityTest.php index fe9db86..91499aa 100644 --- a/tests/Internal/FailureIdentityTest.php +++ b/tests/Internal/FailureIdentityTest.php @@ -77,17 +77,84 @@ public function aFailureRaisedElsewhereIsPlacedAtTheLineThatCalledIn(): void Assert::false($identity->matches($anotherCall)); } + public function aCallWithoutAPlaceOfItsOwnDoesNotEndTheSearch(): void + { + // `array_map` calls the helper from internal code, so the frame of that + // call has no file at all. The search steps over it and reaches the line + // of this file that called in — giving up there would place two calls + // nowhere and make them one failure. + $original = $this->caught(static fn(): array => array_map(FailingHelper::raise(...), ['a'])); + $another = $this->caught(static fn(): array => array_map(FailingHelper::raise(...), ['b'])); + + Assert::false(FailureIdentity::of(self::class . '::property', $original, self::ELSEWHERE)->matches($another)); + } + public function anIdThatNamesNoLoadedClassLeavesTheClassAsTheWholeIdentity(): void { $original = new \RuntimeException('a'); $candidate = new \RuntimeException('b'); + $raisedElsewhere = $this->caught(static fn(): never => FailingHelper::raise('a')); + $raisedElsewhereToo = $this->caught(static fn(): never => FailingHelper::raise('b')); + foreach (['property', 'Rasuvaeff\\PropertyTesting\\Tests\\Internal\\NoSuchTest::property'] as $id) { $identity = FailureIdentity::of($id, $original, self::ELSEWHERE); Assert::true($identity->matches($candidate)); Assert::false($identity->matches(new \LogicException('b'))); + + // Not even a frame of this file is a place without a file to compare it to. + Assert::true(FailureIdentity::of($id, $raisedElsewhere, self::ELSEWHERE)->matches($raisedElsewhereToo)); + } + } + + public function anIdIsNeverPutThroughTheAutoloader(): void + { + // A failing property's id reaches this code on every descent, and an id + // is an arbitrary string: resolving it must not run autoloaders. + /** @var list $attempts */ + $attempts = []; + $autoloader = static function (string $class) use (&$attempts): void { + $attempts[] = $class; + }; + + spl_autoload_register($autoloader); + + try { + FailureIdentity::of(self::class . 'NoSuchClass::property', new \RuntimeException('a'), self::ELSEWHERE); + } finally { + spl_autoload_unregister($autoloader); } + + Assert::same($attempts, []); + } + + public function anIdWhoseClassHasNoFileLeavesTheClassAsTheWholeIdentity(): void + { + // An internal class is loaded and has no file: a property cannot be + // written in one, and the place is unreadable rather than wrong. + $original = new \RuntimeException('a'); + $candidate = new \RuntimeException('b'); + + $identity = FailureIdentity::of(\ArrayObject::class . '::property', $original, self::ELSEWHERE); + + Assert::true($identity->matches($candidate)); + Assert::false($identity->matches(new \LogicException('b'))); + } + + public function anOriginalWithoutAPlaceAcceptsACandidateThatHasOne(): void + { + // The identity is only as sharp as what it could read about the original: + // a failure the engine raised, or one whose stack reaches the engine + // before it reaches the property, is compared on its class alone — and a + // candidate whose place IS readable must not be refused for having one. + $original = $this->caught(static fn(): never => EngineStand::raise('a')); + $boundary = (string) (new \ReflectionClass(EngineStand::class))->getFileName(); + + $identity = FailureIdentity::of(self::class . '::property', $original, $boundary); + + Assert::true($identity->matches(new \RuntimeException('b'))); + Assert::false($identity->matches(new \LogicException('b'))); } public function theEngineBoundaryEndsTheSearchForThePlace(): void From 1280fa810c35fee5d9cbb403e652c6fe063577f9 Mon Sep 17 00:00:00 2001 From: "v.razuvaev" Date: Mon, 28 Sep 2026 10:39:49 +0300 Subject: [PATCH 3/3] Document the criterion on the docs site, and sharpen the wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shrinking guide described the loop but never said what "still fails" means, which is now the interesting part. States the criterion, both fallbacks, and the one consequence a user of stateful properties should know: the rule-based facade's #[Rule] methods live in the test file, so a failure that moves from one rule to another is a different failure. "Not the throw itself" was also wrong in the README and the changelog for a body that throws directly — there the throwable's own line IS the place. --- CHANGELOG.md | 10 +++++++--- README.md | 7 ++++--- README.ru.md | 7 ++++--- docs/src/guide/shrinking.md | 27 +++++++++++++++++++++++++++ 4 files changed, 42 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16ed832..0266c10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,14 +9,18 @@ offer the descent failures it could not tell apart, so the reported minimal counterexample could belong to a bug the run never found, with nothing in the output saying so. The place is the line of the property's own file in the - failure's stack, never the throw itself: an assertion library raises every - assertion of a body from one line of its own. Where the place cannot be read — + failure's stack, which is not where the exception was raised: an assertion + library raises every assertion of a body from one line of its own, while the + line that called in is the assertion the body wrote. Where the place cannot be read — an id that names no loaded class, a body written in another file (a trait, an included fixture), a failure the engine raised rather than the body — the exception class stays the whole criterion. Minimised counterexamples may therefore differ from 1.0.0 on such a body (compatibility policy §2: what a descent minimises to is not under SemVer); a corpus recorded before this - release replays unchanged. + release replays unchanged. Worth knowing for stateful properties: the + rule-based façade's `#[Rule]` methods live in the test file, so a command + sequence whose failure moves from one rule to another now stops the descent + rather than being minimised into the other rule's bug. ## 1.0.0 — 2026-09-20 diff --git a/README.md b/README.md index 44e6264..62f9203 100644 --- a/README.md +++ b/README.md @@ -111,9 +111,10 @@ property's own file — so the descent minimises the bug that was found instead sliding into a different one (a smaller input that trips a `TypeError` in the body's setup is not a smaller counterexample of an assertion failure, and neither is one that trips the body's *other* assertion). The place is the line of the -property's file in the failure's stack, not the throw itself: an assertion -library raises every assertion of a body from one line of its own, so that line -would tell two assertions apart from nothing. Where the place cannot be read — +property's own file in the failure's stack, which is not where the exception was +raised: an assertion library raises every assertion of a body from one line of +its own, so that line would tell two assertions apart from nothing, while the +line that called in is the assertion the body wrote. Where the place cannot be read — a property id that names no loaded class, a body written in another file (a trait, an included fixture), a failure the engine raised rather than the body — the exception class is the whole criterion, as it was before. Reproduce the exact diff --git a/README.ru.md b/README.ru.md index d6ed2e2..7ec2aec 100644 --- a/README.ru.md +++ b/README.ru.md @@ -112,9 +112,10 @@ shrinker'а (принятые и отвергнутые); `shrink step(s)` — минимизирует найденный баг, а не сползает в другой (меньший вход, роняющий `TypeError` в подготовке тела, — не меньший контрпример для assertion-падения, и таким же не является вход, роняющий *другой* assert того же тела). Место — -это строка файла property в стеке падения, а не сам throw: assert-библиотека -роняет любой assert тела из одной своей строки, так что она не отличила бы два -assert'а ни от чего. Когда место прочитать нельзя — id property не называет +это строка собственного файла property в стеке падения, а не то место, где +исключение подняли: assert-библиотека роняет любой assert тела из одной своей +строки, так что она не отличила бы два assert'а ни от чего, тогда как строка, +из которой вызвали, — это и есть написанный в теле assert. Когда место прочитать нельзя — id property не называет загруженный класс, тело написано в другом файле (трейт, подключённая фикстура), падение подняла не body, а движок — критерием остаётся один класс исключения, как и раньше. Точное воспроизведение прогона — запиньте seed из отчёта в diff --git a/docs/src/guide/shrinking.md b/docs/src/guide/shrinking.md index 6e05681..e5046b0 100644 --- a/docs/src/guide/shrinking.md +++ b/docs/src/guide/shrinking.md @@ -50,6 +50,33 @@ step(s)` counts only the accepted descents. You can cap the descent with get the original counterexample unchanged — see [Bounding shrink work](/guide/controlling-runs/bounding-shrink). +## What counts as still failing + +"Still fails" is stricter than *fails*. A candidate is accepted only when it +fails **the same way** the original run did: the same exception class, raised at +the same place in the property's own file. A body can fail in more than one place +— two assertions, or an assertion beside a call that throws the same type — and +without that the descent would leave the failure it was given and minimise +another one, reporting a minimal counterexample for a bug the run never found. + +The place is the line of the property's own file in the failure's stack, which is +not the same as where the exception was raised: an assertion library raises every +assertion of a body from one line of its own (PHPUnit from `Constraint::fail()`, +Testo from `Assert`), so that line tells two assertions apart from nothing, while +the line that called in is exactly the assertion the body wrote. + +Where the place cannot be read, the exception class is the whole criterion — the +engine never guesses one, because a criterion that refused candidates failing +exactly as the original did would stall the descent on the input it started from. +That is the case when the property id names no loaded class (an adapter-supplied +id string), when the body is written in another file (a trait, an included +fixture), and when the failure was raised by the engine rather than by the body. + +A stateful property is worth a note here: with the rule-based façade the +`#[Rule]` methods live in the test file, so a command sequence whose failure +moves from one rule to another is now a different failure, and the descent stops +before it. + ## The termination invariant A shrink tree is only useful if walking it is guaranteed to stop. Two rules,