Skip to content

feat(test): add #[Skip] attribute to skip tests declaratively PR-1. - #314

Open
Meacue wants to merge 36 commits into
php-testo:1.xfrom
Meacue:feat/skip-attribute
Open

feat(test): add #[Skip] attribute to skip tests declaratively PR-1.#314
Meacue wants to merge 36 commits into
php-testo:1.xfrom
Meacue:feat/skip-attribute

Conversation

@Meacue

@Meacue Meacue commented Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What was changed

#[Skip('reason')] on a test method, class, or function: the test is not executed and is reported as Status::Skipped with a composed reason ({testId} is skipped via #[Skip] ==> {reason}), so JUnit/TeamCity/HTML show the origin without any reporter changes.

Implementation follows the approach suggested in #313: a registered TestCaseRunInterceptor in plugin/test filters #[Skip] tests out of the case's test list before any lifecycle hooks and reports them back as skipped TestResults. Skipped tests never enter the per-test pipeline: #[BeforeTest]/#[AfterTest], data providers, #[Retry]/#[Repeat], coverage never engage. No new Status case, no changes under core/.

Why?

See #313: Scope: incremental plan - PR-1.

Semantics (v1)

  • Targets: method, class (including inherited from a parent/trait), function; a method-level reason wins over a class-level one.
  • #[BeforeClass]/#[AfterClass] still run, also when every test of the case is parked; full class suppression is a possible follow-up.
  • A run consisting only of #[Skip] tests is a success (exit 0).
  • reason is optional; the attribute is not repeatable.

Checklist

@Meacue
Meacue marked this pull request as ready for review September 2, 2026 13:51
@Meacue
Meacue requested a review from a team as a code owner September 2, 2026 13:51
roxblnfk
roxblnfk reviewed Sep 2, 2026
View reviewed changes
Comment thread plugin/test/src/Skip.php Outdated
Meacue added a commit to Meacue/testo that referenced this pull request Sep 2, 2026
@Meacue
fix(test): mark #[Skip] as Interceptable with SkipInterceptor as fall…
59ae61f 
…back

Review feedback on php-testo#314: the attribute now implements Interceptable and
declares #[FallbackInterceptor(SkipInterceptor::class)], mirroring Retry.
The interface alone would break class-level usage (an Interceptable without
a fallback alias makes the attributes interceptor throw at pipeline build),
so the pair goes together. The TestPlugin registration stays: the case-level
fallback path only reads class attributes, so a method-level #[Skip] still
needs the registered instance; the duplicate spawn for class-level cases is
collapsed by the sorter's conflict policy.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@Meacue
Meacue marked this pull request as draft September 2, 2026 16:55
@Meacue

Meacue commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Found some bugs, fixing them right now. Marked this PR as draft until they're fixed.

Meacue added a commit to Meacue/testo that referenced this pull request Sep 2, 2026
@Meacue
fix(lifecycle): locate function-case hooks from the case file, not su…
0c06301 
…rviving tests

LifecycleInterceptor::collectCaseFunctions() resolved a function-based
case's source file by scanning the case's remaining tests for the first
ReflectionFunction. Any outer case interceptor that prunes tests before
the lifecycle interceptor runs (it is the innermost, order PHP_INT_MAX)
could therefore empty the scan: with #[Skip] (PR php-testo#314)
parking every #[Test] function of a file, the path came back null and
the #[BeforeClass]/#[AfterClass] hooks were silently dropped, breaking
the documented Skip contract that "BeforeClass/AfterClass hooks still
run - also when every test of the case is skipped". Class-based cases
were unaffected (hooks come from CaseDefinition::$reflection).

The path now comes from CaseDefinition::$file, which every locator fills
from the tokenized file the case was discovered in, so hook discovery no
longer depends on who pruned the test set or when (the same hole existed
for a fully filtered-out case).

Motive check: the surviving-test scan (da29d6a, 2026-07-01) predates
CaseDefinition::$file (f17f4ed, 2026-08-02); it was the only way to get
the path at the time and was simply never migrated - no semantic reason
to keep it.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@Meacue
Meacue force-pushed the feat/skip-attribute branch from 0c06301 to 4664f13 Compare September 3, 2026 14:29
@Meacue Meacue mentioned this pull request Sep 3, 2026
Merged
2 tasks
roxblnfk pushed a commit to Meacue/testo that referenced this pull request Sep 4, 2026
@Meacue @roxblnfk
fix(test): mark #[Skip] as Interceptable with SkipInterceptor as fall…
15e8682 
…back

