From bc62ea8e79acab7618e034810c4a9e2ca56c423a Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Fri, 3 Jul 2026 12:59:48 +0700 Subject: [PATCH] Fix Pyrameter fail-on-violation hiding PHPUnit failure details --- src/Event/FailOnTargetViolationSubscriber.php | 65 ++++++++++ src/Event/PrintReportSubscriber.php | 25 ---- src/Extension.php | 6 + .../FailOnTargetViolationSubscriberTest.php | 114 ++++++++++++++++++ tests/Event/PrintReportSubscriberTest.php | 12 +- tests/ExtensionSmokeTest.php | 15 +++ .../SmokeProject/FailingPdoSmokeFixture.php | 16 +++ .../SmokeProject/phpunit-failing-test.xml | 15 +++ 8 files changed, 233 insertions(+), 35 deletions(-) create mode 100644 src/Event/FailOnTargetViolationSubscriber.php create mode 100644 tests/Event/FailOnTargetViolationSubscriberTest.php create mode 100644 tests/Fixtures/SmokeProject/FailingPdoSmokeFixture.php create mode 100644 tests/Fixtures/SmokeProject/phpunit-failing-test.xml diff --git a/src/Event/FailOnTargetViolationSubscriber.php b/src/Event/FailOnTargetViolationSubscriber.php new file mode 100644 index 0000000..42f22ef --- /dev/null +++ b/src/Event/FailOnTargetViolationSubscriber.php @@ -0,0 +1,65 @@ + $targets + * @param null|Closure(int): void $exit + */ + public function __construct( + private TestCollector $testCollector, + array $targets, + private bool $failOnViolation = false, + private ?Closure $exit = null, + ) { + $this->targetEvaluator = new TargetEvaluator($targets); + } + + public function notify(Finished $event): void + { + if (! $this->failOnViolation) { + return; + } + + $targetEvaluation = $this->targetEvaluator->evaluate($this->testCollector->summary()); + + if ($targetEvaluation->allPassed()) { + return; + } + + echo PHP_EOL . 'Pyrameter target shape violated.' . PHP_EOL; + + if ($event->shellExitCode() !== 0) { + return; + } + + $this->exit(1); + } + + private function exit(int $status): void + { + if ($this->exit instanceof Closure) { + ($this->exit)($status); + + return; + } + + // @codeCoverageIgnoreStart + exit($status); + // @codeCoverageIgnoreEnd + } +} diff --git a/src/Event/PrintReportSubscriber.php b/src/Event/PrintReportSubscriber.php index 7c4e16c..0beb253 100644 --- a/src/Event/PrintReportSubscriber.php +++ b/src/Event/PrintReportSubscriber.php @@ -8,12 +8,9 @@ use Boundwize\Pyrameter\Report\PyramidReporter; use Boundwize\Pyrameter\Report\SuiteShapeResolver; use Boundwize\Pyrameter\Target\TargetEvaluator; -use Closure; use PHPUnit\Event\TestRunner\ExecutionFinished; use PHPUnit\Event\TestRunner\ExecutionFinishedSubscriber; -use const PHP_EOL; - final readonly class PrintReportSubscriber implements ExecutionFinishedSubscriber { private TargetEvaluator $targetEvaluator; @@ -22,14 +19,11 @@ /** * @param array $targets - * @param null|Closure(int): void $exit */ public function __construct( private TestCollector $testCollector, array $targets, private PyramidReporter $pyramidReporter, - private bool $failOnViolation = false, - private ?Closure $exit = null, ) { $this->targetEvaluator = new TargetEvaluator($targets); $this->suiteShapeResolver = new SuiteShapeResolver(); @@ -42,24 +36,5 @@ public function notify(ExecutionFinished $event): void $suiteShape = $this->suiteShapeResolver->resolve($pyramidSummary, $targetEvaluation); $this->pyramidReporter->print($pyramidSummary, $targetEvaluation, $suiteShape); - - if ($this->failOnViolation && ! $targetEvaluation->allPassed()) { - echo PHP_EOL . 'Pyrameter target shape violated.' . PHP_EOL; - - $this->exit(1); - } - } - - private function exit(int $status): void - { - if ($this->exit instanceof Closure) { - ($this->exit)($status); - - return; - } - - // @codeCoverageIgnoreStart - exit($status); - // @codeCoverageIgnoreEnd } } diff --git a/src/Extension.php b/src/Extension.php index b85fd36..1f2ebb5 100644 --- a/src/Extension.php +++ b/src/Extension.php @@ -9,6 +9,7 @@ use Boundwize\Pyrameter\Config\PyrameterConfigLoader; use Boundwize\Pyrameter\Detection\TestUsageScanner; use Boundwize\Pyrameter\Event\CollectTestResultSubscriber; +use Boundwize\Pyrameter\Event\FailOnTargetViolationSubscriber; use Boundwize\Pyrameter\Event\PrintReportSubscriber; use Boundwize\Pyrameter\Report\PyramidReporter; use PHPUnit\Runner\Extension\Extension as PHPUnitExtension; @@ -40,6 +41,11 @@ public function bootstrap( testCollector: $testCollector, targets: $pyrameterConfig->targetPercentages(), pyramidReporter: new PyramidReporter(), + )); + + $facade->registerSubscriber(new FailOnTargetViolationSubscriber( + testCollector: $testCollector, + targets: $pyrameterConfig->targetPercentages(), failOnViolation: $pyrameterConfig->shouldFailOnViolation(), )); } diff --git a/tests/Event/FailOnTargetViolationSubscriberTest.php b/tests/Event/FailOnTargetViolationSubscriberTest.php new file mode 100644 index 0000000..1f3bf84 --- /dev/null +++ b/tests/Event/FailOnTargetViolationSubscriberTest.php @@ -0,0 +1,114 @@ +add(new TestRecord(self::class, 'testUnit', [], TestKind::Unit)); + $testCollector->add(new TestRecord(self::class, 'testIntegration', [], TestKind::Integration)); + + $exitStatus = null; + + $failOnTargetViolationSubscriber = new FailOnTargetViolationSubscriber( + testCollector: $testCollector, + targets: PyrameterConfig::defaults()->targetPercentages(), + failOnViolation: false, + exit: static function (int $status) use (&$exitStatus): void { + $exitStatus = $status; + }, + ); + + $failOnTargetViolationSubscriber->notify(new Finished($this->telemetryInfo(), 0)); + + $this->assertNull($exitStatus); + } + + public function testItDoesNothingWhenTargetsPass(): void + { + $testCollector = new TestCollector(); + $testCollector->add(new TestRecord(self::class, 'testUnit', [], TestKind::Unit)); + + $exitStatus = null; + + $failOnTargetViolationSubscriber = new FailOnTargetViolationSubscriber( + testCollector: $testCollector, + targets: PyrameterConfig::defaults()->targetPercentages(), + failOnViolation: true, + exit: static function (int $status) use (&$exitStatus): void { + $exitStatus = $status; + }, + ); + + $failOnTargetViolationSubscriber->notify(new Finished($this->telemetryInfo(), 0)); + + $this->assertNull($exitStatus); + } + + public function testItTerminatesWithFailureWhenTargetsAreViolatedAndPhpunitWouldPass(): void + { + $testCollector = new TestCollector(); + $testCollector->add(new TestRecord(self::class, 'testUnit', [], TestKind::Unit)); + $testCollector->add(new TestRecord(self::class, 'testIntegration', [], TestKind::Integration)); + + $exitStatus = null; + + $failOnTargetViolationSubscriber = new FailOnTargetViolationSubscriber( + testCollector: $testCollector, + targets: PyrameterConfig::defaults()->targetPercentages(), + failOnViolation: true, + exit: static function (int $status) use (&$exitStatus): void { + $exitStatus = $status; + }, + ); + + $this->expectOutputRegex('/Pyrameter target shape violated\./'); + + $failOnTargetViolationSubscriber->notify(new Finished($this->telemetryInfo(), 0)); + + $this->assertSame(1, $exitStatus); + } + + public function testItDoesNotOverrideExistingPhpunitFailureExitCode(): void + { + $testCollector = new TestCollector(); + $testCollector->add(new TestRecord(self::class, 'testUnit', [], TestKind::Unit)); + $testCollector->add(new TestRecord(self::class, 'testIntegration', [], TestKind::Integration)); + + $exitStatus = null; + + $failOnTargetViolationSubscriber = new FailOnTargetViolationSubscriber( + testCollector: $testCollector, + targets: PyrameterConfig::defaults()->targetPercentages(), + failOnViolation: true, + exit: static function (int $status) use (&$exitStatus): void { + $exitStatus = $status; + }, + ); + + $this->expectOutputRegex('/Pyrameter target shape violated\./'); + + $failOnTargetViolationSubscriber->notify(new Finished($this->telemetryInfo(), 1)); + + $this->assertNull($exitStatus); + } + + private function telemetryInfo(): Info + { + return TelemetryInfoFactory::create(); + } +} diff --git a/tests/Event/PrintReportSubscriberTest.php b/tests/Event/PrintReportSubscriberTest.php index 79cf1d2..68b5b99 100644 --- a/tests/Event/PrintReportSubscriberTest.php +++ b/tests/Event/PrintReportSubscriberTest.php @@ -33,29 +33,21 @@ public function testItPrintsTheReportWhenExecutionFinishes(): void $printReportSubscriber->notify(new ExecutionFinished($this->telemetryInfo())); } - public function testItTerminatesWithFailureWhenTargetsAreViolatedAndFailOnViolationIsEnabled(): void + public function testItPrintsViolationInReportWhenTargetsAreViolated(): void { $testCollector = new TestCollector(); $testCollector->add(new TestRecord(self::class, 'testUnit', [], TestKind::Unit)); $testCollector->add(new TestRecord(self::class, 'testIntegration', [], TestKind::Integration)); - $exitStatus = null; - $printReportSubscriber = new PrintReportSubscriber( testCollector: $testCollector, targets: PyrameterConfig::defaults()->targetPercentages(), pyramidReporter: new PyramidReporter(), - failOnViolation: true, - exit: static function (int $status) use (&$exitStatus): void { - $exitStatus = $status; - }, ); - $this->expectOutputRegex('/Pyrameter.*Pyrameter target shape violated\./s'); + $this->expectOutputRegex('/Pyrameter.*Result: Violated/s'); $printReportSubscriber->notify(new ExecutionFinished($this->telemetryInfo())); - - $this->assertSame(1, $exitStatus); } private function telemetryInfo(): Info diff --git a/tests/ExtensionSmokeTest.php b/tests/ExtensionSmokeTest.php index 7c4c01d..1d664d1 100644 --- a/tests/ExtensionSmokeTest.php +++ b/tests/ExtensionSmokeTest.php @@ -40,6 +40,21 @@ public function testFailOnViolationChangesPhpunitExitCodeWithoutSubscriberWarnin $this->assertStringNotContainsString('Exception in third-party event subscriber', $output); } + public function testFailOnViolationDoesNotHidePhpunitFailureDetails(): void + { + $configuration = __DIR__ . '/Fixtures/SmokeProject/phpunit-failing-test.xml'; + [$exitCode, $output] = $this->runPhpUnit($configuration); + + $this->assertSame(1, $exitCode, $output); + $this->assertStringContainsString('Pyrameter', $output); + $this->assertStringContainsString('Result: Violated', $output); + $this->assertStringContainsString('Pyrameter target shape violated.', $output); + $this->assertStringContainsString('There was 1 failure', $output); + $this->assertStringContainsString('FailingPdoSmokeFixture::testFails', $output); + $this->assertStringContainsString('Failed asserting', $output); + $this->assertStringNotContainsString('Exception in third-party event subscriber', $output); + } + /** * @return array{int, string} */ diff --git a/tests/Fixtures/SmokeProject/FailingPdoSmokeFixture.php b/tests/Fixtures/SmokeProject/FailingPdoSmokeFixture.php new file mode 100644 index 0000000..72a1c5d --- /dev/null +++ b/tests/Fixtures/SmokeProject/FailingPdoSmokeFixture.php @@ -0,0 +1,16 @@ + + + + + + + + + + FailingPdoSmokeFixture.php + + +