diff --git a/.github/.release-please-config.json b/.github/.release-please-config.json index 94fc3c25..a5a269f5 100644 --- a/.github/.release-please-config.json +++ b/.github/.release-please-config.json @@ -41,6 +41,12 @@ "include-component-in-tag": true, "changelog-path": "CHANGELOG.md" }, + "plugin/skip": { + "package-name": "testo/skip", + "component": "skip", + "include-component-in-tag": true, + "changelog-path": "CHANGELOG.md" + }, "plugin/test": { "package-name": "testo/test", "component": "test", diff --git a/.github/workflows/split-publish.yml b/.github/workflows/split-publish.yml index 5d4ac0fe..16e7290f 100644 --- a/.github/workflows/split-publish.yml +++ b/.github/workflows/split-publish.yml @@ -35,6 +35,7 @@ on: # yamllint disable-line rule:truthy - 'lifecycle-[0-9]*' - 'repeat-[0-9]*' - 'retry-[0-9]*' + - 'skip-[0-9]*' - 'test-[0-9]*' name: 📦 Split publish diff --git a/composer.json b/composer.json index 58347abc..343192fd 100644 --- a/composer.json +++ b/composer.json @@ -50,6 +50,7 @@ "testo/lifecycle": "^0.1.6", "testo/repeat": "^0.1.9", "testo/retry": "^0.1.5", + "testo/skip": "^0.1", "testo/test": "^0.1.8", "yiisoft/injector": "^1.2" }, @@ -105,6 +106,7 @@ "Tests\\Lifecycle\\": "plugin/lifecycle/tests/", "Tests\\Repeat\\": "plugin/repeat/tests/", "Tests\\Retry\\": "plugin/retry/tests/", + "Tests\\Skip\\": "plugin/skip/tests/", "Tests\\Test\\": "plugin/test/tests/" }, "files": [ @@ -131,6 +133,7 @@ "testo/lifecycle": "0.1.x-dev", "testo/repeat": "0.1.x-dev", "testo/retry": "0.1.x-dev", + "testo/skip": "0.1.x-dev", "testo/test": "0.1.x-dev" } } diff --git a/core/Application/Config/Plugin/SuitePlugins.php b/core/Application/Config/Plugin/SuitePlugins.php index 883499b2..c822c118 100644 --- a/core/Application/Config/Plugin/SuitePlugins.php +++ b/core/Application/Config/Plugin/SuitePlugins.php @@ -10,6 +10,7 @@ use Testo\Facade\FacadePlugin; use Testo\Inline\InlineTestPlugin; use Testo\Lifecycle\LifecyclePlugin; +use Testo\Skip\SkipPlugin; use Testo\Test\TestPlugin; $_ = []; @@ -18,6 +19,7 @@ \class_exists(FacadePlugin::class) and $_[] = new FacadePlugin(); \class_exists(InlineTestPlugin::class) and $_[] = new InlineTestPlugin(); \class_exists(LifecyclePlugin::class) and $_[] = new LifecyclePlugin(); +\class_exists(SkipPlugin::class) and $_[] = new SkipPlugin(); \class_exists(TestPlugin::class) and $_[] = new TestPlugin(); \define([__NAMESPACE__ . '\DEFAULT_SUITE_PLUGINS'][0], $_); diff --git a/core/Application/Internal/Runner/TestRunner.php b/core/Application/Internal/Runner/TestRunner.php index 2c8ca2a0..94a3202c 100644 --- a/core/Application/Internal/Runner/TestRunner.php +++ b/core/Application/Internal/Runner/TestRunner.php @@ -35,6 +35,10 @@ * {@see Status::Error} result, and a failure of the interceptor pipeline itself is captured as * {@see Status::Aborted}. * + * A test marked {@see \Testo\Core\Definition\TestDefinition::$skipped} still goes down the + * pipeline, so the interceptor that knows the reason may report it; if none does, the runner + * reports it as {@see Status::Skipped} itself instead of running the body. + * * @internal * @psalm-internal Testo\Application */ @@ -61,6 +65,20 @@ public function runTest(TestInfo $info): TestResult ...$interceptors, )->with( function (TestInfo $info) use ($description): TestResult { + # Nothing on the way down reported this skipped test, so no reason is known. + # `TestStarting` announces a body, and there is none: return ahead of it. + if ($info->testDefinition->skipped) { + return new TestResult( + info: $info, + status: Status::Skipped, + failure: new SkipTest("{$info->identity->fqn()} is skipped"), + attributes: [ + 'duration' => 0, + 'description' => $description, + ], + ); + } + $this->eventDispatcher->dispatch(new TestStarting($info)); $startTime = \microtime(true); diff --git a/core/Core/Definition/TestDefinition.php b/core/Core/Definition/TestDefinition.php index 38bfaf80..5153c97b 100644 --- a/core/Core/Definition/TestDefinition.php +++ b/core/Core/Definition/TestDefinition.php @@ -8,7 +8,8 @@ /** * A runnable member of a case: a test, or a non-test such as a lifecycle hook. Interceptors refine - * its role through the mutable {@see self::$isTest} and {@see self::$active} flags. + * its role through the mutable {@see self::$isTest}, {@see self::$active} and {@see self::$skipped} + * flags. * * @api */ @@ -27,6 +28,14 @@ public function __construct( * Whether this member is active. Filtering deactivates a test instead of discarding it. */ public bool $active = true, + + /** + * Whether this test is skipped ahead of time. A skipped test stays active: it is reported as + * {@see \Testo\Core\Value\Status::Skipped} without its body being run, unlike a deactivated + * test, which leaves the results entirely. Nothing that prepares a test body has to engage + * for it — a reader of this flag decides that for itself. + */ + public bool $skipped = false, ) {} public function getDescription(): ?string diff --git a/core/Core/Definition/TestDefinitions.php b/core/Core/Definition/TestDefinitions.php index 314b09bc..c982e2c6 100644 --- a/core/Core/Definition/TestDefinitions.php +++ b/core/Core/Definition/TestDefinitions.php @@ -63,25 +63,26 @@ public function all(): array * * @return array */ - public function filter(?bool $isTest = null, ?bool $active = null): array + public function filter(?bool $isTest = null, ?bool $active = null, ?bool $skipped = null): array { return \array_filter( $this->definitions, static fn(TestDefinition $d): bool => ($isTest === null || $d->isTest === $isTest) - && ($active === null || $d->active === $active), + && ($active === null || $d->active === $active) + && ($skipped === null || $d->skipped === $skipped), ); } /** - * The case's tests — by default only the active ones, i.e. the definitions to run. Pass - * `$active = false` for the deactivated tests alone, or `null` for every test regardless of - * state. + * The case's tests — by default only the active ones, i.e. the definitions to run or to report + * as skipped. Pass `$active = false` for the deactivated tests alone, or `null` for every test + * regardless of state; `$skipped = false` narrows down to the tests whose body will run. * * @return array */ - public function getTests(?bool $active = true): array + public function getTests(?bool $active = true, ?bool $skipped = null): array { - return $this->filter(isTest: true, active: $active); + return $this->filter(isTest: true, active: $active, skipped: $skipped); } /** diff --git a/core/Pipeline/Internal/AttributesInterceptor.php b/core/Pipeline/Internal/AttributesInterceptor.php index f3720d95..6b44bc23 100644 --- a/core/Pipeline/Internal/AttributesInterceptor.php +++ b/core/Pipeline/Internal/AttributesInterceptor.php @@ -64,7 +64,6 @@ static function (\ReflectionAttribute $a): Interceptable { $attrs, )); - # Merge and instantiate attributes $interceptors = $this->interceptorProvider->fromAttributes(TestRunInterceptor::class, ...$attrs); $info = $info->withAttributes(self::groupAttributes($attrs)); diff --git a/plugin/lifecycle/src/Internal/LifecycleInterceptor.php b/plugin/lifecycle/src/Internal/LifecycleInterceptor.php index af7f61ec..99fcac0b 100644 --- a/plugin/lifecycle/src/Internal/LifecycleInterceptor.php +++ b/plugin/lifecycle/src/Internal/LifecycleInterceptor.php @@ -73,10 +73,17 @@ public function locateTestCases(FileDefinitions $file, callable $next): CaseDefi /** * Collect all the lifecycle hooks and cache them for execution during test runs. + * + * A case without a test to run (every active test is skipped) has nothing to set up: its hooks + * stay silent, class-level and per-test alike. */ #[\Override] public function runTestCase(CaseInfo $info, callable $next): CaseResult { + if ($info->definition->tests->getTests(skipped: false) === []) { + return $next($info); + } + $result = self::group(self::collectHooks($info->definition)); # Execute BeforeClass hooks @@ -99,7 +106,7 @@ public function runTest(TestInfo $info, callable $next): TestResult { /** @var array, non-empty-list<\ReflectionFunctionAbstract>> $hooks */ $hooks = $info->caseInfo->getAttribute(self::class, []); - if ($hooks === []) { + if ($hooks === [] || $info->testDefinition->skipped) { return $next($info); } @@ -155,8 +162,8 @@ private static function group(iterable $hooks): array /** * The lifecycle-annotated non-test members of the case. Lifecycle-annotated members a finder * took for tests were demoted in {@see self::locateTestCases()}, so every hook is a non-test. - * Non-tests outlive test filtering: the `#[BeforeClass]`/`#[AfterClass]` hooks run even for a - * case whose tests were all filtered out. + * Discovery therefore does not depend on which tests survived: deactivating or skipping one + * leaves the case's hooks where they are. * * @return list<\ReflectionFunctionAbstract> */ diff --git a/plugin/lifecycle/tests/Feature/FullySkippedCaseFeatureTest.php b/plugin/lifecycle/tests/Feature/FullySkippedCaseFeatureTest.php new file mode 100644 index 00000000..b1538787 --- /dev/null +++ b/plugin/lifecycle/tests/Feature/FullySkippedCaseFeatureTest.php @@ -0,0 +1,73 @@ +status, Status::Skipped); + Assert::same(FullySkippedFunctionState::$beforeClassCalls - $beforeClass, 0); + Assert::same(FullySkippedFunctionState::$afterClassCalls - $afterClass, 0); + Assert::same(FullySkippedFunctionState::$beforeTestCalls - $beforeTest, 0); + Assert::same(FullySkippedFunctionState::$afterTestCalls - $afterTest, 0); + } + + /** + * The class-based analog: the class-level hooks stay silent for a fully skipped class. + */ + public function noHookRunsForAFullySkippedClassCase(): void + { + $beforeClass = FullySkippedClassStub::$beforeClassCalls; + $afterClass = FullySkippedClassStub::$afterClassCalls; + + $result = TestRunner::runTest([FullySkippedClassStub::class, 'skipped']); + + Assert::same($result->status, Status::Skipped); + Assert::same(FullySkippedClassStub::$beforeClassCalls - $beforeClass, 0); + Assert::same(FullySkippedClassStub::$afterClassCalls - $afterClass, 0); + } +} diff --git a/plugin/lifecycle/tests/Stub/FullySkipped/FullySkippedClassStub.php b/plugin/lifecycle/tests/Stub/FullySkipped/FullySkippedClassStub.php new file mode 100644 index 00000000..7ef005d5 --- /dev/null +++ b/plugin/lifecycle/tests/Stub/FullySkipped/FullySkippedClassStub.php @@ -0,0 +1,42 @@ +fixturesDir . 'PrunedFunctionsWithLifecycle.php'; @@ -191,28 +193,82 @@ public function runsClassHooksForFunctionCaseWhoseTestsWereAllPruned(): void PrunedFunctionsState::$afterClassCalls = 0; $info = $this->makeFunctionCaseInfoWithoutTests($this->fixturesDir . 'PrunedFunctionsWithLifecycle.php'); - $hooksAtNext = null; - $beforeClassAtNext = null; - $afterClassAtNext = null; + $caseAtNext = null; $this->interceptor->runTestCase( $info, - static function (CaseInfo $case) use (&$hooksAtNext, &$beforeClassAtNext, &$afterClassAtNext): CaseResult { - $hooksAtNext = $case->getAttribute(LifecycleInterceptor::class, []); - $beforeClassAtNext = PrunedFunctionsState::$beforeClassCalls; - $afterClassAtNext = PrunedFunctionsState::$afterClassCalls; + static function (CaseInfo $case) use (&$caseAtNext): CaseResult { + $caseAtNext = $case; return new CaseResult(results: [], status: Status::Passed); }, ); - # All four hooks were discovered from the case's non-tests and published for the inner pipeline. - Assert::array($hooksAtNext) - ->hasKeys(BeforeClass::class, AfterClass::class, BeforeTest::class, AfterTest::class); - # The class hooks fire exactly once, around the inner pipeline: BeforeClass has already - # fired when `$next` runs, AfterClass has not yet. - Assert::same($beforeClassAtNext, 1); - Assert::same($afterClassAtNext, 0); - Assert::same(PrunedFunctionsState::$beforeClassCalls, 1); - Assert::same(PrunedFunctionsState::$afterClassCalls, 1); + Assert::same($caseAtNext, $info); + Assert::same(PrunedFunctionsState::$beforeClassCalls, 0); + Assert::same(PrunedFunctionsState::$afterClassCalls, 0); + } + + /** + * The skipped flag is what sets a fully skipped case apart from a runnable one: its tests + * stay active, yet the class hooks must not fire for them. + */ + public function runsNoHookForCaseWhoseTestsAreAllSkipped(): void + { + require_once $this->fixturesDir . 'PrunedFunctionsWithLifecycle.php'; + PrunedFunctionsState::$beforeClassCalls = 0; + PrunedFunctionsState::$afterClassCalls = 0; + $info = $this->makeFunctionCaseInfoWithoutTests($this->fixturesDir . 'PrunedFunctionsWithLifecycle.php'); + $info->definition->tests->define(new \ReflectionFunction('strlen'))->skipped = true; + + $caseAtNext = null; + $this->interceptor->runTestCase( + $info, + static function (CaseInfo $case) use (&$caseAtNext): CaseResult { + $caseAtNext = $case; + return new CaseResult(results: [], status: Status::Passed); + }, + ); + + Assert::same($caseAtNext, $info); + Assert::same(PrunedFunctionsState::$beforeClassCalls, 0); + Assert::same(PrunedFunctionsState::$afterClassCalls, 0); + } + + /** + * Per-test hooks are for a test body: a skipped test gets none, even in a case whose other + * tests run and whose hooks are published. + */ + public function runsNoTestHookForASkippedTest(): void + { + require_once $this->fixturesDir . 'PrunedFunctionsWithLifecycle.php'; + PrunedFunctionsState::$beforeTestCalls = 0; + PrunedFunctionsState::$afterTestCalls = 0; + $info = $this->makeFunctionCaseInfoWithoutTests($this->fixturesDir . 'PrunedFunctionsWithLifecycle.php'); + $info->definition->tests->define(new \ReflectionFunction('strrev')); + $skipped = $info->definition->tests->define(new \ReflectionFunction('strlen')); + $skipped->skipped = true; + + $caseAtNext = null; + $this->interceptor->runTestCase( + $info, + static function (CaseInfo $case) use (&$caseAtNext): CaseResult { + $caseAtNext = $case; + return new CaseResult(results: [], status: Status::Passed); + }, + ); + \assert($caseAtNext instanceof CaseInfo); + Assert::array($caseAtNext->getAttribute(LifecycleInterceptor::class, [])) + ->hasKeys(BeforeTest::class, AfterTest::class); + + $testInfo = new TestInfo(name: 'strlen', caseInfo: $caseAtNext, testDefinition: $skipped); + $nextCalled = false; + $this->interceptor->runTest($testInfo, static function (TestInfo $test) use (&$nextCalled): TestResult { + $nextCalled = true; + return new TestResult(info: $test, status: Status::Passed); + }); + + Assert::true($nextCalled); + Assert::same(PrunedFunctionsState::$beforeTestCalls, 0); + Assert::same(PrunedFunctionsState::$afterTestCalls, 0); } /** diff --git a/plugin/skip/.github/workflows/close-prs.yml b/plugin/skip/.github/workflows/close-prs.yml new file mode 100644 index 00000000..7640d59f --- /dev/null +++ b/plugin/skip/.github/workflows/close-prs.yml @@ -0,0 +1,14 @@ +name: Close PRs + +on: + pull_request_target: + types: [opened, reopened] + +permissions: + pull-requests: write + +jobs: + close: + uses: php-testo/gh-actions/.github/workflows/close-foreign-prs.yml@v1 + with: + upstream-url: https://github.com/php-testo/testo diff --git a/plugin/skip/README.md b/plugin/skip/README.md new file mode 100644 index 00000000..71413b19 --- /dev/null +++ b/plugin/skip/README.md @@ -0,0 +1,41 @@ +

+ TESTO +

+ +

Skip attribute plugin

+ +
+ +[![Documentation](https://img.shields.io/badge/Documentation-blue?style=for-the-badge&logo=gitbook&logoColor=white)](https://php-testo.github.io) +[![Support on Boosty](https://img.shields.io/static/v1?style=for-the-badge&label=&message=Sponsorship&logo=Boosty&logoColor=white&color=%23F15F2C)](https://boosty.to/roxblnfk) + +
+ +
+ +> [!IMPORTANT] +> ## 🪞 This is a read-only mirror. +> +> Active development of the Testo project lives in [**php-testo/testo**](https://github.com/php-testo/testo) under `plugin/skip/`. This repository is **automatically synchronized** from there on every release. +> +> File issues and pull requests in the [main monorepo](https://github.com/php-testo/testo/issues), not here. + +## About + +Marks a test, a test class or a test function as skipped without deleting or hiding it. The test is not executed, but stays in every report as Skipped with its reason, so parked tests remain visible until someone returns to them. + +The skip is declared ahead of time, next to the test; skipping at runtime from the test body is covered by the core `SkipTest` exception instead. + +## Install + +```bash +composer require --dev testo/skip +``` + +[![PHP](https://img.shields.io/packagist/php-v/testo/skip.svg?style=flat-square&logo=php)](https://packagist.org/packages/testo/skip) +[![Latest Version on Packagist](https://img.shields.io/packagist/v/testo/skip.svg?style=flat-square&logo=packagist)](https://packagist.org/packages/testo/skip) +[![License](https://img.shields.io/packagist/l/testo/skip.svg?style=flat-square)](https://github.com/php-testo/testo/blob/1.x/LICENSE.md) +[![Total Downloads](https://img.shields.io/packagist/dt/testo/skip.svg?style=flat-square)](https://packagist.org/packages/testo/skip/stats) diff --git a/plugin/skip/Skip.php b/plugin/skip/Skip.php new file mode 100644 index 00000000..074886bf --- /dev/null +++ b/plugin/skip/Skip.php @@ -0,0 +1,46 @@ + {reason}` when a reason is given. Contrast with a filter, which drops the test from + * the run and the results entirely. A run consisting only of skipped tests exits 0. + * + * On a class every test of the case is skipped. The attribute is inherited from parent classes, + * traits and overridden methods; a method-level `#[Skip]` wins over the class-level one, reason + * included. It is inert on a non-test member and on a case of any type but {@see TestType::Test}. + * + * No registration is needed: the attribute wires {@see SkipInterceptor} itself, which reports the + * test at the entry of its own pipeline, so nothing that prepares, wraps or multiplies a test body + * engages for it. {@see SkipPlugin}, part of the default suite plugins, sets + * {@see TestDefinition::$skipped} ahead of the run. + * + * @api + */ +#[\Attribute(\Attribute::TARGET_CLASS | \Attribute::TARGET_METHOD | \Attribute::TARGET_FUNCTION)] +#[FallbackInterceptor(SkipInterceptor::class)] +final readonly class Skip implements Interceptable +{ + /** + * @param string $reason Why the test is skipped. A reference to an issue + * (`'flaky on CI, see ISSUE-123'`) keeps the skip reviewable. + */ + public function __construct( + public string $reason = '', + ) {} +} diff --git a/plugin/skip/composer.json b/plugin/skip/composer.json new file mode 100644 index 00000000..464a9a41 --- /dev/null +++ b/plugin/skip/composer.json @@ -0,0 +1,42 @@ +{ + "name": "testo/skip", + "description": "Skip attribute plugin for the Testo testing framework.", + "license": "BSD-3-Clause", + "type": "library", + "keywords": [ + "testo", + "skip", + "test" + ], + "authors": [ + { + "name": "Aleksei Gagarin (roxblnfk)", + "homepage": "https://github.com/roxblnfk" + } + ], + "funding": [ + { + "type": "boosty", + "url": "https://boosty.to/roxblnfk" + } + ], + "require": { + "php": ">=8.2", + "testo/testo": "0.10.47 - 1" + }, + "autoload": { + "psr-4": { + "Testo\\Skip\\": "src/" + }, + "files": [ + "Skip.php" + ] + }, + "minimum-stability": "dev", + "prefer-stable": true, + "extra": { + "branch-alias": { + "dev-1.x": "1.x-dev" + } + } +} diff --git a/plugin/skip/src/Internal/SkipInterceptor.php b/plugin/skip/src/Internal/SkipInterceptor.php new file mode 100644 index 00000000..99b5b14e --- /dev/null +++ b/plugin/skip/src/Internal/SkipInterceptor.php @@ -0,0 +1,89 @@ + 0, 'description' => $info->testDefinition->getDescription()], + summary: Summary::forTest(Status::Skipped), + ); + } + + /** + * `{testId} is skipped via #[Skip]`, extended with ` ==> {reason}` when a reason is given. + * The test id is the string `--filter` takes back. + */ + private static function reason(TestInfo $info): string + { + $message = "{$info->identity->fqn()} is skipped via #[Skip]"; + $reason = self::attribute($info)?->reason ?? ''; + + return $reason === '' ? $message : "{$message} ==> {$reason}"; + } + + /** + * The attribute whose reason applies to this test: `limit: 1` stops at the nearest declaration, + * so an overriding function's own `#[Skip]` is taken over the one it inherits. The class is + * consulted only when the function carries none of its own. + */ + private static function attribute(TestInfo $info): ?Skip + { + $attributes = Reflection::fetchFunctionAttributes( + $info->testDefinition->reflection, + attributeClass: Skip::class, + limit: 1, + ); + + $class = $info->caseInfo->definition->reflection; + $attributes === [] && $class !== null and $attributes = Reflection::fetchClassAttributes( + $class, + attributeClass: Skip::class, + limit: 1, + ); + + return $attributes === [] ? null : $attributes[0]->newInstance(); + } +} diff --git a/plugin/skip/src/Internal/SkipLocatorInterceptor.php b/plugin/skip/src/Internal/SkipLocatorInterceptor.php new file mode 100644 index 00000000..fb7e417e --- /dev/null +++ b/plugin/skip/src/Internal/SkipLocatorInterceptor.php @@ -0,0 +1,57 @@ +getCases() as $case) { + if ($case->type !== TestType::Test->value) { + continue; + } + + $classSkipped = $case->reflection !== null + && Reflection::fetchClassAttributes($case->reflection, attributeClass: Skip::class, limit: 1) !== []; + + foreach ($case->tests->getTests(active: null) as $test) { + $classSkipped + || Reflection::fetchFunctionAttributes($test->reflection, attributeClass: Skip::class, limit: 1) !== [] + and $test->skipped = true; + } + } + + return $result; + } +} diff --git a/plugin/skip/src/SkipPlugin.php b/plugin/skip/src/SkipPlugin.php new file mode 100644 index 00000000..84b82324 --- /dev/null +++ b/plugin/skip/src/SkipPlugin.php @@ -0,0 +1,29 @@ +get(InterceptorCollector::class)->addInterceptor(new SkipLocatorInterceptor()); + } +} diff --git a/plugin/skip/tests/Feature/SkipFallbackStandaloneTest.php b/plugin/skip/tests/Feature/SkipFallbackStandaloneTest.php new file mode 100644 index 00000000..e43b75f8 --- /dev/null +++ b/plugin/skip/tests/Feature/SkipFallbackStandaloneTest.php @@ -0,0 +1,95 @@ +with(new NamingConventionPlugin())); + + Assert::count($tests, 2); + Assert::true(StandaloneSkippedTest::$enabledRan); + Assert::same($tests['testEnabled']->status, Status::Passed); + self::assertSkippedWithReason($tests['testSkipped']); + } + + public function methodLevelSkipFallsBackWithoutSkipPlugin(): void + { + $tests = self::run( + SuitePlugins::without(TestPlugin::class, SkipPlugin::class)->with(new NamingConventionPlugin()), + ); + + Assert::count($tests, 2); + Assert::same($tests['testEnabled']->status, Status::Passed); + self::assertSkippedWithReason($tests['testSkipped']); + } + + private static function assertSkippedWithReason(TestResult $skipped): void + { + Assert::same($skipped->status, Status::Skipped); + Assert::instanceOf($skipped->failure, SkipTest::class); + Assert::same( + $skipped->failure->getMessage(), + StandaloneSkippedTest::class . '::testSkipped is skipped via #[Skip] ==> standalone method is skipped', + ); + } + + /** + * @return array + */ + private static function run(PluginCollection $plugins): array + { + $run = Application::createFromConfig(new ApplicationConfig( + src: [], + suites: [ + new SuiteConfig( + 'SkipStandalone', + location: new FinderConfig(include: [__DIR__ . '/../Stub/SkipStandalone']), + plugins: $plugins, + ), + ], + ))->run(); + + $tests = []; + foreach ($run as $suite) { + foreach ($suite as $case) { + foreach ($case as $test) { + $tests[$test->info->name] = $test; + } + } + } + + return $tests; + } +} diff --git a/plugin/skip/tests/Feature/SkipFeatureTest.php b/plugin/skip/tests/Feature/SkipFeatureTest.php new file mode 100644 index 00000000..cb190eed --- /dev/null +++ b/plugin/skip/tests/Feature/SkipFeatureTest.php @@ -0,0 +1,358 @@ +status, Status::Skipped); + Assert::instanceOf($result->failure, SkipTest::class); + Assert::same( + $result->failure?->getMessage(), + SkipMethodStub::class . '::skipped is skipped via #[Skip] ==> broken by the pricing rework, see ISSUE-123', + ); + } + + public function emptyReasonFallsBackToGeneratedMessage(): void + { + $result = TestRunner::runTest([SkipMethodStub::class, 'skippedNoReason']); + + Assert::same($result->status, Status::Skipped); + Assert::same( + $result->failure?->getMessage(), + SkipMethodStub::class . '::skippedNoReason is skipped via #[Skip]', + ); + } + + public function controlNeighborNextToSkippedTestsStillRuns(): void + { + $result = TestRunner::runTest([SkipMethodStub::class, 'enabled']); + + Assert::same($result->status, Status::Passed); + } + + public function classLevelSkipSkipsEveryTestWithClassReason(): void + { + $first = TestRunner::runTest([SkipClassLevelStub::class, 'firstSkipped']); + $second = TestRunner::runTest([SkipClassLevelStub::class, 'secondSkipped']); + + Assert::same($first->status, Status::Skipped); + Assert::same($second->status, Status::Skipped); + Assert::true(\str_ends_with((string) $first->failure?->getMessage(), ' ==> the whole case is skipped')); + Assert::true(\str_ends_with((string) $second->failure?->getMessage(), ' ==> the whole case is skipped')); + } + + public function methodReasonWinsOverClassReason(): void + { + $own = TestRunner::runTest([SkipClassAndMethodStub::class, 'ownReason']); + $inherited = TestRunner::runTest([SkipClassAndMethodStub::class, 'classReason']); + + Assert::true(\str_ends_with((string) $own->failure?->getMessage(), ' ==> method-specific reason')); + Assert::true(\str_ends_with((string) $inherited->failure?->getMessage(), ' ==> class-wide reason')); + } + + /** + * The method-level attribute wins as a whole: an empty method reason is not filled in + * from the class reason. + */ + public function emptyMethodReasonStillWinsOverClassReason(): void + { + $result = TestRunner::runTest([SkipClassAndMethodStub::class, 'emptyOwnReason']); + + Assert::same($result->status, Status::Skipped); + Assert::same( + $result->failure?->getMessage(), + SkipClassAndMethodStub::class . '::emptyOwnReason is skipped via #[Skip]', + ); + } + + public function functionalTestUsesFunctionFqnInMessage(): void + { + $result = TestRunner::runTest('Tests\Skip\Stub\Skip\skippedFunction'); + + Assert::same($result->status, Status::Skipped); + Assert::same( + $result->failure?->getMessage(), + 'Tests\Skip\Stub\Skip\skippedFunction is skipped via #[Skip] ==> functional test is skipped', + ); + } + + /** + * A function carries no class to fall back on, so the empty-reason fallback has to hold on its + * own: the message is the generated part alone. + */ + public function functionWithoutReasonFallsBackToGeneratedMessage(): void + { + $result = TestRunner::runTest('Tests\Skip\Stub\Skip\skippedFunctionNoReason'); + + Assert::same($result->status, Status::Skipped); + Assert::same( + $result->failure?->getMessage(), + 'Tests\Skip\Stub\Skip\skippedFunctionNoReason is skipped via #[Skip]', + ); + } + + /** + * The function-based analog of the control neighbor: an enabled function of a partially + * skipped file still runs and passes. + */ + public function controlNeighborFunctionNextToSkippedFunctionStillRuns(): void + { + $result = TestRunner::runTest('Tests\Skip\Stub\Skip\enabledFunction'); + + Assert::same($result->status, Status::Passed); + } + + /** + * The origin contract for downstream consumers: a result skipped by `#[Skip]` carries the + * attribute instances in `$result->info`, unlike a runtime `throw SkipTest` skip. + */ + public function skippedResultCarriesOriginAttribute(): void + { + $result = TestRunner::runTest([SkipMethodStub::class, 'skipped']); + + $origin = $result->info->getAttribute(Skip::class); + Assert::array($origin)->hasCount(1); + Assert::instanceOf($origin[0], Skip::class); + } + + /** + * A partially skipped case: class-level hooks fire as usual (once per directory run) because + * the case still has a test to run, per-test hooks fire only for the enabled control test. + */ + public function classHooksRunButTestHooksDoNot(): void + { + $beforeClass = SkipWithHooksStub::$beforeClassCalls; + $afterClass = SkipWithHooksStub::$afterClassCalls; + $beforeTest = SkipWithHooksStub::$beforeTestCalls; + $afterTest = SkipWithHooksStub::$afterTestCalls; + + $result = TestRunner::runTest([SkipWithHooksStub::class, 'skipped']); + + Assert::same($result->status, Status::Skipped); + Assert::same(SkipWithHooksStub::$beforeClassCalls - $beforeClass, 1); + Assert::same(SkipWithHooksStub::$afterClassCalls - $afterClass, 1); + # Only the enabled control test of the case went through the per-test pipeline. + Assert::same(SkipWithHooksStub::$beforeTestCalls - $beforeTest, 1); + Assert::same(SkipWithHooksStub::$afterTestCalls - $afterTest, 1); + } + + public function fullySkippedCaseWithoutHooksIsNeverInstantiated(): void + { + $result = TestRunner::runTest([SkipConstructorSpyStub::class, 'firstSkipped']); + + Assert::same($result->status, Status::Skipped); + Assert::false(SkipConstructorSpyStub::$constructed); + } + + /** + * A fully skipped case gets no class-level hooks at all, so not even a non-static + * `#[BeforeClass]` hook builds the class. + */ + public function fullySkippedCaseRunsNoClassHooksAndIsNotBuiltForThem(): void + { + $constructions = SkipNonStaticHookStub::$constructions; + $hookCalls = SkipNonStaticHookStub::$hookCalls; + + $result = TestRunner::runTest([SkipNonStaticHookStub::class, 'skipped']); + + Assert::same($result->status, Status::Skipped); + Assert::same(SkipNonStaticHookStub::$hookCalls - $hookCalls, 0); + Assert::same(SkipNonStaticHookStub::$constructions - $constructions, 0); + } + + public function classLevelSkipIsInheritedFromParent(): void + { + $result = TestRunner::runTest([SkipChildStub::class, 'skipped']); + + Assert::same($result->status, Status::Skipped); + Assert::true(\str_ends_with((string) $result->failure?->getMessage(), ' ==> inherited from the parent class')); + } + + public function classLevelSkipIsInheritedFromTrait(): void + { + $result = TestRunner::runTest([SkipTraitStub::class, 'skipped']); + + Assert::same($result->status, Status::Skipped); + Assert::true(\str_ends_with((string) $result->failure?->getMessage(), ' ==> inherited from the trait')); + } + + /** + * A method-level `#[Skip]` follows the prototype chain: an overriding method without the + * attribute is still skipped, with the parent's reason. + */ + public function methodLevelSkipIsInheritedByOverridingMethod(): void + { + $result = TestRunner::runTest([SkipOverridingMethodStub::class, 'skipped']); + + Assert::same($result->status, Status::Skipped); + Assert::true(\str_ends_with((string) $result->failure?->getMessage(), ' ==> inherited from the overridden method')); + } + + /** + * The nearest declaration wins: an override that repeats `#[Skip]` reports its own reason. + * Both declarations spawn an interceptor, and the surviving one carries no reason of its own, + * so this passes only while the reason is resolved by reflection. + */ + public function ownReasonOfAnOverridingMethodWinsOverTheInheritedOne(): void + { + $result = TestRunner::runTest([SkipOverridingMethodOwnReasonStub::class, 'skipped']); + + Assert::same($result->status, Status::Skipped); + Assert::same( + $result->failure?->getMessage(), + SkipOverridingMethodOwnReasonStub::class . '::skipped is skipped via #[Skip] ==> own reason of the overriding method', + ); + } + + /** + * A data-driven skipped test yields a single Skipped node: the provider is never called + * (not once across all directory runs of this class), no `MultipleResult` aggregate is + * attached. + */ + public function dataProviderIsNotCalledForSkippedTest(): void + { + $result = TestRunner::runTest([SkipWithDataProviderStub::class, 'skipped']); + + Assert::same($result->status, Status::Skipped); + Assert::null($result->getAttribute(MultipleResult::class)); + Assert::same(SkipWithDataProviderStub::$providerCalls, 0); + } + + /** + * The positive control on the enabled neighbor proves that `#[Retry]` does engage in this + * run — its first attempt fails and the second passes — so the zero on the skipped test is + * the skip at work, not a retry plugin that never ran. + */ + public function retryDoesNotEngageForSkippedTest(): void + { + $attempts = SkipWithRetryStub::$attempts; + $enabledAttempts = SkipWithRetryStub::$enabledAttempts; + + $result = TestRunner::runTest([SkipWithRetryStub::class, 'skipped']); + + Assert::same($result->status, Status::Skipped); + Assert::same(SkipWithRetryStub::$attempts - $attempts, 0); + Assert::same(SkipWithRetryStub::$enabledAttempts - $enabledAttempts, 2); + } + + /** + * Same shape for `#[Repeat]`: the enabled neighbor runs all three of its repetitions, the + * skipped test not even once. + */ + public function repeatDoesNotEngageForSkippedTest(): void + { + $enabledRuns = SkipWithRepeatStub::$enabledRuns; + + $result = TestRunner::runTest([SkipWithRepeatStub::class, 'skipped']); + + Assert::same($result->status, Status::Skipped); + Assert::false(SkipWithRepeatStub::$bodyRan); + Assert::same(SkipWithRepeatStub::$enabledRuns - $enabledRuns, 3); + } + + /** + * The common ground of the hook/provider/retry/repeat checks above: a skipped test is cut off + * at the entry of its pipeline. A spy interceptor at the default position sees the enabled + * neighbors of the directory and none of the skipped tests. + */ + public function skippedTestsNeverGetPastTheSkipInterceptor(): void + { + $offset = \count(PipelineEntrySpyPlugin::$entered); + + TestRunner::runTest([SkipMethodStub::class, 'skipped']); + + $entered = \array_slice(PipelineEntrySpyPlugin::$entered, $offset); + Assert::array($entered) + ->contains(SkipMethodStub::class . '::enabled') + ->contains('Tests\Skip\Stub\Skip\enabledFunction') + ->notContains(SkipMethodStub::class . '::skipped') + ->notContains(SkipMethodStub::class . '::skippedNoReason') + ->notContains(SkipWithHooksStub::class . '::skipped') + ->notContains(SkipWithDataProviderStub::class . '::skipped') + ->notContains(SkipWithRetryStub::class . '::skipped') + ->notContains(SkipWithRepeatStub::class . '::skipped') + ->notContains(SkipInFiberStub::class . '::skipped') + ->notContains(SkipOverridingMethodStub::class . '::skipped') + ->notContains('Tests\Skip\Stub\Skip\skippedFunction') + ->notContains('Tests\Skip\Stub\Skip\skippedFunctionNoReason'); + } + + /** + * Fiber compatibility: a skipped test inside a fiber-driven case does not disturb the case + * scheduler. The round-robin interleaving of the two enabled tests is produced only by that + * scheduler — run sequentially, their `\Fiber::suspend()` would throw and the log would stop + * short — while the skipped test is still skipped. + */ + public function fiberScheduledCaseKeepsItsInterleaving(): void + { + $offset = \count(SkipInFiberStub::$log); + + $skipped = TestRunner::runTest([SkipInFiberStub::class, 'skipped']); + + Assert::same($skipped->status, Status::Skipped); + Assert::same( + \array_slice(SkipInFiberStub::$log, $offset), + ['first.1', 'second.1', 'first.2', 'second.2'], + ); + } +} diff --git a/plugin/skip/tests/Feature/SkipSummaryTest.php b/plugin/skip/tests/Feature/SkipSummaryTest.php new file mode 100644 index 00000000..9c193f42 --- /dev/null +++ b/plugin/skip/tests/Feature/SkipSummaryTest.php @@ -0,0 +1,91 @@ +summary; + Assert::same($summary->count(Status::Passed), 1); + Assert::same($summary->count(Status::Failed), 1); + Assert::same($summary->count(Status::Skipped), 2); + # Four tests total: the skipped data-driven one is counted once, not once per data set. + Assert::same($summary->total(), 4); + Assert::same($run->status, Status::Failed); + } + + /** + * A run consisting only of {@see Skip}-marked tests is a success: {@see Status::Skipped} + * is neither a success nor a failure, so nothing fails the run. + * + * The same run pins one result per skipped test. Every `#[Skip]` occurrence of the case + * spawns its own {@see SkipInterceptor} through the fallback alias; a second delivery would + * show up here as an inflated total and an extra name. + */ + public function runOfOnlySkippedTestsIsSuccessfulAndDeliveredOnce(): void + { + $run = self::run(__DIR__ . '/../Stub/SkipSummary/OnlySkipped'); + + Assert::same($run->status, Status::Passed); + Assert::same($run->summary->count(Status::Skipped), 2); + Assert::same($run->summary->total(), 2); + $cases = []; + foreach ($run as $suite) { + foreach ($suite as $case) { + $cases[] = $case; + } + } + # The directory holds one class with two skipped tests. + Assert::count($cases, 1); + $names = \array_map( + static fn(TestResult $result): string => $result->info->name, + \iterator_to_array($cases[0], preserve_keys: false), + ); + \sort($names); + Assert::same($names, ['firstSkipped', 'secondSkipped']); + } + + private static function run(string $path): RunResult + { + return Application::createFromConfig(new ApplicationConfig( + src: [], + suites: [ + new SuiteConfig( + 'SkipSummary', + location: new FinderConfig(include: [$path]), + ), + ], + ))->run(); + } +} diff --git a/plugin/skip/tests/Stub/PipelineEntrySpyPlugin.php b/plugin/skip/tests/Stub/PipelineEntrySpyPlugin.php new file mode 100644 index 00000000..32c970f2 --- /dev/null +++ b/plugin/skip/tests/Stub/PipelineEntrySpyPlugin.php @@ -0,0 +1,42 @@ + */ + public static array $entered = []; + + #[\Override] + public function configure(Container $container): void + { + $container->get(InterceptorCollector::class)->addInterceptor( + new class implements TestRunInterceptor { + #[\Override] + public function runTest(TestInfo $info, callable $next): TestResult + { + PipelineEntrySpyPlugin::$entered[] = $info->identity->fqn(); + + return $next($info); + } + }, + ); + } +} diff --git a/plugin/skip/tests/Stub/Skip/SkipChildStub.php b/plugin/skip/tests/Stub/Skip/SkipChildStub.php new file mode 100644 index 00000000..7d2de11a --- /dev/null +++ b/plugin/skip/tests/Stub/Skip/SkipChildStub.php @@ -0,0 +1,19 @@ + */ + public static array $log = []; + + #[Skip('skipped inside a fiber-driven case')] + public function skipped(): void + { + throw new \LogicException('Must never run: the test is skipped.'); + } + + public function first(): void + { + self::$log[] = 'first.1'; + \Fiber::suspend(); + self::$log[] = 'first.2'; + + # Round-robin: after the yield, "second" has had its first step in between. + Assert::same(\array_slice(self::$log, -3), ['first.1', 'second.1', 'first.2']); + } + + public function second(): void + { + self::$log[] = 'second.1'; + \Fiber::suspend(); + self::$log[] = 'second.2'; + + Assert::same(\array_slice(self::$log, -4), ['first.1', 'second.1', 'first.2', 'second.2']); + } +} diff --git a/plugin/skip/tests/Stub/Skip/SkipMarkerTrait.php b/plugin/skip/tests/Stub/Skip/SkipMarkerTrait.php new file mode 100644 index 00000000..a373127c --- /dev/null +++ b/plugin/skip/tests/Stub/Skip/SkipMarkerTrait.php @@ -0,0 +1,13 @@ + + */ + public static function provide(): array + { + ++self::$providerCalls; + + return [ + 'one' => [1], + 'two' => [2], + ]; + } + + #[Skip('data-driven test is skipped as a whole')] + #[DataProvider('provide')] + public function skipped(int $value): void + { + throw new \LogicException('Must never run: the test is skipped.'); + } +} diff --git a/plugin/skip/tests/Stub/Skip/SkipWithHooksStub.php b/plugin/skip/tests/Stub/Skip/SkipWithHooksStub.php new file mode 100644 index 00000000..e777a3fa --- /dev/null +++ b/plugin/skip/tests/Stub/Skip/SkipWithHooksStub.php @@ -0,0 +1,67 @@ +firstAttemptFailed) { + $this->firstAttemptFailed = true; + throw new \RuntimeException('First attempt fails by design.'); + } + } +} diff --git a/plugin/skip/tests/Stub/Skip/skip_functions.php b/plugin/skip/tests/Stub/Skip/skip_functions.php new file mode 100644 index 00000000..c281cf8a --- /dev/null +++ b/plugin/skip/tests/Stub/Skip/skip_functions.php @@ -0,0 +1,33 @@ + + */ + public static function provide(): iterable + { + yield [1]; + yield [2]; + } + + public function passes(): void + { + Assert::true(true); + } + + public function fails(): void + { + # Controlled failure: the skipped tests must not hide it from the totals. + Assert::true(false); + } + + #[Skip('skipped in the mixed case')] + public function skipped(): void + { + throw new \LogicException('Must never run: the test is skipped.'); + } + + #[Skip('data-driven test skipped as a whole')] + #[DataProvider('provide')] + public function skippedDataDriven(int $value): void + { + throw new \LogicException('Must never run: the test is skipped.'); + } +} diff --git a/plugin/skip/tests/Stub/SkipSummary/OnlySkipped/OnlySkippedStub.php b/plugin/skip/tests/Stub/SkipSummary/OnlySkipped/OnlySkippedStub.php new file mode 100644 index 00000000..b979159f --- /dev/null +++ b/plugin/skip/tests/Stub/SkipSummary/OnlySkipped/OnlySkippedStub.php @@ -0,0 +1,26 @@ +runTest( + self::createTestInfo(SkipMixedMethodsFixture::class, 'skipped'), + static function (TestInfo $info) use (&$nextCalled): TestResult { + $nextCalled = true; + return new TestResult(info: $info, status: Status::Passed); + }, + ); + + Assert::false($nextCalled); + Assert::same($result->status, Status::Skipped); + Assert::instanceOf($result->failure, SkipTest::class); + Assert::same($result->summary->count(Status::Skipped), 1); + } + + public function composesReasonAfterGeneratedPart(): void + { + $result = self::skip(SkipMixedMethodsFixture::class, 'skipped'); + + Assert::same( + $result->failure?->getMessage(), + SkipMixedMethodsFixture::class + . '::skipped is skipped via #[Skip] ==> broken by the pricing rework, see ISSUE-123', + ); + } + + /** + * An empty reason falls back to the generated part alone — no reporter ever shows an + * empty skip message. + */ + public function fallsBackToGeneratedMessageWithoutReason(): void + { + $result = self::skip(SkipMixedMethodsFixture::class, 'skippedNoReason'); + + Assert::same( + $result->failure?->getMessage(), + SkipMixedMethodsFixture::class . '::skippedNoReason is skipped via #[Skip]', + ); + } + + /** + * A test without an attribute of its own takes the class-level reason. + */ + public function classLevelReasonAppliesToATestWithoutItsOwn(): void + { + $result = self::skip(SkipClassLevelFixture::class, 'first'); + + Assert::true(\str_ends_with((string) $result->failure?->getMessage(), ' ==> entire case is skipped')); + } + + /** + * The method-level attribute wins as a whole, so an empty method reason is not filled in from + * the class reason. + */ + public function methodLevelReasonWinsOverTheClassLevelOne(): void + { + $own = self::skip(SkipClassLevelFixture::class, 'second'); + $empty = self::skip(SkipClassLevelFixture::class, 'third'); + + Assert::true(\str_ends_with((string) $own->failure?->getMessage(), ' ==> method beats class')); + Assert::same( + $empty->failure?->getMessage(), + SkipClassLevelFixture::class . '::third is skipped via #[Skip]', + ); + } + + /** + * The nearest declaration wins: an override repeating `#[Skip]` reports its own reason, not + * the one on the prototype it also inherits. + */ + public function ownReasonOfAnOverrideWinsOverTheInheritedOne(): void + { + $result = self::skip(SkipOwnReasonOverrideFixture::class, 'skipped'); + + Assert::true(\str_ends_with((string) $result->failure?->getMessage(), ' ==> own reason of the override')); + } + + /** + * The terminal renders a test's PHPDoc description from the result attributes (as the + * regular test path stamps it), so the skipped result must carry it too. + */ + public function carriesDescriptionInResult(): void + { + $result = self::skip(SkipMixedMethodsFixture::class, 'skipped'); + + Assert::same($result->attributes['description'], 'Checks that order totals include the reworked pricing.'); + Assert::same($result->attributes['duration'], 0); + } + + /** + * `#[Skip]` is a plain-test feature: without this declaration the attribute would skip a case + * of any other type too. + */ + public function declaresTestTypeScopingSkipToPlainTests(): void + { + $attributes = (new \ReflectionClass(SkipInterceptor::class)) + ->getAttributes(InterceptorOptions::class); + + Assert::count($attributes, 1); + Assert::same($attributes[0]->newInstance()->testType, TestType::Test); + } + + /** + * The rest of the placement contract: the slot sits outer to everything that prepares a test + * body, and one of the instances the attribute spawns per occurrence is enough. + */ + public function declaresOrderAndConflictPolicy(): void + { + $attributes = (new \ReflectionClass(SkipInterceptor::class)) + ->getAttributes(InterceptorOptions::class); + + Assert::count($attributes, 1); + $options = $attributes[0]->newInstance(); + Assert::true($options->order < InterceptorOptions::ORDER_DATA_PROVIDER - 1); + Assert::true($options->order > InterceptorOptions::ORDER_FILTER); + Assert::same($options->onConflict, ConflictPolicy::First); + } + + /** + * @param class-string $class + * @param non-empty-string $method + */ + private static function skip(string $class, string $method): TestResult + { + return (new SkipInterceptor())->runTest( + self::createTestInfo($class, $method), + static fn(TestInfo $info): TestResult => throw new \LogicException('Must never be reached.'), + ); + } + + /** + * @param class-string $class + * @param non-empty-string $method + */ + private static function createTestInfo(string $class, string $method): TestInfo + { + $definition = new TestDefinition(new \ReflectionMethod($class, $method)); + $caseDefinition = new CaseDefinition( + name: $class, + type: TestType::Test->value, + file: Path::create(__FILE__), + reflection: new \ReflectionClass($class), + tests: TestDefinitions::fromArray(...[$method => $definition]), + ); + $caseInfo = new CaseInfo(definition: $caseDefinition, suiteIdentity: new SuiteIdentity('Test/Unit')); + + return new TestInfo(name: $method, caseInfo: $caseInfo, testDefinition: $definition); + } +} diff --git a/plugin/skip/tests/Unit/Internal/SkipLocatorInterceptorTest.php b/plugin/skip/tests/Unit/Internal/SkipLocatorInterceptorTest.php new file mode 100644 index 00000000..ee2fd5c5 --- /dev/null +++ b/plugin/skip/tests/Unit/Internal/SkipLocatorInterceptorTest.php @@ -0,0 +1,186 @@ +locateTestCases(self::file(), self::next($case)); + + Assert::array($case->tests->getTests(skipped: true))->hasKeys('skipped', 'skippedNoReason'); + Assert::array($case->tests->getTests(skipped: false))->hasKeys('enabled'); + } + + public function classLevelSkipFlagsEveryTest(): void + { + $case = self::createCase(SkipClassLevelFixture::class, 'first', 'second', 'third'); + + (new SkipLocatorInterceptor())->locateTestCases(self::file(), self::next($case)); + + Assert::array($case->tests->getTests(skipped: false))->hasCount(0); + } + + /** + * A test stays a member of its case and stays active: the flag is the only change, so the + * case is neither dropped by the suite factory nor loses the test from its results. + */ + public function flaggedTestStaysActive(): void + { + $case = self::createCase(SkipMixedMethodsFixture::class, 'skipped', 'enabled'); + + (new SkipLocatorInterceptor())->locateTestCases(self::file(), self::next($case)); + + Assert::array($case->tests->getTests())->hasKeys('skipped', 'enabled'); + } + + /** + * `#[Skip]` on a non-test member is inert: the interceptor walks the tests only. + */ + public function skipOnANonTestMemberIsInert(): void + { + $case = self::createCaseWith(SkipMixedMethodsFixture::class, [ + 'skipped' => new TestDefinition( + new \ReflectionMethod(SkipMixedMethodsFixture::class, 'skipped'), + isTest: false, + ), + ]); + + (new SkipLocatorInterceptor())->locateTestCases(self::file(), self::next($case)); + + Assert::false($case->tests->all()['skipped']->skipped); + } + + /** + * The interceptor does not read `active`: a test an earlier filter deactivated is flagged all + * the same, and being inactive it is neither run nor reported either way. + */ + public function flagsADeactivatedTestToo(): void + { + $case = self::createCaseWith(SkipMixedMethodsFixture::class, [ + 'skipped' => new TestDefinition( + new \ReflectionMethod(SkipMixedMethodsFixture::class, 'skipped'), + active: false, + ), + ]); + + (new SkipLocatorInterceptor())->locateTestCases(self::file(), self::next($case)); + + Assert::true($case->tests->all()['skipped']->skipped); + } + + /** + * A flag set by someone else is not cleared by a test without `#[Skip]`. + */ + public function keepsAFlagSetElsewhere(): void + { + $case = self::createCaseWith(SkipMixedMethodsFixture::class, [ + 'enabled' => new TestDefinition( + new \ReflectionMethod(SkipMixedMethodsFixture::class, 'enabled'), + skipped: true, + ), + ]); + + (new SkipLocatorInterceptor())->locateTestCases(self::file(), self::next($case)); + + Assert::true($case->tests->all()['enabled']->skipped); + } + + /** + * `#[Skip]` is a plain-test feature: a bench or inline case of the same file is left alone. + */ + public function leavesCasesOfOtherTypesAlone(): void + { + $case = self::createCase(SkipClassLevelFixture::class, 'first'); + $bench = new CaseDefinition( + name: SkipClassLevelFixture::class, + type: TestType::BenchInline->value, + file: Path::create(__FILE__), + reflection: new \ReflectionClass(SkipClassLevelFixture::class), + tests: TestDefinitions::fromArray( + first: new TestDefinition(new \ReflectionMethod(SkipClassLevelFixture::class, 'first')), + ), + ); + + (new SkipLocatorInterceptor())->locateTestCases( + self::file(), + static fn(FileDefinitions $file): CaseDefinitions => CaseDefinitions::fromArray($case, $bench), + ); + + Assert::true($case->tests->all()['first']->skipped); + Assert::false($bench->tests->all()['first']->skipped); + } + + public function declaresTestTypeScopingSkipToPlainTests(): void + { + $attributes = (new \ReflectionClass(SkipLocatorInterceptor::class)) + ->getAttributes(InterceptorOptions::class); + + Assert::count($attributes, 1); + Assert::same($attributes[0]->newInstance()->testType, TestType::Test); + } + + /** + * @param class-string $class + * @param non-empty-string ...$methods + */ + private static function createCase(string $class, string ...$methods): CaseDefinition + { + $definitions = []; + foreach ($methods as $method) { + $definitions[$method] = new TestDefinition(new \ReflectionMethod($class, $method)); + } + + return self::createCaseWith($class, $definitions); + } + + /** + * @param class-string $class + * @param array $definitions + */ + private static function createCaseWith(string $class, array $definitions): CaseDefinition + { + return new CaseDefinition( + name: $class, + type: TestType::Test->value, + file: Path::create(__FILE__), + reflection: new \ReflectionClass($class), + tests: TestDefinitions::fromArray(...$definitions), + ); + } + + private static function file(): FileDefinitions + { + return new FileDefinitions(new TokenizedFile(file: new \SplFileInfo(__FILE__), path: __FILE__)); + } + + private static function next(CaseDefinition $case): \Closure + { + return static fn(FileDefinitions $file): CaseDefinitions => CaseDefinitions::fromArray($case); + } +} diff --git a/plugin/skip/tests/Unit/SkipAttributeTest.php b/plugin/skip/tests/Unit/SkipAttributeTest.php new file mode 100644 index 00000000..4a281e7d --- /dev/null +++ b/plugin/skip/tests/Unit/SkipAttributeTest.php @@ -0,0 +1,73 @@ +reason, ''); + } + + public function customReason(): void + { + $skip = new Skip('flaky on CI, see ISSUE-123'); + + Assert::same($skip->reason, 'flaky on CI, see ISSUE-123'); + } + + /** + * Exactly class, method and function — and nothing else, so no `IS_REPEATABLE`. + */ + public function targetsClassMethodAndFunctionOnly(): void + { + $attributes = (new \ReflectionClass(Skip::class))->getAttributes(\Attribute::class); + + Assert::count($attributes, 1); + /** @var \Attribute $attribute */ + $attribute = $attributes[0]->newInstance(); + + Assert::same( + $attribute->flags, + \Attribute::TARGET_CLASS | \Attribute::TARGET_METHOD | \Attribute::TARGET_FUNCTION, + ); + } + + /** + * The pipeline collects `Interceptable` attributes; without the marker a class-level + * `#[Skip]` would be invisible to the attributes interceptor. + */ + public function isInterceptable(): void + { + Assert::instanceOf(new Skip(), Interceptable::class); + } + + /** + * An `Interceptable` attribute must resolve to an interceptor, or the attributes + * interceptor throws at pipeline build time; the fallback names the handler. + */ + public function declaresSkipInterceptorAsFallback(): void + { + $attributes = (new \ReflectionClass(Skip::class))->getAttributes(FallbackInterceptor::class); + + Assert::count($attributes, 1); + Assert::same($attributes[0]->newInstance()->class, SkipInterceptor::class); + } +} diff --git a/plugin/skip/tests/Unit/SkipPluginTest.php b/plugin/skip/tests/Unit/SkipPluginTest.php new file mode 100644 index 00000000..f620774c --- /dev/null +++ b/plugin/skip/tests/Unit/SkipPluginTest.php @@ -0,0 +1,44 @@ +addsInterceptor(SkipLocatorInterceptor::class) + ->addsInterceptors(1) + ->addsListeners(0); + } + + /** + * The lifecycle hooks learn about a skip from the flag the plugin sets, so the plugin has to be + * on by default, like the lifecycle plugin itself. + */ + public function isAmongTheDefaultSuitePlugins(): void + { + $classes = \array_map( + static fn(object $plugin): string => $plugin::class, + \iterator_to_array(SuitePlugins::defaults(), false), + ); + + Assert::contains($classes, SkipPlugin::class); + } +} diff --git a/plugin/skip/tests/suites.php b/plugin/skip/tests/suites.php new file mode 100644 index 00000000..63b4cded --- /dev/null +++ b/plugin/skip/tests/suites.php @@ -0,0 +1,22 @@ +skipped = true` on each of them +(`TestDefinition::$skipped`). A flagged test stays active, so the case keeps it and reports it: the +core returns a `Status::Skipped` result at the entry of its pipeline (no `TestStarting`, no body), +the lifecycle plugin runs no per-test hooks for it, and a case whose active tests are all flagged +gets no class-level hooks either. To attach a reason, pair the flag with a per-test interceptor that +returns the `Skipped` result itself, ordered outer to data providers and fibers +(`InterceptorOptions::ORDER_FILTER + 1_000` is the slot `#[Skip]` uses). + +The shipped implementation of exactly this shape is `plugin/skip`: `SkipLocatorInterceptor` flags, +`SkipInterceptor` reports with the reason, serving the `#[Skip]` attribute (whose contract is in the +`testo-write-tests` skill). Read them as a reference — they are `@internal` (and `final`), don't +import them. + ## Container scopes — provision per-case / per-suite resources `$container->scope($closure)` runs `$closure` in a **child scope**: services bound inside live only for @@ -242,9 +260,20 @@ $method = $info->testDefinition->reflection; $optedOut = $method->getAttributes(WithoutTransaction::class) !== []; ``` +An attribute can also bring its own interceptor, so users need no plugin registration at all — +`#[Retry]`, `#[Repeat]` and `#[Skip]` ship this way. Implement `Testo\Pipeline\Attribute\Interceptable` +and name the handler with `#[FallbackInterceptor(MyInterceptor::class)]` (repeatable — one per +pipeline position); the core instantiates the interceptor with the attribute instance as a constructor +argument when the attribute is found on a class (case and test pipelines) or on a test (test pipeline +only). A test-level attribute never reaches the case pipeline: whatever has to be known about the +case ahead of the run (a skipped test, say) is flagged on the definitions from a +`CaseLocatorInterceptor` registered by a plugin instead. + ## Pitfalls - **Skipping**: return a `Status::Skipped` `TestResult`; never `throw SkipTest` from an interceptor. + Never return early from a **case** interceptor to skip — flag the definitions as `skipped` from a + locator instead, and let the core report them. - **Cleanup**: wrap `$next()` in `try/finally`; a later interceptor may throw. - **State**: prefer pipeline attributes / container scope over mutable interceptor fields. - **Listeners** observe; **interceptors** change behaviour. Don't try to alter a run from a listener. diff --git a/skills/testo-write-tests/SKILL.md b/skills/testo-write-tests/SKILL.md index efec36e8..6d1f63fb 100644 --- a/skills/testo-write-tests/SKILL.md +++ b/skills/testo-write-tests/SKILL.md @@ -135,6 +135,50 @@ Constraints: - Subclasses work: `class MissingExtensionSkip extends SkipTest {}` is still recognized. - Return type stays `void`, or `never` if the throw is unconditional. +## Skipping a test with #[Skip] + +To skip a test declaratively — without running any of its code — put `Testo\Skip` (from the +`testo/skip` plugin) on the test method (inherited by an overriding +method that does not repeat it), the class (skips every test of the case; inherited from parents +and traits, a method-level reason wins), or a free function: + +```php +use Testo\Skip; + +#[Test] +#[Skip('broken by the pricing rework, see ISSUE-123')] +public function calculatesTotal(): void { /* ... */ } // reported as Skipped, body never runs +``` + +The test is reported as `Status::Skipped` and counted in the totals; its reason travels in the +result's failure message `{testId} is skipped via #[Skip] ==> {reason}` (without ` ==> ...` when +the reason is empty). The JUnit, TeamCity and HTML reports show that message; the terminal prints +the skipped line without it, and the compact `--json` report only counts the test in +`totals.skipped`. + +`reason` is optional and the attribute is not repeatable — but **always pass a reason that points +at an issue** (`#[Skip('flaky on CI, see ISSUE-123')]`); a bare `#[Skip]` is how a skipped test rots +unreviewed. The attribute needs no plugin registration: it wires its own interceptor, from a class, +a method or a function alike. + +Which skipping tool to reach for: + +| Tool | Decided by | Visibility | Use when | +|---|---|---|---| +| `#[Skip('...')]` | code, ahead of time | always reported; reason in JUnit/TeamCity/HTML | the test is knowingly broken, tracked in an issue, and must be returned to | +| `throw SkipTest` | test body, at runtime | reported when the run gets there | test isn't applicable in this environment | +| `#[Group]` + `--group=!x` | runner invocation | invisible — filtered out of reports | a category you sometimes don't run | + +Runtime contract of `#[Skip]`: the test is reported at the entry of its pipeline, so +`#[BeforeTest]`/`#[AfterTest]`, data providers, `#[Retry]`/`#[Repeat]`, fibers and coverage never +engage, and a data-driven test yields a single Skipped entry (the provider is not called). +`#[BeforeClass]`/`#[AfterClass]` run when the case still has a test to run; when every test of the +case is skipped they stay silent and the case class is never constructed (enabled neighbors +construct it as usual). A run of only `#[Skip]`-marked tests is a success (exit 0). `#[Skip]` +applies to plain tests only: on a `#[Bench]` or `#[TestInline]` target it is inert — the benchmark +or inline case runs as usual. The `testo/skip` plugin (`Testo\Skip\SkipPlugin`) is part of the +default suite plugins; it is what tells the lifecycle hooks about the skip ahead of the run. + ## Tests that intentionally perform no assertions A test that finishes successfully without recording a single assertion is reported as diff --git a/testo.php b/testo.php index 859dc758..69b9941c 100644 --- a/testo.php +++ b/testo.php @@ -72,6 +72,7 @@ require 'plugin/lifecycle/tests/suites.php', require 'plugin/repeat/tests/suites.php', require 'plugin/retry/tests/suites.php', + require 'plugin/skip/tests/suites.php', require 'plugin/test/tests/suites.php', require 'tests/Testo/suites.php', require 'tests/Application/suites.php', diff --git a/tests/Application/Feature/Runner/SkippedDefinitionTest.php b/tests/Application/Feature/Runner/SkippedDefinitionTest.php new file mode 100644 index 00000000..62403af1 --- /dev/null +++ b/tests/Application/Feature/Runner/SkippedDefinitionTest.php @@ -0,0 +1,66 @@ +status, Status::Skipped); + Assert::instanceOf($result->failure, SkipTest::class); + Assert::same($result->failure->getMessage(), SkippedByLocator::class . '::flagged is skipped'); + Assert::false(SkippedByLocator::$flaggedRan); + } + + public function flaggedTestIsCountedAsSkipped(): void + { + $result = TestingRunner::runTest([SkippedByLocator::class, 'flagged']); + + Assert::same($result->summary->counts, [Status::Skipped->name => 1]); + } + + /** + * `TestStarting` announces a test body; a skipped definition has none, so only the enabled + * neighbor is announced. + */ + public function noTestStartingIsDispatchedForAFlaggedTest(): void + { + $offset = \count(FlagSkippedPlugin::$started); + + TestingRunner::runTest([SkippedByLocator::class, 'flagged']); + + Assert::array(\array_slice(FlagSkippedPlugin::$started, $offset)) + ->contains('enabled') + ->notContains('flagged'); + } + + public function enabledNeighborStillRuns(): void + { + $result = TestingRunner::runTest([SkippedByLocator::class, 'enabled']); + + Assert::same($result->status, Status::Passed); + } +} diff --git a/tests/Application/Stub/Skipped/FlagSkippedPlugin.php b/tests/Application/Stub/Skipped/FlagSkippedPlugin.php new file mode 100644 index 00000000..ba5c7bda --- /dev/null +++ b/tests/Application/Stub/Skipped/FlagSkippedPlugin.php @@ -0,0 +1,54 @@ + */ + public static array $started = []; + + #[\Override] + public function configure(Container $container): void + { + $container->get(InterceptorCollector::class)->addInterceptor( + new class implements CaseLocatorInterceptor { + #[\Override] + public function locateTestCases(FileDefinitions $file, callable $next): CaseDefinitions + { + /** @var CaseDefinitions $result */ + $result = $next($file); + foreach ($result->getCases() as $case) { + foreach ($case->tests->getTests() as $name => $test) { + $name === 'flagged' and $test->skipped = true; + } + } + + return $result; + } + }, + ); + + $container->get(EventListenerCollector::class)->addListener( + TestStarting::class, + static function (TestStarting $event): void { + self::$started[] = $event->testInfo->name; + }, + ); + } +} diff --git a/tests/Application/Stub/Skipped/SkippedByLocator.php b/tests/Application/Stub/Skipped/SkippedByLocator.php new file mode 100644 index 00000000..761b3343 --- /dev/null +++ b/tests/Application/Stub/Skipped/SkippedByLocator.php @@ -0,0 +1,30 @@ +filter(isTest: false))->hasCount(0); } + /** + * A skipped test stays in the active test set — it is still reported — and leaves only the set + * of tests whose body runs. + */ + public function skippingATestKeepsItActiveButOutOfTheRunnableSet(): void + { + $definitions = new TestDefinitions(); + $definitions->define(new \ReflectionFunction('strlen'))->skipped = true; + $definitions->define(new \ReflectionFunction('strrev')); + + Assert::array($definitions->getTests())->hasKeys('strlen', 'strrev'); + Assert::array($definitions->getTests(skipped: false))->hasKeys('strrev')->doesNotHaveKeys('strlen'); + Assert::array($definitions->getTests(skipped: true))->hasKeys('strlen')->doesNotHaveKeys('strrev'); + Assert::array($definitions->filter(skipped: true))->hasCount(1); + } + /** * `all()` returns every definition, and `filter()` slices it over both flags — a null * constraint matches either value. diff --git a/tests/Output/Unit/JUnit/JUnitWriterTest.php b/tests/Output/Unit/JUnit/JUnitWriterTest.php index 77a4e4cc..613f387b 100644 --- a/tests/Output/Unit/JUnit/JUnitWriterTest.php +++ b/tests/Output/Unit/JUnit/JUnitWriterTest.php @@ -18,6 +18,7 @@ use Testo\Core\Context\TestResult; use Testo\Core\Definition\CaseDefinition; use Testo\Core\Definition\TestDefinition; +use Testo\Core\Exception\SkipTest; use Testo\Core\Value\Status; use Testo\Output\JUnit\Internal\JUnitWriter; use Testo\Test; @@ -163,6 +164,52 @@ public function skippedTestRendersSkippedElement(): void Assert::count($xml->testsuite->testcase->skipped, 1); } + /** + * The failure message is the single source of truth for the skip reason: every producer + * (a runtime throw, a declarative `#[Skip]`) delivers it the same way, and the writer + * renders it as the `message` of ``. + */ + #[Covers(JUnitWriter::class)] + public function skippedTestCarriesTheReasonFromTheFailureMessage(): void + { + $writer = new JUnitWriter(); + $writer->startSuite('MySuite'); + $writer->addTestResult(self::makeResult( + 'passingTest', + Status::Skipped, + failure: new SkipTest('sqlite extension is missing'), + )); + $writer->finishSuite(); + + $xml = self::loadXml($writer->generate('Testo')); + + $skipped = $xml->testsuite->testcase->skipped; + Assert::count($skipped, 1); + Assert::same((string) $skipped['message'], 'sqlite extension is missing'); + } + + /** + * No reason — no `message`: an empty attribute would read as an empty reason. + */ + #[Covers(JUnitWriter::class)] + public function skippedTestWithoutAReasonOmitsTheMessage(): void + { + $writer = new JUnitWriter(); + $writer->startSuite('MySuite'); + $writer->addTestResult(self::makeResult( + 'passingTest', + Status::Skipped, + failure: new SkipTest(), + )); + $writer->finishSuite(); + + $xml = self::loadXml($writer->generate('Testo')); + + $skipped = $xml->testsuite->testcase->skipped; + Assert::count($skipped, 1); + Assert::null($skipped['message']); + } + public function cancelledTestCountsAsSkipped(): void { // Arrange diff --git a/tools/phpunit/infection.phpunit.json b/tools/phpunit/infection.phpunit.json index 1cda0608..b79b1828 100644 --- a/tools/phpunit/infection.phpunit.json +++ b/tools/phpunit/infection.phpunit.json @@ -15,6 +15,7 @@ "../../plugin/lifecycle/src", "../../plugin/repeat/src", "../../plugin/retry/src", + "../../plugin/skip/src", "../../plugin/test/src", "../../bridge/double/src", "../../bridge/mockery/src", diff --git a/tools/phpunit/phpunit.xml b/tools/phpunit/phpunit.xml index 4b56a7fd..91a08397 100644 --- a/tools/phpunit/phpunit.xml +++ b/tools/phpunit/phpunit.xml @@ -66,6 +66,7 @@ ../../plugin/filter/Filter.php ../../plugin/repeat/Repeat.php ../../plugin/retry/Retry.php + ../../plugin/skip/Skip.php ../../plugin/test/Test.php ../../bridge/vcr/VCR.php