Review feedback on php-testo#314: the attribute now implements Interceptable and
declares #[FallbackInterceptor(SkipInterceptor::class)], mirroring Retry.
The interface alone would break class-level usage (an Interceptable without
a fallback alias makes the attributes interceptor throw at pipeline build),
so the pair goes together. The TestPlugin registration stays: the case-level
fallback path only reads class attributes, so a method-level #[Skip] still
needs the registered instance; the duplicate spawn for class-level cases is
collapsed by the sorter's conflict policy.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@roxblnfk
roxblnfk force-pushed the feat/skip-attribute branch from 4664f13 to 6059ac0 Compare September 4, 2026 09:50
@codecov

codecov Bot commented Sep 4, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.87755% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
plugin/test/src/TestPlugin.php 0.00% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

Meacue added 18 commits September 8, 2026 05:21
@Meacue
feat(test): add #[Skip] attribute to skip tests declaratively
be1fb8d 
A plain marker attribute (method, class, or free function) plus a registered
case-level TestCaseRunInterceptor: parked tests are filtered out of the case
before any lifecycle hooks and reported back as synthetic Skipped results with
a composed reason ("{testId} is skipped via #[Skip] ==> {reason}"), delivered
through a batch-runner wrapper so reporters render them inside the case block
with no reporter changes.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@Meacue
test(test): cover the #[Skip] v1 semantics with unit and feature tests
79109b6 
Unit: interceptor filtration before $next, reason composition, synthetic
result shape, origin attribute, batch-runner wrapping, pipeline events.
Feature: full v1 contract over a stub catalog - method/class/function targets,
inheritance from parent and trait, lifecycle hook contract, instantiation
rules, DataProvider/Retry/Repeat/fiber composition, summary arithmetic and
the success of an only-parked run. Plus one narrow JUnitWriter case pinning
that a Skipped result renders its failure message.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@Meacue
docs(skills): document #[Skip] in write-tests, plugin-author and flak…
dd86e2c 
…y-tests

write-tests gains a "Parking a test" section with the skip-tool comparison
table and the runtime contract; plugin-author points to SkipInterceptor as the
canonical shipped "return, do not throw" example; flaky-tests adds the parking
branch to the decision flow.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@Meacue
build: prepare root constraints for the testo/test 0.2.0 release
e0e2858 
feat bumps testo/test 0.1.6 -> 0.2.0 on release: widen the root require to
"0.1.6 - 1" so composer resolves both before and after the release, and move
the path-repository version to 0.2.x-dev per docs/spec/plugin-creation.md.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@Meacue
fix(test): mark #[Skip] as Interceptable with SkipInterceptor as fall…
708787c 
…back

Review feedback on php-testo#314: the attribute now implements Interceptable and
declares #[FallbackInterceptor(SkipInterceptor::class)], mirroring Retry.
The interface alone would break class-level usage (an Interceptable without
a fallback alias makes the attributes interceptor throw at pipeline build),
so the pair goes together. The TestPlugin registration stays: the case-level
fallback path only reads class attributes, so a method-level #[Skip] still
needs the registered instance; the duplicate spawn for class-level cases is
collapsed by the sorter's conflict policy.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@Meacue
refactor(test): declare SkipInterceptor conflict policy explicitly
62a12da 
Class-level #[Skip] spawns a duplicate interceptor instance through the
fallback alias; ConflictPolicy::First (previously implicit) collapses it
onto the instance registered by TestPlugin. Spell the policy out and
explain why, mirroring DataProviderInterceptor.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@Meacue
docs(test): correct the fallback coverage claim in the Skip docblock
e766061 
The fallback interceptor is spawned from class attributes only
(AttributesInterceptor::runTestCase), so without TestPlugin it rescues
a class-level #[Skip] alone; a method- or function-level #[Skip] is
inert in that setup. The previous wording implied full standalone
coverage.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@Meacue
docs(test): explain why SkipInterceptor takes no Skip constructor par…
29eb900 
…ameter

