diff --git a/bridge/rector/FEATURE_PARITY.md b/bridge/rector/FEATURE_PARITY.md index ffbda363..fe19e23a 100644 --- a/bridge/rector/FEATURE_PARITY.md +++ b/bridge/rector/FEATURE_PARITY.md @@ -19,7 +19,8 @@ Conversion coverage across the three directions supported by `testo/bridge-recto | **Exception expectation (bare)** | ✅ *bare `\Testo\Expect::exception($c)` → `$this->expectException($c)` (`ExpectExceptionToPhpUnitRector`); the attribute form `#[\Testo\Assert\ExpectException($c)]` → prepended `$this->expectException($c)` (`ExpectExceptionAttributeToPhpUnitRector`)* | 🟡 | ✅ *`TestCallToFunctionRector` folds `->throws(X::class)` into a prepended `\Testo\Expect::exception(X)` + `never` return type* | | **Exception message/code (fluent)** `withMessage/withCode` ↔ `expectExceptionMessage/Code` | ✅ *`ExpectExceptionToPhpUnitRector` expands one chain into several statements (`withMessage`→`expectExceptionMessage`, `withCode`→`expectExceptionCode`, regex `withMessagePattern`→`expectExceptionMessageMatches`); substring `withMessageContaining` aborts the chain (no faithful PCRE target)* | ✅ *`ExpectExceptionToTestoRector` folds an uninterrupted run of sibling `expectExceptionMessage/Code` after `expectException` into the `->withMessage()/->withCode()` chain (StmtsAware); a non-foldable call ends the run* | 🟡 *`->throws(X, 'msg')`'s second arg folds to `->withMessage('msg')`; Pest has no exception-code modifier to map* | | **Exception message by regex** (`expectExceptionMessageMatches`) | ➖ | ⛔ *Testo's `withMessageContaining` is substring, not regex* | ➖ | -| **Skip** (`throw SkipTest` ↔ `markTestSkipped`) | ✅ | ✅ | 🟡 *`->skip('reason')` → prepended `throw new \Testo\Core\Exception\SkipTest('reason')`; a conditional `->skip(fn () => …)` is left untouched* | +| **Skip at runtime** (`throw SkipTest` ↔ `markTestSkipped`) | ✅ | ✅ *`MarkTestSkippedToTestoRector` takes what the attribute form below leaves: a guarded call, a call deeper in the body, a non-literal message* | ➖ *every Pest `->skip()` is unconditional — see the row below* | +| **Skip declaratively** (`#[Skip]`) | ✅ *`SkipAttributeToPhpUnitRector`: PHPUnit has no skip attribute, so `#[\Testo\Skip('reason')]` becomes a leading `$this->markTestSkipped('reason')` and the attribute is dropped; a class-level attribute is fanned out onto each test method (a method's own reason wins). **Residual:** the skip turns into a runtime one — PHPUnit runs `setUp()` and the data provider before aborting, where Testo keeps the test out of the pipeline entirely* | ✅ *`MarkTestSkippedToSkipAttributeRector`: an unconditional `markTestSkipped('literal')` opening a test method → `#[\Testo\Skip('literal')]`, dropping the call. A guarded/non-literal one stays a throw (row above); a call opening `setUp()` stays a throw too, rather than becoming a class-level `#[Skip]`* | ✅ *`->skip()` / `->skip('reason')` → `#[\Testo\Skip('reason')]`; a conditional `->skip(fn () => …)` / `->skip($bool, 'reason')` leaves the whole statement untouched* | | **Incomplete** (`markTestIncomplete`) | ➖ | 🟡 *`MarkTestIncompleteRector`: Testo has no Incomplete status, so it maps to the nearest one — a `throw new \Testo\Core\Exception\SkipTest(...)` (Skipped). Lossy: the Incomplete-vs-Skipped nuance survives only as an `Incomplete: ` prefix on the reason (literal folds to `'Incomplete: '`, non-literal to `'Incomplete: ' . $expr`, bare call to `'Incomplete'`)* | ➖ | | **Cancel** (`CancelTest`) | ⛔ *no PHPUnit equivalent* | ➖ | ➖ | | **Coverage attribute** (`#[Covers]` ↔ `#[CoversClass]`) | ✅ *`CoversToCoversClassRector` maps by target kind: a class/enum → `#[CoversClass]`, a trait → `#[CoversTrait]`, an interface is dropped (PHPUnit rejects a non-class coverage target); kind resolved by autoload, an unresolvable name defaults to `#[CoversClass]`* | ✅ | ✅ *`->covers(X::class)` → `#[\Testo\Codecov\Covers(X::class)]`* | @@ -115,3 +116,12 @@ Retry/Repeat moved off this list: PHPUnit 13.3 added `#[Repeat]`/`#[Retry]`, so convert as a documented 🟡 (`RepeatRetryRector` / `RepeatRetryToTestoRector`). PHPUnit's `markTestIncomplete` moved off this list — it now converts to a Skipped throw with an `Incomplete: ` reason prefix (`MarkTestIncompleteRector`), a documented lossy 🟡 rather than a ⛔. + +Also done: **declarative skip** — Testo's `#[Skip]` attribute (`testo/skip`) splits the skip rows in +two. A skip that states something about the test rather than about a path through it now converts as +an attribute in every direction: `SkipAttributeToPhpUnitRector` unrolls it into the leading +`markTestSkipped()` call PHPUnit needs (fanning a class-level attribute onto each test method), and +both reverse directions produce it — a `markTestSkipped('literal')` opening a test method +(`MarkTestSkippedToSkipAttributeRector`) and Pest's `->skip('reason')` modifier. What stays a +`SkipTest` throw is exactly what cannot be declared: a guarded call, one deeper in the body, or a +message no attribute argument can hold. diff --git a/bridge/rector/config/phpunit-to-testo.php b/bridge/rector/config/phpunit-to-testo.php index af506ade..6776b2e0 100644 --- a/bridge/rector/config/phpunit-to-testo.php +++ b/bridge/rector/config/phpunit-to-testo.php @@ -14,6 +14,7 @@ use Testo\Bridge\Rector\PhpunitToTesto\GroupToTestoRector; use Testo\Bridge\Rector\PhpunitToTesto\LifecycleMethodToTestoRector; use Testo\Bridge\Rector\PhpunitToTesto\MarkTestIncompleteRector; +use Testo\Bridge\Rector\PhpunitToTesto\MarkTestSkippedToSkipAttributeRector; use Testo\Bridge\Rector\PhpunitToTesto\MarkTestSkippedToTestoRector; use Testo\Bridge\Rector\PhpunitToTesto\MergeAssertChainRector; use Testo\Bridge\Rector\PhpunitToTesto\RepeatRetryToTestoRector; @@ -35,6 +36,9 @@ # emptiness) rather than a flat facade call — see TypedAssertCallToTestoRector. $rectorConfig->rule(TypedAssertCallToTestoRector::class); + # A skip that opens a test method is a property of the test: it becomes `#[Skip]`, and only what + # is left — a guarded or non-literal skip — falls through to the throw form below. + $rectorConfig->rule(MarkTestSkippedToSkipAttributeRector::class); $rectorConfig->rule(MarkTestSkippedToTestoRector::class); # Incomplete has no exact Testo status; mapped to a Skipped throw with an "Incomplete:" reason diff --git a/bridge/rector/config/testo-to-phpunit.php b/bridge/rector/config/testo-to-phpunit.php index d660c315..7d84681d 100644 --- a/bridge/rector/config/testo-to-phpunit.php +++ b/bridge/rector/config/testo-to-phpunit.php @@ -14,6 +14,7 @@ use Testo\Bridge\Rector\TestoToPhpunit\GroupToPhpUnitRector; use Testo\Bridge\Rector\TestoToPhpunit\LifecycleAttributesToPhpUnitRector; use Testo\Bridge\Rector\TestoToPhpunit\RepeatRetryRector; +use Testo\Bridge\Rector\TestoToPhpunit\SkipAttributeToPhpUnitRector; use Testo\Bridge\Rector\TestoToPhpunit\TestClassToTestCaseRector; use Testo\Bridge\Rector\TestoToPhpunit\ThrowSkipTestToPhpUnitRector; use Testo\Bridge\Rector\TestoToPhpunit\TypedAssertChainRector; @@ -28,6 +29,7 @@ return static function (RectorConfig $rectorConfig): void { $rectorConfig->rule(AssertCallToPhpUnitRector::class); $rectorConfig->rule(ThrowSkipTestToPhpUnitRector::class); + $rectorConfig->rule(SkipAttributeToPhpUnitRector::class); $rectorConfig->rule(CoversToCoversClassRector::class); $rectorConfig->rule(LifecycleAttributesToPhpUnitRector::class); $rectorConfig->rule(ExpectExceptionToPhpUnitRector::class); diff --git a/bridge/rector/src/PestToTesto/TODO.md b/bridge/rector/src/PestToTesto/TODO.md index 0aa81929..5fb10b69 100644 --- a/bridge/rector/src/PestToTesto/TODO.md +++ b/bridge/rector/src/PestToTesto/TODO.md @@ -25,7 +25,8 @@ synthesize a class, we synthesize functions. - `->group('a','b')` → `#[\Testo\Filter\Group('a','b')]` - `->covers(X::class)` → `#[\Testo\Codecov\Covers(X::class)]` - `->throws(X::class[, 'msg'])` → prepended `\Testo\Expect::exception(X)[->withMessage('msg')]`, return type `never` - - `->skip(['reason'])` → prepended `throw new \Testo\Core\Exception\SkipTest('reason')` + - `->skip(['reason'])` → `#[\Testo\Skip('reason')]` (the modifier is unconditional, so the + declarative attribute is the faithful form) - `->with([ ])` → one `#[\Testo\Data\DataSet([...])]` per row (inline array literal only) - **`ExpectToAssertRector`** — runs after the structural rule and maps each `expect($value)->toX(...)` expectation inside the generated bodies to the matching actual-first diff --git a/bridge/rector/src/PestToTesto/TestCallToFunctionRector.php b/bridge/rector/src/PestToTesto/TestCallToFunctionRector.php index 5bb007cf..a768a22f 100644 --- a/bridge/rector/src/PestToTesto/TestCallToFunctionRector.php +++ b/bridge/rector/src/PestToTesto/TestCallToFunctionRector.php @@ -13,9 +13,7 @@ use PhpParser\Node\Expr\Closure; use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\MethodCall; -use PhpParser\Node\Expr\New_; use PhpParser\Node\Expr\StaticCall; -use PhpParser\Node\Expr\Throw_; use PhpParser\Node\Identifier; use PhpParser\Node\Name; use PhpParser\Node\Name\FullyQualified; @@ -59,7 +57,7 @@ * - `->group('a','b')` => `#[\Testo\Filter\Group('a','b')]` * - `->covers(X::class)` => `#[\Testo\Codecov\Covers(X::class)]` (repeatable) * - `->throws(X::class[, 'msg'])` => prepended `\Testo\Expect::exception(X::class)[->withMessage('msg')]`, return type `never` - * - `->skip(['reason'])` => prepended `throw new \Testo\Core\Exception\SkipTest('reason')` + * - `->skip(['reason'])` => `#[\Testo\Skip('reason')]` * - `->with([ ])` => one `#[\Testo\Data\DataSet([...])]` per row (array literal only) * * Operates at the statements level ({@see StmtsAwareInterface}) so the file/namespace body is @@ -358,8 +356,10 @@ private function throwsModifier(array $args): ?array } /** - * `->skip()` / `->skip('reason')` => prepended `throw new \Testo\Core\Exception\SkipTest(['reason'])`. - * A conditional skip (`->skip(fn () => …)` / `->skip($bool, 'reason')`) is unsupported. + * `->skip()` / `->skip('reason')` => `#[\Testo\Skip(['reason'])]`. Both are unconditional + * declarations about the test, which is what the attribute expresses — the test never enters + * the pipeline. A conditional skip (`->skip(fn () => …)` / `->skip($bool, 'reason')`) is a + * runtime decision and is unsupported. * * @param array $args * @return array{attributes: list, prepend: list, returnType: ?non-empty-string}|null @@ -375,9 +375,11 @@ private function skipModifier(array $args): ?array $skipArgs = [$args[0]]; } - $throw = new Throw_(new New_(new FullyQualified('Testo\\Core\\Exception\\SkipTest'), $skipArgs)); - - return ['attributes' => [], 'prepend' => [new Expression($throw)], 'returnType' => null]; + return [ + 'attributes' => [$this->attribute('Testo\\Skip', $skipArgs)], + 'prepend' => [], + 'returnType' => null, + ]; } /** diff --git a/bridge/rector/src/PestToTesto/TestCallToFunctionRector/skip_modifier.php.inc b/bridge/rector/src/PestToTesto/TestCallToFunctionRector/skip_modifier.php.inc index e6518cc6..f778c522 100644 --- a/bridge/rector/src/PestToTesto/TestCallToFunctionRector/skip_modifier.php.inc +++ b/bridge/rector/src/PestToTesto/TestCallToFunctionRector/skip_modifier.php.inc @@ -8,8 +8,8 @@ it('is work in progress', function () { /** is work in progress */ #[\Testo\Test] +#[\Testo\Skip('not ready')] function it_is_work_in_progress(): void { - throw new \Testo\Core\Exception\SkipTest('not ready'); doThing(); } diff --git a/bridge/rector/src/PhpunitToTesto/MarkTestSkippedToSkipAttributeRector.php b/bridge/rector/src/PhpunitToTesto/MarkTestSkippedToSkipAttributeRector.php new file mode 100644 index 00000000..dca81fdd --- /dev/null +++ b/bridge/rector/src/PhpunitToTesto/MarkTestSkippedToSkipAttributeRector.php @@ -0,0 +1,177 @@ +markTestSkipped($reason)`, a concatenation with a variable) cannot live in an attribute + * argument. `self::`/`static::` calls convert the same way as `$this->`. + * + * **Residual:** a `markTestSkipped()` opening `setUp()` skips every test of the class in PHPUnit; + * that stays a throw rather than becoming a class-level `#[Skip]`, so the hook keeps whatever else + * it does. + */ +#[TestRectorFixtures('MarkTestSkippedToSkipAttributeRector')] +final class MarkTestSkippedToSkipAttributeRector extends AbstractRector +{ + private const SKIP_TESTO = 'Testo\\Skip'; + + /** @var list */ + private const TEST_ATTRIBUTES = ['PHPUnit\\Framework\\Attributes\\Test', 'Testo\\Test']; + + /** @var list */ + private const LIFECYCLE_NAMES = ['setup', 'teardown', 'setupbeforeclass', 'teardownafterclass']; + + public function getRuleDefinition(): RuleDefinition + { + return new RuleDefinition( + 'Convert a leading unconditional markTestSkipped() into the Testo #[Skip] attribute', + [ + new CodeSample( + <<<'PHP' + public function testSomething(): void + { + $this->markTestSkipped('not ready'); + $this->doSomething(); + } + PHP, + <<<'PHP' + #[\Testo\Skip('not ready')] + public function testSomething(): void + { + $this->doSomething(); + } + PHP, + ), + ], + ); + } + + #[\Override] + public function getNodeTypes(): array + { + return [ClassMethod::class]; + } + + /** + * @param ClassMethod $node + */ + #[\Override] + public function refactor(Node $node): ?Node + { + if ($node->stmts === null || !$this->isTestMethod($node)) { + return null; + } + + $first = $node->stmts[0] ?? null; + if (!$first instanceof Expression) { + return null; + } + + $args = $this->skipCallArgs($first->expr); + if ($args === null) { + return null; + } + + \array_shift($node->stmts); + $node->attrGroups[] = new AttributeGroup([ + new Attribute(new FullyQualified(self::SKIP_TESTO), $args), + ]); + + return $node; + } + + /** + * The `#[Skip]` arguments for a `markTestSkipped()` call expression: none for a message-less + * call, the literal message otherwise. Null when the expression is not such a call at all, or + * carries a message an attribute cannot hold — a non-literal one stays a throw. + * + * @return list|null + */ + private function skipCallArgs(Node\Expr $expr): ?array + { + if ($expr instanceof MethodCall) { + if (!$this->isName($expr->var, 'this')) { + return null; + } + } elseif ($expr instanceof StaticCall) { + if (!$this->isName($expr->class, 'self') && !$this->isName($expr->class, 'static')) { + return null; + } + } else { + return null; + } + + if (!$this->isName($expr->name, 'markTestSkipped')) { + return null; + } + + $message = $expr->args[0] ?? null; + if ($message === null) { + return []; + } + + return $message instanceof Arg && $message->value instanceof String_ + ? [new Arg(new String_($message->value->value))] + : null; + } + + /** + * Whether the method is a PHPUnit test: public, non-static, not a lifecycle hook, and either + * `test`-prefixed or carrying a `#[Test]` attribute (PHPUnit's or the converted Testo one). + */ + private function isTestMethod(ClassMethod $method): bool + { + if (!$method->isPublic() || $method->isStatic()) { + return false; + } + + $name = \strtolower((string) $this->getName($method)); + if (\in_array($name, self::LIFECYCLE_NAMES, true)) { + return false; + } + + return \str_starts_with($name, 'test') || $this->hasTestAttribute($method); + } + + private function hasTestAttribute(ClassMethod $method): bool + { + foreach ($method->attrGroups as $attrGroup) { + foreach ($attrGroup->attrs as $attr) { + foreach (self::TEST_ATTRIBUTES as $name) { + if ($this->isName($attr->name, $name)) { + return true; + } + } + } + } + + return false; + } +} diff --git a/bridge/rector/src/PhpunitToTesto/MarkTestSkippedToSkipAttributeRector/guarded_call_left_to_throw_rule.php.inc b/bridge/rector/src/PhpunitToTesto/MarkTestSkippedToSkipAttributeRector/guarded_call_left_to_throw_rule.php.inc new file mode 100644 index 00000000..d0636dcd --- /dev/null +++ b/bridge/rector/src/PhpunitToTesto/MarkTestSkippedToSkipAttributeRector/guarded_call_left_to_throw_rule.php.inc @@ -0,0 +1,11 @@ +markTestSkipped('needs pcntl'); + } + } +} diff --git a/bridge/rector/src/PhpunitToTesto/MarkTestSkippedToSkipAttributeRector/leading_call_with_message.php.inc b/bridge/rector/src/PhpunitToTesto/MarkTestSkippedToSkipAttributeRector/leading_call_with_message.php.inc new file mode 100644 index 00000000..7bbb051c --- /dev/null +++ b/bridge/rector/src/PhpunitToTesto/MarkTestSkippedToSkipAttributeRector/leading_call_with_message.php.inc @@ -0,0 +1,21 @@ +markTestSkipped('not ready'); + $this->doSomething(); + } +} +----- +doSomething(); + } +} diff --git a/bridge/rector/src/PhpunitToTesto/MarkTestSkippedToSkipAttributeRector/leading_call_without_message.php.inc b/bridge/rector/src/PhpunitToTesto/MarkTestSkippedToSkipAttributeRector/leading_call_without_message.php.inc new file mode 100644 index 00000000..30fbf34b --- /dev/null +++ b/bridge/rector/src/PhpunitToTesto/MarkTestSkippedToSkipAttributeRector/leading_call_without_message.php.inc @@ -0,0 +1,19 @@ +markTestSkipped(); + } +} +----- +markTestSkipped($this->reason()); + } +} diff --git a/bridge/rector/src/PhpunitToTesto/MarkTestSkippedToSkipAttributeRector/self_static_call.php.inc b/bridge/rector/src/PhpunitToTesto/MarkTestSkippedToSkipAttributeRector/self_static_call.php.inc new file mode 100644 index 00000000..125d2925 --- /dev/null +++ b/bridge/rector/src/PhpunitToTesto/MarkTestSkippedToSkipAttributeRector/self_static_call.php.inc @@ -0,0 +1,21 @@ +markTestSkipped('the whole case is on hold'); + } +} diff --git a/bridge/rector/src/PhpunitToTesto/TODO.md b/bridge/rector/src/PhpunitToTesto/TODO.md index cc208fcc..152cbdd4 100644 --- a/bridge/rector/src/PhpunitToTesto/TODO.md +++ b/bridge/rector/src/PhpunitToTesto/TODO.md @@ -23,6 +23,14 @@ exist for each so the intent and blockers are discoverable in code. ## Implemented since the first cut +- **MarkTestSkippedToSkipAttributeRector** (registered) — a `markTestSkipped('literal')` opening a test + method is a statement about the test, not about a path through it, so it becomes `#[\Testo\Skip('literal')]` + and the call is dropped. That buys what the throw cannot: Testo keeps such a test out of the per-test + pipeline entirely (no `#[BeforeTest]`, no data-provider call, no retries). Everything else keeps + converting to a `SkipTest` throw via `MarkTestSkippedToTestoRector`, which is registered right after + it — a guarded call, one deeper in the body, and a non-literal message no attribute argument can hold. + A call opening `setUp()` also stays a throw: hoisting it to a class-level `#[Skip]` would strip the rest + of the hook. - **CreateMockToDoubleRector** (registered) — converts PHPUnit mocks/stubs onto the Double bridge (`testo/bridge-double`), which gives the previously-missing target API. `$this->createMock(X)` / `$this->createStub(X)` → `\JMac\Testing\Double::for(X)`, `create{Mock,Stub}ForIntersectionOfInterfaces([A, B])` diff --git a/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector.php b/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector.php new file mode 100644 index 00000000..ae02b8d9 --- /dev/null +++ b/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector.php @@ -0,0 +1,272 @@ +markTestSkipped($reason)` call at the + * top of each test method it applies to, and drops the attribute. + * + * PHPUnit has no declarative skip attribute, so the nearest faithful form is the call PHPUnit's own + * `markTestSkipped()` makes: the test is reported as skipped and its body never runs. The reason is + * forwarded as the message; a reason-less `#[Skip]` becomes a bare `$this->markTestSkipped()` + * (Testo's generated `{testId} is skipped via #[Skip]` message has no PHPUnit counterpart). + * + * Testo allows the attribute on a **class** too; there it is fanned out onto each test method + * (mirroring how Testo skips every test of the case) and removed from the class. A method carrying + * its own `#[Skip]` keeps its own reason — method-level wins over class-level as a whole, reason + * included. Test methods are found the same way as {@see RepeatRetryRector}: the `#[Test]`-marked + * methods (Testo's or the already-converted PHPUnit form), or — under a class-level + * `#[\Testo\Test]` — every public, non-static, `void`/`never`, non-lifecycle method. + * + * **Residual:** the skip becomes a runtime one. Testo keeps a `#[Skip]`-ed test out of the per-test + * pipeline entirely (no `#[BeforeTest]`, no data provider call, no retries); PHPUnit runs `setUp()` + * and the data provider, then aborts inside the test body. A `#[Skip]` on a free function or on a + * non-test member is left untouched — `$this->markTestSkipped()` needs a test method to live in, + * and on a non-test member the attribute is inert in Testo anyway. + */ +#[TestRectorFixtures('SkipAttributeToPhpUnitRector')] +final class SkipAttributeToPhpUnitRector extends AbstractRector +{ + private const SKIP_TESTO = 'Testo\\Skip'; + + /** @var list */ + private const LIFECYCLE_ATTRIBUTES = [ + 'Testo\\Lifecycle\\BeforeTest', + 'Testo\\Lifecycle\\AfterTest', + 'Testo\\Lifecycle\\BeforeClass', + 'Testo\\Lifecycle\\AfterClass', + 'PHPUnit\\Framework\\Attributes\\Before', + 'PHPUnit\\Framework\\Attributes\\After', + 'PHPUnit\\Framework\\Attributes\\BeforeClass', + 'PHPUnit\\Framework\\Attributes\\AfterClass', + ]; + + /** @var list */ + private const LIFECYCLE_NAMES = ['setup', 'teardown', 'setupbeforeclass', 'teardownafterclass']; + + public function getRuleDefinition(): RuleDefinition + { + return new RuleDefinition( + 'Convert the Testo #[Skip] attribute into a leading $this->markTestSkipped($reason) call, fanning a class-level attribute out onto each test method', + [ + new CodeSample( + <<<'PHP' + #[\Testo\Skip('broken by the pricing rework')] + public function test(): void + { + $this->doSomething(); + } + PHP, + <<<'PHP' + public function test(): void + { + $this->markTestSkipped('broken by the pricing rework'); + $this->doSomething(); + } + PHP, + ), + ], + ); + } + + #[\Override] + public function getNodeTypes(): array + { + return [Class_::class]; + } + + /** + * @param Class_ $node + */ + #[\Override] + public function refactor(Node $node): ?Node + { + $targets = $this->testMethods($node); + if ($targets === []) { + # Nothing that carries a skip here (not a test class); leave every attribute untouched. + return null; + } + + $classSkip = $this->classAttribute($node, self::SKIP_TESTO); + $changed = false; + + foreach ($targets as $method) { + $skip = $this->takeMethodAttribute($method, self::SKIP_TESTO) ?? $classSkip; + if ($skip === null || $method->stmts === null) { + continue; + } + + \array_unshift($method->stmts, new Expression( + new MethodCall(new Variable('this'), new Identifier('markTestSkipped'), $this->messageArgs($skip)), + )); + $changed = true; + } + + if ($classSkip !== null) { + $this->removeClassAttribute($node, self::SKIP_TESTO); + $changed = true; + } + + return $changed ? $node : null; + } + + /** + * Removing the attribute here is what keeps the emitted call the only trace of the skip. + */ + private function takeMethodAttribute(ClassMethod $method, string $name): ?Attribute + { + $found = null; + $kept = []; + foreach ($method->attrGroups as $attrGroup) { + $attrGroup->attrs = \array_values(\array_filter( + $attrGroup->attrs, + function (Attribute $attr) use ($name, &$found): bool { + if (!$this->isName($attr->name, $name)) { + return true; + } + + $found ??= $attr; + return false; + }, + )); + $attrGroup->attrs === [] or $kept[] = $attrGroup; + } + + $found === null or $method->attrGroups = $kept; + + return $found; + } + + private function classAttribute(Class_ $class, string $name): ?Attribute + { + foreach ($class->attrGroups as $attrGroup) { + foreach ($attrGroup->attrs as $attr) { + if ($this->isName($attr->name, $name)) { + return $attr; + } + } + } + + return null; + } + + private function removeClassAttribute(Class_ $class, string $name): void + { + $kept = []; + foreach ($class->attrGroups as $attrGroup) { + $attrGroup->attrs = \array_values(\array_filter( + $attrGroup->attrs, + fn(Attribute $attr): bool => !$this->isName($attr->name, $name), + )); + $attrGroup->attrs === [] or $kept[] = $attrGroup; + } + + $class->attrGroups = $kept; + } + + /** + * `markTestSkipped()` arguments from a `#[Skip]`: the reason as the sole positional message + * (the named `reason:` form loses its name — PHPUnit's parameter is `$message`), cloned so a + * class-level attribute can be fanned out onto several methods with independent nodes. A + * reason-less attribute yields no arguments. + * + * @return list + */ + private function messageArgs(Attribute $skip): array + { + foreach ($skip->args as $position => $arg) { + if (!$arg instanceof Arg) { + continue; + } + + if ($arg->name === null ? $position === 0 : $arg->name->toString() === 'reason') { + return [new Arg(clone $arg->value)]; + } + } + + return []; + } + + /** + * The methods a skip can apply to, mirroring Testo discovery. + * + * @return list + */ + private function testMethods(Class_ $class): array + { + $marked = []; + foreach ($class->getMethods() as $method) { + if ($this->methodHasKind($method, 'Testo\\Test', 'PHPUnit\\Framework\\Attributes\\Test')) { + $marked[] = $method; + } + } + if ($marked !== []) { + return $marked; + } + + if ($this->classAttribute($class, 'Testo\\Test') === null) { + return []; + } + + $discovered = []; + foreach ($class->getMethods() as $method) { + $this->isDiscoverableByClassLevelTest($method) and $discovered[] = $method; + } + + return $discovered; + } + + /** + * Mirrors Testo's locator for a class-level `#[Test]`. + */ + private function isDiscoverableByClassLevelTest(ClassMethod $method): bool + { + if (!$method->isPublic() || $method->isStatic() || $this->isLifecycleMethod($method)) { + return false; + } + + $returnType = $method->returnType; + + return $returnType instanceof Identifier && \in_array($returnType->toLowerString(), ['void', 'never'], true); + } + + private function isLifecycleMethod(ClassMethod $method): bool + { + if (\in_array(\strtolower((string) $this->getName($method)), self::LIFECYCLE_NAMES, true)) { + return true; + } + + return $this->methodHasKind($method, ...self::LIFECYCLE_ATTRIBUTES); + } + + private function methodHasKind(ClassMethod $method, string ...$names): bool + { + foreach ($method->attrGroups as $attrGroup) { + foreach ($attrGroup->attrs as $attr) { + foreach ($names as $name) { + if ($this->isName($attr->name, $name)) { + return true; + } + } + } + } + + return false; + } +} diff --git a/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/class_level_fans_out_to_methods.php.inc b/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/class_level_fans_out_to_methods.php.inc new file mode 100644 index 00000000..509a30e9 --- /dev/null +++ b/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/class_level_fans_out_to_methods.php.inc @@ -0,0 +1,31 @@ +markTestSkipped('the whole case waits for the new API'); + } + #[\Testo\Test] + public function testB(): void + { + $this->markTestSkipped('the whole case waits for the new API'); + } +} diff --git a/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/class_level_method_reason_wins.php.inc b/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/class_level_method_reason_wins.php.inc new file mode 100644 index 00000000..c601d877 --- /dev/null +++ b/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/class_level_method_reason_wins.php.inc @@ -0,0 +1,32 @@ +markTestSkipped('method reason'); + } + #[\Testo\Test] + public function testB(): void + { + $this->markTestSkipped('class reason'); + } +} diff --git a/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/class_level_test_discovery_fans_out.php.inc b/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/class_level_test_discovery_fans_out.php.inc new file mode 100644 index 00000000..f29ab779 --- /dev/null +++ b/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/class_level_test_discovery_fans_out.php.inc @@ -0,0 +1,31 @@ +markTestSkipped('case reason'); + } + + public function helper(): string + { + return 'x'; + } +} diff --git a/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/method_level_with_reason.php.inc b/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/method_level_with_reason.php.inc new file mode 100644 index 00000000..15c63875 --- /dev/null +++ b/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/method_level_with_reason.php.inc @@ -0,0 +1,23 @@ +doSomething(); + } +} +----- +markTestSkipped('broken by the pricing rework'); + $this->doSomething(); + } +} diff --git a/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/method_level_without_reason.php.inc b/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/method_level_without_reason.php.inc new file mode 100644 index 00000000..3e07efd6 --- /dev/null +++ b/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/method_level_without_reason.php.inc @@ -0,0 +1,21 @@ +markTestSkipped(); + } +} diff --git a/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/named_reason_argument.php.inc b/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/named_reason_argument.php.inc new file mode 100644 index 00000000..603ef479 --- /dev/null +++ b/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/named_reason_argument.php.inc @@ -0,0 +1,21 @@ +markTestSkipped('flaky on CI, see ISSUE-123'); + } +} diff --git a/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/non_test_method_untouched.php.inc b/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/non_test_method_untouched.php.inc new file mode 100644 index 00000000..3ad3d099 --- /dev/null +++ b/bridge/rector/src/TestoToPhpunit/SkipAttributeToPhpUnitRector/non_test_method_untouched.php.inc @@ -0,0 +1,14 @@ +markTestSkipped($reason)` call opening the test method, with the + attribute dropped. The reason becomes the message; a reason-less attribute emits a bare call (Testo's + generated `{testId} is skipped via #[Skip]` text has no counterpart). A class-level attribute is fanned + out onto each test method — the same method set `TestClassToTestCaseRector` uses — and removed from the + class; a method carrying its own `#[Skip]` keeps its own reason, mirroring Testo's method-wins rule. + **Residuals:** (1) the skip becomes a runtime one — PHPUnit runs `setUp()` and the data provider before + aborting inside the body, where Testo never lets the test enter the per-test pipeline at all; (2) a + `#[Skip]` on a free function or a non-test member is left untouched (no method to host the call, and in + Testo the attribute is inert there anyway). - **`TestClassToTestCaseRector`** (registered) — adds `extends \PHPUnit\Framework\TestCase` to a Testo test class and reconciles discovery. A class-level `#[\Testo\Test]` is removed and a per-method `#[\PHPUnit\Framework\Attributes\Test]` is added to every public, non-static method with a `void`/`never` diff --git a/skills/testo-migrate-from-phpunit/references/phpunit-to-testo-map.md b/skills/testo-migrate-from-phpunit/references/phpunit-to-testo-map.md index 976b7fad..6ca0781c 100644 --- a/skills/testo-migrate-from-phpunit/references/phpunit-to-testo-map.md +++ b/skills/testo-migrate-from-phpunit/references/phpunit-to-testo-map.md @@ -41,7 +41,7 @@ the assertion **argument order flips** (see the pitfalls), and discovery is attr | `$this->assertEmpty($a)` / `assertNotEmpty($a)` | `Assert::blank($a)` / `Assert::notBlank($a)` **only when `$a` is an array** — `blank()` treats `false`/`0`/`'0'` as valid data, so for other types port by hand. | | `$this->expectException(X::class)` before Act | `Expect::exception(X::class)->withMessage(...)->withCode(...)` before Act. Method return type becomes `never`. | | `$this->expectExceptionMessageMatches('/.../')` | `withMessageContaining('substring')` if a literal substring suffices; otherwise catch and assert manually. No PCRE. | -| `$this->markTestSkipped('reason')` | `throw new \Testo\Core\Exception\SkipTest('reason')` from the test body. | +| `$this->markTestSkipped('reason')` | `#[Skip('reason')]` from **`Testo\Skip`** when the call opens the test unconditionally — the test then never enters the pipeline (no hooks, no provider, no retries). A guarded call, one deeper in the body, or a non-literal message stays runtime: `throw new \Testo\Core\Exception\SkipTest('reason')` from the test body. | | `$this->markTestIncomplete('reason')` | No "incomplete" status. Port to `throw new SkipTest('TODO: reason')`, or leave the body empty → `Status::Risky`. | | `#[DoesNotPerformAssertions]` / `$this->expectNotToPerformAssertions()` | `#[ExpectNoAssertions]` from **`Testo\Assert`**, on a method or function (not a class) — no method-call form. Two-way contract: a marked test that *does* assert is `Status::Risky`. | | `$this->createMock(Foo::class)` | Testo core ships no mocking; the doubling library is `testo/bridge-double`. `$this->createMock`/`createStub` → `\JMac\Testing\Double::for(Foo::class)` and the `expects()->method()->willReturn()` chain → `expects('m')->times(1)->returns(...)`, with `with()` constraints mapped onto `Argument::*` (`anything`→`any`, `isInstanceOf`→`type`, `callback`→`satisfies`, …). The `CreateMockToDoubleRector` Rector rule does all this automatically (also `willReturnSelf` → `returns()`, `getMockBuilder(X)->disableOriginalConstructor()->getMock()` → `Double::for(X)`, and comparison/string/`logicalNot`/`logicalOr` constraints via `Argument::satisfies`/`not`/`any`); only `willReturnMap`, `prophesize`, a `getMockBuilder` step beyond `disableOriginalConstructor`, and `with()` constraints with no faithful form (`logicalAnd`, `equalToWithDelta`, case-insensitive `stringContains`) stay manual. Prefer keeping Mockery instead? Add `testo/bridge-mockery` — like the Double bridge, it verifies and isolates mocks after every test (drops the `tearDown()` / `MockeryPHPUnitIntegration` boilerplate) and counts a fulfilled expectation as an assertion, so a mock-only test stays out of `Status::Risky`. **Never** mock `final` classes or enums. | diff --git a/skills/testo-migrate-from-phpunit/references/subagent-port-prompt.md b/skills/testo-migrate-from-phpunit/references/subagent-port-prompt.md index e30025f8..d6abbb1a 100644 --- a/skills/testo-migrate-from-phpunit/references/subagent-port-prompt.md +++ b/skills/testo-migrate-from-phpunit/references/subagent-port-prompt.md @@ -72,7 +72,8 @@ Read the verdict straight from the JSON: - `@dataProvider`/`#[DataProvider]` → `#[DataProvider('m')]` (provider `public static`, returns `iterable`, labelled rows). `@testWith`/`#[TestWith]` → repeated `#[DataSet([...], 'label')]`. - `@group`/`#[Group]` → one variadic `#[Group(...)]` from `Testo\Filter\Group`. - - `@covers`/`#[CoversClass]` → `#[Covers(...)]`; `markTestSkipped` → `throw new SkipTest(...)`. + - `@covers`/`#[CoversClass]` → `#[Covers(...)]`; a `markTestSkipped` opening the test → `#[Skip(...)]` + from `Testo\Skip`, a guarded or non-literal one → `throw new SkipTest(...)`. - Mocks: replace with a hand-rolled fake (preferred) — never mock `final`/enums. If the file uses a kept mock library, leave it but make it run under Testo. If a fake is non-trivial and the orchestrator told you a shared fake exists, use it; do not invent a divergent copy.