From defe6ae0049c01a1cadfa2e459e356a8abb32700 Mon Sep 17 00:00:00 2001 From: Ross Addison Date: Mon, 6 Jul 2026 15:45:28 +0100 Subject: [PATCH 1/9] feat(testing): add ExpectTestStatus, ExpectAssertionsCount, ExpectTestResultAttribute (#36) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three PHP attributes allow a test that uses TestRunner::runTest() to assert properties of the inner stub TestResult without writing explicit assertion code: - #[ExpectTestStatus(Status::X)] — validates stub.status - #[ExpectAssertionsCount(N)] — validates stub.summary.metric('assertions') - #[ExpectTestResultAttribute(K)] — validates stub.getAttribute(K) is not null (repeatable) ExpectInterceptor reads these from the test method's reflection, runs the test, extracts outerResult->result as the stub TestResult, and converts any mismatches into a single Status::Failed with a combined message. Pre-existing outer failures are preserved untouched. Registered automatically via InjectPlugin. Co-Authored-By: Claude Sonnet 4.6 --- .../Attribute/ExpectAssertionsCount.php | 24 ++ .../Attribute/ExpectTestResultAttribute.php | 24 ++ core/Testing/Attribute/ExpectTestStatus.php | 24 ++ core/Testing/InjectPlugin.php | 2 + core/Testing/Internal/ExpectInterceptor.php | 98 ++++++++ .../Testing/Unit/ExpectInterceptorTest.php | 209 ++++++++++++++++++ 6 files changed, 381 insertions(+) create mode 100644 core/Testing/Attribute/ExpectAssertionsCount.php create mode 100644 core/Testing/Attribute/ExpectTestResultAttribute.php create mode 100644 core/Testing/Attribute/ExpectTestStatus.php create mode 100644 core/Testing/Internal/ExpectInterceptor.php create mode 100644 tests/Core/Testing/Unit/ExpectInterceptorTest.php diff --git a/core/Testing/Attribute/ExpectAssertionsCount.php b/core/Testing/Attribute/ExpectAssertionsCount.php new file mode 100644 index 00000000..1b68552b --- /dev/null +++ b/core/Testing/Attribute/ExpectAssertionsCount.php @@ -0,0 +1,24 @@ + $count Expected number of assertions. */ + public function __construct(public int $count) {} +} diff --git a/core/Testing/Attribute/ExpectTestResultAttribute.php b/core/Testing/Attribute/ExpectTestResultAttribute.php new file mode 100644 index 00000000..e691ad32 --- /dev/null +++ b/core/Testing/Attribute/ExpectTestResultAttribute.php @@ -0,0 +1,24 @@ +get(InterceptorCollector::class)->addInterceptor(InjectInterceptor::class); + $container->get(InterceptorCollector::class)->addInterceptor(new ExpectInterceptor()); } } diff --git a/core/Testing/Internal/ExpectInterceptor.php b/core/Testing/Internal/ExpectInterceptor.php new file mode 100644 index 00000000..f5985537 --- /dev/null +++ b/core/Testing/Internal/ExpectInterceptor.php @@ -0,0 +1,98 @@ +testDefinition->reflection; + + $statusAttrs = Reflection::fetchFunctionAttributes($reflection, attributeClass: ExpectTestStatus::class); + $countAttrs = Reflection::fetchFunctionAttributes($reflection, attributeClass: ExpectAssertionsCount::class); + $attrAttrs = Reflection::fetchFunctionAttributes($reflection, attributeClass: ExpectTestResultAttribute::class); + + if ($statusAttrs === [] && $countAttrs === [] && $attrAttrs === []) { + return $next($info); + } + + $outerResult = $next($info); + + // Pre-existing failure or non-terminal status — preserve as-is so the original error is + // not obscured by a misleading "expected TestResult" message. + if (!$outerResult->status->isCompleted() || $outerResult->status->isFailure()) { + return $outerResult; + } + + $stubResult = $outerResult->result; + if (!$stubResult instanceof TestResult) { + return $outerResult + ->with(status: Status::Failed) + ->withFailure(new \LogicException( + 'Test must return the TestResult from TestRunner::runTest() when using Expect* attributes, got ' + . \get_debug_type($stubResult), + )); + } + + $failures = []; + + if ($statusAttrs !== []) { + /** @var ExpectTestStatus $expect */ + $expect = $statusAttrs[0]->newInstance(); + if ($stubResult->status !== $expect->status) { + $failures[] = "Expected stub status {$expect->status->name}, got {$stubResult->status->name}"; + } + } + + if ($countAttrs !== []) { + /** @var ExpectAssertionsCount $expect */ + $expect = $countAttrs[0]->newInstance(); + $actual = $stubResult->summary->metric('assertions'); + if ($actual !== $expect->count) { + $failures[] = "Expected {$expect->count} assertion(s), got {$actual}"; + } + } + + foreach ($attrAttrs as $attr) { + /** @var ExpectTestResultAttribute $expect */ + $expect = $attr->newInstance(); + if ($stubResult->getAttribute($expect->name) === null) { + $failures[] = "Expected TestResult attribute '{$expect->name}' to be present"; + } + } + + if ($failures === []) { + return $outerResult; + } + + return $outerResult + ->with(status: Status::Failed) + ->withFailure(new \RuntimeException(\implode("\n", $failures))); + } +} diff --git a/tests/Core/Testing/Unit/ExpectInterceptorTest.php b/tests/Core/Testing/Unit/ExpectInterceptorTest.php new file mode 100644 index 00000000..262cde2e --- /dev/null +++ b/tests/Core/Testing/Unit/ExpectInterceptorTest.php @@ -0,0 +1,209 @@ + $inner; + + $result = (new ExpectInterceptor())->runTest($info, $next); + + Assert::same($result, $inner); + } + + public function passesWhenStubStatusMatchesExpected(): void + { + $info = self::createTestInfoFor('fixtureExpectFailed'); + $stub = new TestResult(info: $info, status: Status::Failed); + $outer = new TestResult(info: $info, status: Status::Passed, result: $stub); + $next = static fn(TestInfo $i): TestResult => $outer; + + $result = (new ExpectInterceptor())->runTest($info, $next); + + Assert::same(Status::Passed, $result->status); + } + + public function failsWhenStubStatusDoesNotMatchExpected(): void + { + $info = self::createTestInfoFor('fixtureExpectFailed'); + $stub = new TestResult(info: $info, status: Status::Passed); + $outer = new TestResult(info: $info, status: Status::Passed, result: $stub); + $next = static fn(TestInfo $i): TestResult => $outer; + + $result = (new ExpectInterceptor())->runTest($info, $next); + + Assert::same(Status::Failed, $result->status); + Assert::instanceOf($result->failure, \RuntimeException::class); + } + + public function passesWhenAssertionCountMatches(): void + { + $info = self::createTestInfoFor('fixtureExpectThreeAssertions'); + $stub = new TestResult( + info: $info, + status: Status::Passed, + summary: new Summary(metrics: ['assertions' => 3]), + ); + $outer = new TestResult(info: $info, status: Status::Passed, result: $stub); + $next = static fn(TestInfo $i): TestResult => $outer; + + $result = (new ExpectInterceptor())->runTest($info, $next); + + Assert::same(Status::Passed, $result->status); + } + + public function failsWhenAssertionCountDoesNotMatch(): void + { + $info = self::createTestInfoFor('fixtureExpectThreeAssertions'); + $stub = new TestResult( + info: $info, + status: Status::Passed, + summary: new Summary(metrics: ['assertions' => 2]), + ); + $outer = new TestResult(info: $info, status: Status::Passed, result: $stub); + $next = static fn(TestInfo $i): TestResult => $outer; + + $result = (new ExpectInterceptor())->runTest($info, $next); + + Assert::same(Status::Failed, $result->status); + } + + public function passesWhenExpectedResultAttributeIsPresent(): void + { + $info = self::createTestInfoFor('fixtureExpectFooAttribute'); + $stub = (new TestResult(info: $info, status: Status::Passed)) + ->withAttribute('foo', 'bar'); + $outer = new TestResult(info: $info, status: Status::Passed, result: $stub); + $next = static fn(TestInfo $i): TestResult => $outer; + + $result = (new ExpectInterceptor())->runTest($info, $next); + + Assert::same(Status::Passed, $result->status); + } + + public function failsWhenExpectedResultAttributeIsAbsent(): void + { + $info = self::createTestInfoFor('fixtureExpectFooAttribute'); + $stub = new TestResult(info: $info, status: Status::Passed); + $outer = new TestResult(info: $info, status: Status::Passed, result: $stub); + $next = static fn(TestInfo $i): TestResult => $outer; + + $result = (new ExpectInterceptor())->runTest($info, $next); + + Assert::same(Status::Failed, $result->status); + } + + public function repeatableAttributeChecksAllKeys(): void + { + $info = self::createTestInfoFor('fixtureExpectTwoAttributes'); + // Only 'alpha' present; 'beta' is missing → should fail + $stub = (new TestResult(info: $info, status: Status::Passed)) + ->withAttribute('alpha', 1); + $outer = new TestResult(info: $info, status: Status::Passed, result: $stub); + $next = static fn(TestInfo $i): TestResult => $outer; + + $result = (new ExpectInterceptor())->runTest($info, $next); + + Assert::same(Status::Failed, $result->status); + } + + public function preservesOuterFailureWithoutRunningValidation(): void + { + $info = self::createTestInfoFor('fixtureExpectFailed'); + $outer = new TestResult(info: $info, status: Status::Failed); + $next = static fn(TestInfo $i): TestResult => $outer; + + $result = (new ExpectInterceptor())->runTest($info, $next); + + Assert::same($result, $outer); + } + + public function failsWithLogicExceptionWhenResultIsNotATestResult(): void + { + $info = self::createTestInfoFor('fixtureExpectFailed'); + $outer = new TestResult(info: $info, status: Status::Passed, result: 'not-a-test-result'); + $next = static fn(TestInfo $i): TestResult => $outer; + + $result = (new ExpectInterceptor())->runTest($info, $next); + + Assert::same(Status::Failed, $result->status); + Assert::instanceOf($result->failure, \LogicException::class); + } + + public function combinesMultipleViolationsIntoSingleFailure(): void + { + $info = self::createTestInfoFor('fixtureExpectPassedWithThreeAssertions'); + // Stub is Failed with 1 assertion — both status and count fail + $stub = new TestResult( + info: $info, + status: Status::Failed, + summary: new Summary(metrics: ['assertions' => 1]), + ); + $outer = new TestResult(info: $info, status: Status::Passed, result: $stub); + $next = static fn(TestInfo $i): TestResult => $outer; + + $result = (new ExpectInterceptor())->runTest($info, $next); + + Assert::same(Status::Failed, $result->status); + } + + // ── Attribute fixtures ──────────────────────────────────────────────────── + + private function noAttributes(): void {} + + #[ExpectTestStatus(Status::Failed)] + private function fixtureExpectFailed(): void {} + + #[ExpectAssertionsCount(3)] + private function fixtureExpectThreeAssertions(): void {} + + #[ExpectTestResultAttribute('foo')] + private function fixtureExpectFooAttribute(): void {} + + #[ExpectTestResultAttribute('alpha')] + #[ExpectTestResultAttribute('beta')] + private function fixtureExpectTwoAttributes(): void {} + + #[ExpectTestStatus(Status::Passed)] + #[ExpectAssertionsCount(3)] + private function fixtureExpectPassedWithThreeAssertions(): void {} + + // ── Helpers ─────────────────────────────────────────────────────────────── + + private static function createTestInfoFor(string $method): TestInfo + { + $reflection = new \ReflectionMethod(self::class, $method); + $caseDefinition = new CaseDefinition(name: 'ExpectInterceptorTest', type: 'test'); + $caseInfo = new CaseInfo(definition: $caseDefinition); + $testDefinition = new TestDefinition(reflection: $reflection); + + return new TestInfo( + name: $method, + caseInfo: $caseInfo, + testDefinition: $testDefinition, + ); + } +} From 49386b3b501f2b09eabdfcbe5696fd9f6636911e Mon Sep 17 00:00:00 2001 From: Ross Addison Date: Mon, 6 Jul 2026 15:55:33 +0100 Subject: [PATCH 2/9] fix(output): widen Style::dim to string; fix ChannelRenderer::formatTime return type Style::dim() worked correctly with any string (empty or not) but declared @param non-empty-string, which Psalm flagged at every call site where a plain string was passed. Removed the over-restrictive annotation. ChannelRenderer::formatTime() claimed @return non-empty-string with a /** @var non-empty-string */ inline cast, which Psalm 7 does not accept. Replaced date() with integer arithmetic + sprintf so the implementation is cleaner, and removed the annotation since Psalm 7 does not narrow sprintf to non-empty-string for this version. Co-Authored-By: Claude Sonnet 4.6 --- core/Output/Rendering/ChannelRenderer.php | 12 ++++++------ core/Output/Terminal/Renderer/Style.php | 2 -- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/core/Output/Rendering/ChannelRenderer.php b/core/Output/Rendering/ChannelRenderer.php index 6b74a174..68cec020 100644 --- a/core/Output/Rendering/ChannelRenderer.php +++ b/core/Output/Rendering/ChannelRenderer.php @@ -98,15 +98,15 @@ private static function header(string $channel, float $time): string /** * Formats a {@see \microtime()} timestamp as `HH:MM:SS.mmm` wall-clock time. - * - * @return non-empty-string */ private static function formatTime(float $time): string { - $seconds = (int) $time; - $millis = \min(999, (int) \round(($time - (float) $seconds) * 1000.0)); + $totalSeconds = (int) $time; + $millis = \min(999, (int) \round(($time - (float) $totalSeconds) * 1000.0)); + $s = $totalSeconds % 60; + $m = (int) ($totalSeconds / 60) % 60; + $h = (int) ($totalSeconds / 3600) % 24; - /** @var non-empty-string */ - return \date('H:i:s', $seconds) . \sprintf('.%03d', $millis); + return \sprintf('%02d:%02d:%02d.%03d', $h, $m, $s, $millis); } } diff --git a/core/Output/Terminal/Renderer/Style.php b/core/Output/Terminal/Renderer/Style.php index b1e03b3d..46b4cc97 100644 --- a/core/Output/Terminal/Renderer/Style.php +++ b/core/Output/Terminal/Renderer/Style.php @@ -59,8 +59,6 @@ public static function bold(string $text): string /** * Makes text dim (less visible). - * - * @param non-empty-string $text */ public static function dim(string $text): string { From 37373cf3a657b22aae732deae58fb8671fdad48f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 6 Jul 2026 14:46:14 +0000 Subject: [PATCH 3/9] style(cs): apply php-cs-fixer --- core/Testing/Attribute/ExpectAssertionsCount.php | 4 +++- core/Testing/Attribute/ExpectTestResultAttribute.php | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/core/Testing/Attribute/ExpectAssertionsCount.php b/core/Testing/Attribute/ExpectAssertionsCount.php index 1b68552b..682af4cc 100644 --- a/core/Testing/Attribute/ExpectAssertionsCount.php +++ b/core/Testing/Attribute/ExpectAssertionsCount.php @@ -19,6 +19,8 @@ #[\Attribute(\Attribute::TARGET_METHOD | \Attribute::TARGET_FUNCTION)] final readonly class ExpectAssertionsCount { - /** @param int<0, max> $count Expected number of assertions. */ + /** + * @param int<0, max> $count Expected number of assertions. + */ public function __construct(public int $count) {} } diff --git a/core/Testing/Attribute/ExpectTestResultAttribute.php b/core/Testing/Attribute/ExpectTestResultAttribute.php index e691ad32..bf323219 100644 --- a/core/Testing/Attribute/ExpectTestResultAttribute.php +++ b/core/Testing/Attribute/ExpectTestResultAttribute.php @@ -19,6 +19,8 @@ #[\Attribute(\Attribute::TARGET_METHOD | \Attribute::TARGET_FUNCTION | \Attribute::IS_REPEATABLE)] final readonly class ExpectTestResultAttribute { - /** @param non-empty-string $name Attribute key to look up, typically a class-string. */ + /** + * @param non-empty-string $name Attribute key to look up, typically a class-string. + */ public function __construct(public string $name) {} } From 22ea9a2a5875ce9c19b971f3d46db09a8e8e4097 Mon Sep 17 00:00:00 2001 From: Ross Addison Date: Mon, 6 Jul 2026 18:39:18 +0100 Subject: [PATCH 4/9] fix(phpunit-mirror): add .placeholder.php so EmptyRun stub directory is mirrored MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tests/Application/Stub/EmptyRun/ is intentionally empty — it is the test fixture for EmptyRunTest, which asserts that a Testo run over an empty directory yields Status::Risky with zero tests collected. Git does not track empty directories, and bin/build-phpunit.php only copies *.php files when populating the tests/PhpUnit/ mirror, so the mirror never contained tests/PhpUnit/Application/Stub/EmptyRun/. The mirrored EmptyRunTest resolved __DIR__ . '/../../Stub/EmptyRun' to that missing path and threw InvalidArgumentException: File or directory not found — aborting Infection's initial PHPUnit test run on every CI push to 1.x. Add .placeholder.php (no namespace, no classes, no tests) to the source directory. The build script copies it verbatim into the mirror, which creates the required directory. Testo's FinderConfig still discovers zero tests there, so Status::Risky is reported and the assertion holds. Co-Authored-By: Claude Sonnet 5 --- tests/Application/Stub/EmptyRun/.placeholder.php | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 tests/Application/Stub/EmptyRun/.placeholder.php diff --git a/tests/Application/Stub/EmptyRun/.placeholder.php b/tests/Application/Stub/EmptyRun/.placeholder.php new file mode 100644 index 00000000..72680edf --- /dev/null +++ b/tests/Application/Stub/EmptyRun/.placeholder.php @@ -0,0 +1,10 @@ + Date: Mon, 6 Jul 2026 18:56:45 +0100 Subject: [PATCH 5/9] test(codecov): attribute Expect* constructors to ExpectInterceptorTest coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ExpectAssertionsCount, ExpectTestStatus, and ExpectTestResultAttribute each had 0% coverage per Codecov, despite ExpectInterceptorTest exercising every constructor via newInstance(). Testo's codecov plugin scopes coverage per test to the classes named in #[Covers(...)] on that test, so lines executed by a test are only credited to files the test explicitly declares — and this test only declared #[Covers(ExpectInterceptor::class)]. Add #[Covers(...)] for the three attribute classes so their already-exercised constructors are credited. --- tests/Core/Testing/Unit/ExpectInterceptorTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/Core/Testing/Unit/ExpectInterceptorTest.php b/tests/Core/Testing/Unit/ExpectInterceptorTest.php index 262cde2e..5004a2fd 100644 --- a/tests/Core/Testing/Unit/ExpectInterceptorTest.php +++ b/tests/Core/Testing/Unit/ExpectInterceptorTest.php @@ -21,6 +21,9 @@ #[Test] #[Covers(ExpectInterceptor::class)] +#[Covers(ExpectTestStatus::class)] +#[Covers(ExpectAssertionsCount::class)] +#[Covers(ExpectTestResultAttribute::class)] final class ExpectInterceptorTest { public function passesThroughWhenNoExpectAttributesPresent(): void From 1dc4f3d5e19062688b0b7c755604f29fb1f3d8cc Mon Sep 17 00:00:00 2001 From: Ross Addison Date: Thu, 13 Aug 2026 17:20:21 +0100 Subject: [PATCH 6/9] test: fix createTestInfoFor() for the rebased CaseDefinition/CaseInfo constructors 1.x moved 25 commits since this PR was opened, including required-argument additions to CaseDefinition::$file and CaseInfo::$suiteIdentity. Updates the test helper to pass both, matching the pattern used elsewhere in the suite (Path::create(__FILE__), a real SuiteIdentity). Verified after rebase: - composer rector:ci: clean, 0 files - Full-project Psalm (--no-cache): clean, exit 0 - Full Testo suite: 1682 passed, 6 failed/7 error (same pre-existing Bench/Self baseline as the current rector/* PR series, unrelated) - ExpectInterceptorTest: 11/11 passed Co-Authored-By: Claude Sonnet 5 --- tests/Core/Testing/Unit/ExpectInterceptorTest.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/Core/Testing/Unit/ExpectInterceptorTest.php b/tests/Core/Testing/Unit/ExpectInterceptorTest.php index 5004a2fd..dbf7e085 100644 --- a/tests/Core/Testing/Unit/ExpectInterceptorTest.php +++ b/tests/Core/Testing/Unit/ExpectInterceptorTest.php @@ -4,9 +4,11 @@ namespace Tests\Core\Testing\Unit; +use Internal\Path; use Testo\Assert; use Testo\Codecov\Covers; use Testo\Core\Context\CaseInfo; +use Testo\Core\Context\Identity\SuiteIdentity; use Testo\Core\Context\TestInfo; use Testo\Core\Context\TestResult; use Testo\Core\Definition\CaseDefinition; @@ -199,8 +201,8 @@ private function fixtureExpectPassedWithThreeAssertions(): void {} private static function createTestInfoFor(string $method): TestInfo { $reflection = new \ReflectionMethod(self::class, $method); - $caseDefinition = new CaseDefinition(name: 'ExpectInterceptorTest', type: 'test'); - $caseInfo = new CaseInfo(definition: $caseDefinition); + $caseDefinition = new CaseDefinition(name: 'ExpectInterceptorTest', type: 'test', file: Path::create(__FILE__)); + $caseInfo = new CaseInfo(definition: $caseDefinition, suiteIdentity: new SuiteIdentity('Core/Testing/Unit')); $testDefinition = new TestDefinition(reflection: $reflection); return new TestInfo( From abd2600d7e5121bc9553dc7f85dc7e9c446d2970 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Tue, 25 Aug 2026 10:12:04 +0400 Subject: [PATCH 7/9] fix(bench): guard filtered RStDev against a zero mean MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Relative deviation over the filtered iterations divided by the filtered mean without the guard its unfiltered sibling already had, so an all-zero set of measurements — reachable on a coarse timer when a trivial body fits inside one tick — crashed the runner with DivisionByZeroError instead of reporting a benchmark. Refs #303 Assisted-By: Claude Opus 4.8 (1M context) --- plugin/bench/src/Internal/Calculator.php | 2 +- plugin/bench/tests/Unit/CalculatorTest.php | 38 ++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 plugin/bench/tests/Unit/CalculatorTest.php diff --git a/plugin/bench/src/Internal/Calculator.php b/plugin/bench/src/Internal/Calculator.php index 469ddae3..98880c99 100644 --- a/plugin/bench/src/Internal/Calculator.php +++ b/plugin/bench/src/Internal/Calculator.php @@ -52,7 +52,7 @@ public static function calculate(CaseSet $caseSet): CaseResult # Calc RMS, average, and relative standard deviation for the filtered iterations $frms = self::rms(...$filtered); $favg = self::avg(...$filtered); - $frstdev = $frms / $favg * 100; + $frstdev = $favg > 0 ? ($frms / $favg) * 100 : 0.0; return new CaseResult( mean: self::avg(...$averages), diff --git a/plugin/bench/tests/Unit/CalculatorTest.php b/plugin/bench/tests/Unit/CalculatorTest.php new file mode 100644 index 00000000..fbb0c341 --- /dev/null +++ b/plugin/bench/tests/Unit/CalculatorTest.php @@ -0,0 +1,38 @@ +frstdev, 0.0); + Assert::same($result->rstdev, 0.0); + } + + /** + * @param list $perCallUs Per-call time of each iteration, in microseconds. + */ + private static function caseSet(array $perCallUs): CaseSet + { + return new CaseSet('probe', \array_map( + static fn(float $us): Snap => new Snap(calls: 20, memory: 0, time: $us * 20), + $perCallUs, + )); + } +} From 7f3eaa08041907e2b5be7974e5d4db9ba1743393 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Tue, 25 Aug 2026 10:23:25 +0400 Subject: [PATCH 8/9] fix(bench): stop rejecting every off-median sample when MAD is zero MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The outlier filter multiplies the median absolute deviation into its threshold, so a zero MAD — which a coarse timer produces routinely, whenever over half the samples share a value — collapsed the limit to zero and kept only exact-median samples, rejecting the mild tail and pushing the rejection rate past the point where the reporter declares the result invalid. A zero MAD marks a degenerately narrow distribution, so skip filtering and keep every sample. Refs #303 Assisted-By: Claude Opus 4.8 (1M context) --- plugin/bench/src/Internal/Calculator.php | 14 +++++++++----- plugin/bench/tests/Unit/CalculatorTest.php | 10 ++++++++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/plugin/bench/src/Internal/Calculator.php b/plugin/bench/src/Internal/Calculator.php index 98880c99..3e5919df 100644 --- a/plugin/bench/src/Internal/Calculator.php +++ b/plugin/bench/src/Internal/Calculator.php @@ -38,11 +38,15 @@ public static function calculate(CaseSet $caseSet): CaseResult # Set limit for outliers $limit = 3 * $mad * 1.4826; - # Filter out iterations that are considered outliers - $filtered = \array_filter( - $averages, - static fn(float $avg): bool => \abs($avg - $median) <= $limit, - ); + # A zero MAD means a degenerately narrow distribution — over half the samples share a value, + # which a coarse timer produces routinely — not that every sample off the median is an outlier. + # Filtering on a zero limit would keep only exact-median samples and reject the rest, so skip it. + $filtered = $mad > 0.0 + ? \array_filter( + $averages, + static fn(float $avg): bool => \abs($avg - $median) <= $limit, + ) + : $averages; # Calc RMS of the original averages for relative standard deviation calculation $rms = self::rms(...$averages); diff --git a/plugin/bench/tests/Unit/CalculatorTest.php b/plugin/bench/tests/Unit/CalculatorTest.php index fbb0c341..404a7467 100644 --- a/plugin/bench/tests/Unit/CalculatorTest.php +++ b/plugin/bench/tests/Unit/CalculatorTest.php @@ -25,6 +25,16 @@ public function allZeroMeasurementsDoNotDivideByZero(): void Assert::same($result->rstdev, 0.0); } + public function aZeroMadKeepsEverySampleInsteadOfRejectingTheTail(): void + { + # Six of ten samples share a value, so the median absolute deviation collapses to zero. + $set = self::caseSet([10.0, 10.0, 10.0, 10.0, 10.0, 10.0, 10.1, 10.1, 10.2, 10.2]); + + $result = Calculator::calculate($set); + + Assert::same($result->rejected, 0); + } + /** * @param list $perCallUs Per-call time of each iteration, in microseconds. */ From db1863cb3f7e3308386d5fe813781d8d732e884d Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Tue, 25 Aug 2026 10:32:05 +0400 Subject: [PATCH 9/9] fix(bench): guard the mean percentage against a zero baseline mean docs(bench): name `current` as the percentage baseline, distinct from the fastest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mean-difference percentage guarded on the baseline filtered time yet divided by the baseline mean, so a zero mean would still divide by zero; guard on the value actually used as the divisor, matching the median and filtered-mean rows. Relative percentages are measured against `current` (the marked method), while first place goes to the fastest callable — two independent things. The docblocks conflated them, calling the fastest the baseline; align them with the behaviour so `current` reads as the baseline and the fastest only as the rank winner. Refs #303 Assisted-By: Claude Opus 4.8 (1M context) --- core/Output/Rendering/BenchMapper.php | 6 +++--- plugin/bench/Bench.php | 7 ++++--- plugin/bench/src/Dto/ValueRel.php | 4 ++-- plugin/bench/src/Internal/Explanator.php | 2 +- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/core/Output/Rendering/BenchMapper.php b/core/Output/Rendering/BenchMapper.php index 7d026ae0..4a19f6fe 100644 --- a/core/Output/Rendering/BenchMapper.php +++ b/core/Output/Rendering/BenchMapper.php @@ -19,8 +19,8 @@ * loads any of this. * * The numbers are taken from {@see Line}, the ranked view the plugin computes: it carries the relative - * difference against the baseline case and the diagnostics that explain when a measurement should not be - * trusted. Times stay in microseconds, the unit the plugin measures in — converting here would only add a + * difference against the `current` baseline and the diagnostics that explain when a measurement should not + * be trusted. Times stay in microseconds, the unit the plugin measures in — converting here would only add a * rounding step for the renderer to undo. * * Two shapes are offered, because the reporters need different ones: {@see map()} for documents that can @@ -142,7 +142,7 @@ private static function case(Line $line, int $calls, int $memory): array # Microseconds, as the bench plugin measures. 'mean' => $line->avg->value, 'median' => $line->med->value, - # Percent against the baseline case: 0.0 for the baseline, positive for slower. + # Percent against `current`, the baseline: 0.0 for it, positive for slower, negative for faster. 'meanDiff' => $line->avg->diff, 'medianDiff' => $line->med->diff, # Standard deviation as a percentage of the mean. diff --git a/plugin/bench/Bench.php b/plugin/bench/Bench.php index d4d81c62..36ad647a 100644 --- a/plugin/bench/Bench.php +++ b/plugin/bench/Bench.php @@ -14,8 +14,9 @@ * * The marked method (or function) automatically receives the alias `current`. * All callables listed in {@see Bench::$callables} are benchmarked under the same conditions. - * Results are ranked by filtered average time — the fastest callable takes first place - * and serves as the baseline for relative comparison. + * Results are ranked by filtered average time — the fastest callable takes first place — while the + * relative percentages are measured against `current`, the baseline the alternatives are compared to. + * The fastest callable and the baseline are independent: `current` need not be the fastest. * * ## How it works * @@ -26,7 +27,7 @@ * * ## Aliases * - * - The marked method always has the alias `current`. + * - The marked method always has the alias `current` and is the baseline for relative percentages. * - For other callables, use **string keys** in the `$callables` array to assign aliases. * These aliases appear in the results table for easy identification. * diff --git a/plugin/bench/src/Dto/ValueRel.php b/plugin/bench/src/Dto/ValueRel.php index ba1b4ded..a82d21cc 100644 --- a/plugin/bench/src/Dto/ValueRel.php +++ b/plugin/bench/src/Dto/ValueRel.php @@ -13,8 +13,8 @@ public function __construct( public float $value, /** - * @var float Difference from the baseline value (the best-performing benchmark) in percentage. - * For the baseline, this will be 0.0. For others, can be positive (worse) or negative (better). + * @var float Difference from the baseline — the `current` case — in percentage. Zero for the + * baseline itself; positive means slower than it, negative means faster. * * The formula for calculating this is: ((current_value - baseline_value) / baseline_value) * 100 */ diff --git a/plugin/bench/src/Internal/Explanator.php b/plugin/bench/src/Internal/Explanator.php index af8abb2a..b5844ba4 100644 --- a/plugin/bench/src/Internal/Explanator.php +++ b/plugin/bench/src/Internal/Explanator.php @@ -40,7 +40,7 @@ public static function prepareLines(array $cases, array $results): array name: $cases[$k]->name, avg: new ValueRel( value: $result->mean, - diff: $baseTime > 0 ? ($result->mean - $baseMean) / $baseMean * 100 : 0.0, + diff: $baseMean > 0 ? ($result->mean - $baseMean) / $baseMean * 100 : 0.0, ), med: new ValueRel( value: $result->med,