The instance is built both by the container (TestPlugin registration,
where an attribute cannot be resolved) and by the injector on a fallback
spawn (where the attribute is passed and ignored); attributes are looked
up per case in findParked(). A Skip parameter would break the container
path at pipeline construction.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@Meacue
docs(test): document that #[Skip] is inert on bench and inline cases
93f8580 
test(test): pin the declared testType of SkipInterceptor

SkipInterceptor declares testType: TestType::Test, so the type filter
drops it for #[Bench] and #[TestInline] cases and the attribute has no
effect there. Add the point to the Skip runtime contract and the
write-tests skill, and pin the declaration with a unit test.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@Meacue
fix(test): carry the test description into the synthetic Skipped result
9f44b4a 
TerminalLogger reads the description from the result attributes, where
the regular test path stamps it; the synthetic result built for a parked
test lacked it, so the PHPDoc description was not rendered. Stamp
TestDefinition::getDescription() the way TestRunner does.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@Meacue
test(test): cover class-level #[Skip] standalone fallback without Tes…
9cc7603 
…tPlugin

Every existing feature test runs through suite defaults where TestPlugin
is registered, so the FallbackInterceptor declaration itself was never
exercised. Run a convention-discovered catalog with
SuitePlugins::without(TestPlugin::class) and assert the class-level
#[Skip] still parks every test, with exactly one result per test.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@Meacue
test(test): pin single delivery of class-level #[Skip] with both path…
83a8237 
…s live

The dedup invariant (registered interceptor + fallback spawn collapse
into one delivery) was held only by the arithmetic of the summary tests.
Name it: a full-application run of a class-level parked catalog yields
exactly one result per test in its CaseResult.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@Meacue
test(lifecycle): pin class-level hooks for fully parked cases end-to-end
ce84744 
Integration proof of the #[Skip] contract documented on the attribute:
BeforeClass/AfterClass hooks still run when every test of the case is parked.
Drives the real pipeline over a function-based and a class-based catalog and
checks the hooks fire exactly once while the per-test hooks stay silent.

Complements the Skip-independent unit regression test that landed with the
lifecycle fix: that one pins hook discovery on a pruned case in isolation,
this one pins the contract with the actual #[Skip] interceptor doing the
pruning.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@Meacue
test(test): drop a cannot-fail assertion and pin the enabled function…
9c98bab 
… neighbor

Audit follow-up. namedReasonArgument asserted a PHP language guarantee (named
argument binding to the single promoted parameter) and could not fail while its
positional twin passes, so it is removed. enabled_function in skip_functions.php
was asserted by nothing: TestRunner::runTest() returns only the requested
result, so a throwing neighbor would go unnoticed. The new feature test runs it
and expects Passed, closing the one untested cell of the semantics matrix: an
enabled function of a partially parked file survives the wrapped batch runner.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@Meacue
fix(test): load the function stub in SkipFeatureTest regardless of ru…
806ab59 
…n order

The two function-based tests resolved their targets only because an earlier
test's inner catalog run happened to include skip_functions.php: functions are
not autoloadable, and TestRunner::runTest() throws "Invalid test function
provided." when the name does not exist yet. Running either test in isolation
(--filter) errored before any assertion. Load the stub in the constructor, the
same way the other function-based feature tests already do.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
@Meacue
docs(test): fix #[Skip] docs and strengthen its tests
9f4f75c 
The docs no longer promise the skip reason in every report, and the construction, fiber and provider checks now assert per-run evidence.

Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
@Meacue
docs(test): clarify skip reporter visibility
26dd926 
Assisted-By: GPT-5.6 Terra
@Meacue
fix(test): deactivate #[Skip] tests instead of undefining them
7764b46 
php-testo#318 removed TestDefinitions::undefine() and gave TestDefinition an
`active` flag. The interceptor now flips that flag the way the filter
plugin does, walks the case's active tests only, and the docblocks
describe the mechanic that is left instead of the one that is gone.
Three unit tests pin deactivation-not-discard, an inert #[Skip] on a
non-test member, and no Skipped report for an already-filtered test;
a fourth pins the declared order and conflict policy.

Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
Meacue added 13 commits September 8, 2026 06:22
@Meacue
style(test): order static methods first in the #[Skip] sources
19b6580 
The repository php-cs-fixer config puts static methods before instance
methods; SkipInterceptor::reason() and the two static data providers in
the stubs sat at the bottom of their classes.

Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
@Meacue
docs(test): say what each #[Skip] stub proves and tighten the attribu…
6182310 
…te docs

Every stub in Stub/Skip and Stub/FullyParked opens with the behaviour it
pins instead of its mechanics, and the sentences that described a
pre-php-testo#318 contrast (hooks resolved from the class reflection, tests
removed from the case) now match the deactivation the code performs.
Skip::class drops the "(v1)" tag from its runtime contract and states
the TestPlugin prerequisite as a fact; TestPlugin and the plugin README
mention #[Skip] next to #[Test].

Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
@Meacue
test(test): pin the #[Skip] fallback message and align the stubs with…
967e2c6 
… the fixtures

test(output): pin that a reason-less skip writes no JUnit message

The standalone fallback test asserts the SkipTest failure and its full
message instead of the status alone; the feature suite carries a
class-level #[Group('async')] because every method replays the fiber
stub; SkipWithHooksStub counters take the Calls suffix of their
lifecycle twin; the data-provider, repeat and retry stubs mark the
class with #[Test] like the rest of the directory. SkipSummaryTest
pins the totals as literals, SkipAttributeTest uses Assert::instanceOf
and drops the attribute-repeatability test, and the unit fixtures keep
empty bodies since nothing ever executes them. JUnitWriterTest gains
the negative twin of the skip-reason test.

Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
@Meacue
docs(skills): describe #[Skip] deactivation and its package in the sk…
fca9911 
…ills

testo-plugin-author gets a subsection on skipping from a case-level
interceptor with the post-php-testo#318 mechanic (TestDefinition::$active,
getTests() yielding only active tests, synthetic results through
CaseInfo::withBatchRunner); testo-write-tests names the testo/test
package and the TestPlugin prerequisite, prescribes a reason that
points at an issue and says how a skipped test is reported;
testo-flaky-tests gains the #[Skip] x #[Retry]/#[Repeat] pitfall and
delegates the reporter mechanics to testo-write-tests.

Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
@Meacue
test(test): use the typed Assert facade in the #[Skip] tests
153e46f 
Assert::array()->hasCount() replaces the is_array()/count() pair on the
origin attribute, and the pipeline-entry check reads as a chain of
contains()/notContains() instead of an array_intersect() against [].
Applied to the Feature and the Unit test together so the two layers
keep the same idiom.

Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
@Meacue
test(test): pin that an overriding method inherits the #[Skip] of its…
9324f84 
… prototype

docs(test): document method-level #[Skip] inheritance

SkipInterceptor reads the attribute through Reflection::fetchFunctionAttributes
with prototypes included, the way #[Group] is read, so a child method that
overrides a #[Skip]-marked one is skipped with the parent's reason. A parent/
child stub pair and a feature test pin it; the Skip docblock and the
testo-write-tests skill say so.

Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
@Meacue
test(test): add positive controls for the #[Retry]/#[Repeat] skip checks
d755af1 
The negative assertions on the skipped tests would also hold if retry or
repeat never engaged in the testing suite at all. Each stub gains an
enabled neighbor with the same attribute: the retry one fails its first
attempt and passes the second (two attempts per run), the repeat one
counts its three runs.

Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
@Meacue
refactor(test): say "skipped" instead of "parked" across the #[Skip] …
9d504d6 
…code and tests

The attribute reports a test as Skipped, so the code, the stubs, the
tests and the skills now use that word too: findSkipped(), the
skipped()/skippedNoReason() stub methods, the skippedFunction/
enabledFunction pair, the FullySkipped and OnlySkipped stub directories,
StandaloneSkippedTest, MixedStub, the reason strings and every docblock.
Pure rename: no behaviour, structure or assertion changed. "parked" stays
where it means a suspended coroutine (testo/fiber, codecov, revolt).

Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
@Meacue
style(test): use a # line comment in SkipWithHooksStub
2f16f36 
Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
@Meacue
test(test): pin what the #[Skip] docblocks promise
0bcca01 
The pass-through unit test now asserts the test set it claims stays
untouched, the attribute test guards the #[\Attribute] lookup before
indexing it, and the pipeline-entry check pins the enabled function
neighbor next to the enabled method. The standalone stub's reason says
"case" instead of "catalog", together with the test that pins it.

Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
@Meacue
docs(test): say "directory" and "case" instead of "catalog" in the #[…
aa048ea 
…Skip] docs

docs(skills): complete the skip-from-a-case-interceptor recipe

The last "catalog" calques give way to the glossary words; the stub
docblocks name the feature test that reads each counter or latch, the
trait marker and the mixed data provider get their one-line docblocks,
and SkipInterceptor states why bench and inline cases are out of its
reach. testo-plugin-author says what to do when the case carries no
batch runner; testo-flaky-tests and testo-write-tests phrase the
"when to skip" entries as conditions.

Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
@Meacue
build: revert the hand-edited testo/test root constraints
4a454e8 
Reverts e0e2858. Both fields are regenerated by
.github/release-please/sync-deps.php from resources/version.json on
release: split packages get a caret constraint (the "x - 1" form is
reserved for the testo/testo meta-package) and the path-repo alias
follows the manifest, which still reads 0.1.7. With
bump-patch-for-minor-pre-major the #[Skip] feat releases as 0.1.8, not
0.2.0, so the 0.2.x-dev alias would never match.

Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
@Meacue
test(test): make the #[Retry] control in SkipWithRetryStub history-proof
58bd873 
The enabled neighbor failed its first attempt by the parity of the
static attempt counter, which holds only while every run adds exactly
two attempts. A run narrowed to that method by --filter, or one
aborted between the attempts, would flip the parity for good and break
the delta assertion in SkipFeatureTest ever after.

Track "first attempt failed" on the case instance instead: it is built
anew for every run of the case and shared by the retry attempts within
it, so the marker starts fresh each run. The cumulative static stays
for the delta assertion.

Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
@Meacue
Meacue force-pushed the feat/skip-attribute branch from 6379b2a to 58bd873 Compare September 8, 2026 07:00
@Meacue
Meacue marked this pull request as ready for review September 8, 2026 07:01
@Meacue

Meacue commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@roxblnfk sorry for the pause. PR is ready for review, rebased onto the current 1.x.

@roxblnfk
Merge branch '1.x' into feat/skip-attribute
2c16e71
@roxblnfk
feat(core): add CaseInterceptable for test-level attributes on the …
2d95649 
…case pipeline

The case pipeline is built from class attributes alone, so an attribute that has to act on the whole case from a single test (take it out before the class-level hooks) could only be wired by a plugin. Scanning every `Interceptable` on the tests would make a method-level `#[RunInFiber]` install a fiber batch runner for the entire case, hence an explicit opt-in interface. Test-level attributes are not stamped on `CaseInfo::$attributes`.

Assisted-By: Claude Fable 5.1
@roxblnfk
roxblnfk force-pushed the feat/skip-attribute branch from 04d7d87 to bda23e9 Compare September 8, 2026 12:01
@roxblnfk
feat(skip): move #[Skip] into its own testo/skip plugin
6421baa 
`#[Skip]` is a one-attribute plugin of the same shape as `testo/retry` and `testo/repeat`, and the plugin naming rule puts the top-level class `\Testo\Skip` in a package of the same short name. The manifest starts at 0.0.0 so the first release is 0.1.0.

Assisted-By: Claude Fable 5.1
@roxblnfk
docs(skip): drop the TestPlugin wiring and the issue reference from…
ff59c73 
… the `#[Skip]` docblocks

Assisted-By: Claude Fable 5.1
@roxblnfk
feat(skip): wire #[Skip] through CaseInterceptable instead of `Te…
b9fd5d6 
…stPlugin`

docs(skills): point `#[Skip]` guidance at the self-wiring attribute and document `CaseInterceptable`

The attribute is wired by its `#[FallbackInterceptor]` alone: as a `CaseInterceptable` it reaches the case pipeline from a method or function too, so no plugin registers anything and the attribute works in any suite. `ConflictPolicy::First` now collapses the instances spawned per `#[Skip]` occurrence.

Assisted-By: Claude Fable 5.1
@roxblnfk
roxblnfk force-pushed the feat/skip-attribute branch from bda23e9 to b9fd5d6 Compare September 8, 2026 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@roxblnfk roxblnfk roxblnfk left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Meacue @roxblnfk