Add test-state cleanup registrars#415
Conversation
Document the agreed design for extending after-each test cleanup to apps and packages. The plan records why cleanup registration happens from the PHPUnit extension rather than service providers, how package metadata is discovered without writing the package manifest, and how app skeletons should expose a generated Tests\Support\TestState entry point. It also captures the testing strategy, self-review checklist, and the final decisions from the Codesonic design review so future work can understand the intended shape of the API.
Move Composer installed package scanning out of PackageManifest::build() into a reusable no-write helper. The new discoverInstalledPackages() method preserves the existing build semantics while making package metadata available to PHPUnit extension bootstrap, where writing bootstrap/cache is not appropriate. The extraction keeps build() using the virtual packagesToIgnore() seam so testbench can continue to override root composer ignores. The existing format() method remains available and delegates to a static formatter for callers that still depend on the instance method. The helper also centralizes metadata parsing robustness for missing or malformed installed.json files, non-array dont-discover metadata, and wildcard package discovery disables.
Add regression coverage for PackageManifest::discoverInstalledPackages() so the extracted helper stays byte-compatible with the manifest that build() writes for the existing fixtures. The tests cover Composer 2 and legacy installed.json shapes, version-only packages, wildcard dont-discover handling, missing installed metadata, malformed installed metadata, and malformed root composer metadata. This locks in the parsing behavior needed by PHPUnit bootstrap discovery without requiring the helper to write package cache files.
Add a fixture-level root dont-discover entry and assert testbench package manifest builds still ignore that project-level metadata during build. This protects the virtual packagesToIgnore() seam used by testbench: the foundation manifest can read root composer ignores, while testbench remains free to manage discovery filtering at read time through its own ignore list and required-package behavior.
Introduce AfterEachTestCleanup as the process-local registry for app and package cleanup callbacks that should run after every PHPUnit test. Callbacks are keyed by name so registrations are idempotent and duplicate names are last-wins. runCallbacks() continues through all registered callbacks before rethrowing the first exception, preventing one broken cleanup from suppressing the rest. The registry intentionally has no flushState() method because its callbacks are suite-level registrations that must survive for the PHPUnit worker lifetime. Tests cover registration order, persistence, replacement, clearing, root-after-package ordering, and exception isolation.
Refactor AfterEachTestSubscriber so notify() delegates to flushStateAfterTest(), which runs app and package callbacks before the framework-owned cleanup list. The framework cleanup body now lives in flushFrameworkState(), and it still runs from a finally block so framework state is flushed even when a custom cleanup callback throws. The existing first-party optional package cleanup methods remain grouped at the bottom of the framework cleanup list.
Add focused tests for AfterEachTestSubscriber::flushStateAfterTest() using an anonymous subclass that observes framework cleanup without tearing down the live test process. The tests prove custom callbacks run before framework cleanup and that framework cleanup still runs when a custom callback throws, while preserving the original callback exception.
Discover app and package test-state registrars during PHPUnit extension bootstrap, before the after-each subscriber is registered. Package registrars are read from installed package metadata through the no-write PackageManifest helper, while root registrars are read from root composer.json and appended last. This makes cleanup available even in workers that only execute app-less unit tests and never boot a Hypervel application. The testing package now declares its direct filesystem dependency because registrar discovery reads Composer metadata directly.
Add coverage for package and root test-state registrar discovery, package-before-root callback ordering, root and package dont-discover behavior, duplicate callback names, and COMPOSER_VENDOR_DIR handling. The test matrix also locks in the fail-fast boundary for declared bad metadata: non-string registrar values, missing classes, and classes without register() throw attributed RuntimeExceptions, while missing or malformed composer metadata degrades to no custom cleanup.
Add the Test State Cleanup section to the testing guide and explain how applications should use tests/Support/TestState as one app-level cleanup entry point. The macro guidance now reflects that Hypervel already flushes framework macroable classes after each test, so apps should only register cleanup for their own worker-lifetime state. The docs also warn that forgetCallbacks() clears discovered app and package registrations for the current PHPUnit worker.
Document the extra.hypervel.test-state composer metadata for package authors. The new package docs explain that registrar classes must be normal package autoload classes, should aggregate cleanup for the package's stateful classes, and are discovered during PHPUnit extension bootstrap so cleanup runs even in workers that never boot an application.
Clarify where different kinds of test cleanup belong after introducing app and package test-state registrars. Framework-owned classes still belong in AfterEachTestSubscriber, first-party optional package cleanup remains grouped at the bottom of that subscriber, and third-party packages, private packages, and applications should use extra.hypervel.test-state registrars instead. The guidance also corrects the stale subscriber path and explicitly warns not to add AfterEachTestCleanup itself to the subscriber, because its callbacks must persist for the PHPUnit worker lifetime.
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds an extensible after-each PHPUnit test cleanup mechanism, Composer-metadata-based registrar discovery, PackageManifest discovery helpers, bootstrap wiring, documentation updates, and tests. ChangesTest State Cleanup Feature
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds a first-class after-each test cleanup path for Hypervel apps and packages. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "fix(foundation): guard root hypervel met..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR adds a first-class mechanism for Hypervel applications and packages to register after-each-test cleanup (worker-lifetime/static state) that runs in every PHPUnit/Paratest worker, even when no application is booted. It keeps framework-owned cleanup centralized in the existing PHPUnit subscriber while layering an app/package registry and Composer-metadata discovery on top.
Changes:
- Introduce
AfterEachTestCleanup(named callback registry) andTestStateRegistrars(Composerextra.hypervel.test-statediscovery at PHPUnit extension bootstrap). - Run app/package callbacks before framework cleanup, while ensuring framework cleanup always runs via
finally. - Extract no-write installed-package discovery from
PackageManifestso testing bootstrap can read Composer metadata without writingbootstrap/cache, and document the new API.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Testing/PHPUnit/TestStateRegistrarsTest.php | Coverage for registrar discovery order, dont-discover behavior, failure modes, and env vendor dir override. |
| tests/Testing/PHPUnit/AfterEachTestSubscriberTest.php | Verifies custom callbacks run before framework cleanup and framework cleanup still runs on exceptions. |
| tests/Testing/PHPUnit/AfterEachTestCleanupTest.php | Verifies callback naming, ordering, replacement, persistence, and exception behavior. |
| tests/Testbench/Foundation/PackageManifestTest.php | Ensures testbench build does not apply root composer dont-discover during manifest build. |
| tests/Testbench/Foundation/Fixtures/PackageManifest/composer.json | Fixture update to include root-level dont-discover for the new test. |
| tests/Foundation/FoundationPackageManifestTest.php | Adds coverage for discoverInstalledPackages() behavior and resilience for missing/malformed metadata. |
| src/testing/src/PHPUnit/TestStateRegistrars.php | Discovers and invokes app/package test-state registrars from Composer metadata. |
| src/testing/src/PHPUnit/AfterEachTestSubscriber.php | Refactors cleanup flow to run custom callbacks first, then framework cleanup in a finally. |
| src/testing/src/PHPUnit/AfterEachTestExtension.php | Bootstraps registrar discovery before registering the global after-each subscriber. |
| src/testing/src/PHPUnit/AfterEachTestCleanup.php | New process-local registry for after-each cleanup callbacks (named, worker-lifetime). |
| src/testing/composer.json | Adds an explicit dependency needed by new discovery/cleanup code paths. |
| src/foundation/src/PackageManifest.php | Extracts installed-package discovery into a pure helper and reuses it for build(). |
| src/boost/docs/testing.md | Documents app-level test-state cleanup and clarifies macro cleanup guidance. |
| src/boost/docs/packages.md | Documents package-level extra.hypervel.test-state registrars and usage pattern. |
| docs/plans/2026-07-04-testing-after-each-cleanup-registrars.md | Design plan and rationale for the architecture and behavior. |
| AGENTS.md | Updates contributor guidance to prefer extra.hypervel.test-state for non-framework cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/testing/src/PHPUnit/AfterEachTestCleanup.php (1)
42-58: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winLater callback exceptions are silently discarded.
Only the first
Throwableis retained ($exception ??= $throwable); any exceptions thrown by subsequent callbacks are swallowed with no trace at all. With multiple apps/packages registering cleanup, a second (or third) package's cleanup failure could go completely unnoticed while only the first failure is ever surfaced.Consider at least logging the discarded exceptions (e.g., via
trigger_error()or PHP's error log) so later failures aren't fully invisible.♻️ Suggested improvement
public static function runCallbacks(): void { /** `@var` null|Throwable $exception */ $exception = null; foreach (static::$callbacks as $callback) { try { $callback(); } catch (Throwable $throwable) { - $exception ??= $throwable; + if ($exception === null) { + $exception = $throwable; + } else { + trigger_error((string) $throwable, E_USER_WARNING); + } } } if ($exception !== null) { throw $exception; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/testing/src/PHPUnit/AfterEachTestCleanup.php` around lines 42 - 58, The runCallbacks() method in AfterEachTestCleanup only preserves the first Throwable and silently drops later cleanup failures. Update the callback loop so any additional exceptions are at least reported when caught, while still rethrowing the first stored exception at the end; use the existing runCallbacks() and $exception handling to locate the change.src/testing/composer.json (1)
31-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare
composer-runtime-apisinceTestStateRegistrarsusesComposer\InstalledVersions.
TestStateRegistrars::installedRootPath()callsComposer\InstalledVersions::getRootPackage(), but thisrequireblock only addshypervel/filesystem. Composer's own docs state: "If your code relies on any of these runtime features, you should require "composer-runtime-api": "^2.0" in your composer.json." Without it, nothing guarantees the class is available to consumers with unusual autoload setups, andcomposer validate/dependency tooling won't reflect the real contract.♻️ Proposed fix
"require": { "php": "^8.4", "ext-dom": "*", + "composer-runtime-api": "^2.0", "mockery/mockery": "1.6.x-dev",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/testing/composer.json` around lines 31 - 53, Add an explicit composer runtime dependency in the testing composer manifest because TestStateRegistrars::installedRootPath() depends on Composer\InstalledVersions. Update the require block in composer.json to declare composer-runtime-api with a compatible 2.x constraint, alongside the existing hypervel and symfony packages, so the contract matches the runtime usage and dependency tooling reflects it.src/testing/src/PHPUnit/TestStateRegistrars.php (1)
80-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate composer.json-reading logic with
PackageManifest::packagesToIgnoreFromComposer.
rootRegistrars()re-implements the same isFile/json_decode/is_array guard sequence already extracted intoPackageManifest::packagesToIgnoreFromComposer(just readingtest-stateinstead ofdont-discover). Consider factoring a shared "read root composer extra key" helper (e.g. onPackageManifest) so malformed-JSON handling stays in one place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/testing/src/PHPUnit/TestStateRegistrars.php` around lines 80 - 95, The `rootRegistrars()` method is duplicating the composer.json read/validate flow already handled in `PackageManifest::packagesToIgnoreFromComposer`. Refactor the shared isFile/json_decode/is_array logic into a reusable helper on `PackageManifest` (or similar) that reads a requested `extra.hypervel` key, then update `rootRegistrars()` to use that helper for `test-state` so malformed JSON handling lives in one place.tests/Testing/PHPUnit/TestStateRegistrarsTest.php (1)
276-338: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove generic test-support classes into a test-specific namespace.
TestStateRegistrarRecorder,PackageTestStateRegistrar,RootTestStateRegistrar,RegistrarWithoutRegisterMethod,TestStateRegistrarsProbe, etc. are declared directly in the sharedHypervel\Tests\Testing\PHPUnitnamespace. Any other test file in this directory declaring a similarly-named helper (e.g. another "Recorder" or "Probe" class) will collide with these.Based on path instructions: "When helper classes in tests have collision-prone generic names, use a test-specific namespace that ends with the test class name."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Testing/PHPUnit/TestStateRegistrarsTest.php` around lines 276 - 338, Move the shared helper classes out of the generic Hypervel\Tests\Testing\PHPUnit namespace into a test-specific namespace tied to TestStateRegistrarsTest so they won’t collide with similarly named helpers in other test files. Update the declarations for TestStateRegistrarRecorder, PackageTestStateRegistrar, ReplacementPackageTestStateRegistrar, RootTestStateRegistrar, RegistrarWithoutRegisterMethod, and TestStateRegistrarsProbe together, and keep their usage within TestStateRegistrarsTest aligned with the new namespace.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/boost/docs/testing.md`:
- Around line 176-199: The example still points to a standalone test support
class under tests/Support, which conflicts with the repo convention that such
files live in a capitalized Fixtures directory. Update the example so the
TestState class and its namespace reflect the Fixtures location, and keep the
register and flushState references aligned with that renamed support path. Make
sure any referenced stateful helpers like InvoiceNumbers, TaxRates, and
ReceiptMacros are shown as part of the Fixtures-based test support structure.
---
Nitpick comments:
In `@src/testing/composer.json`:
- Around line 31-53: Add an explicit composer runtime dependency in the testing
composer manifest because TestStateRegistrars::installedRootPath() depends on
Composer\InstalledVersions. Update the require block in composer.json to declare
composer-runtime-api with a compatible 2.x constraint, alongside the existing
hypervel and symfony packages, so the contract matches the runtime usage and
dependency tooling reflects it.
In `@src/testing/src/PHPUnit/AfterEachTestCleanup.php`:
- Around line 42-58: The runCallbacks() method in AfterEachTestCleanup only
preserves the first Throwable and silently drops later cleanup failures. Update
the callback loop so any additional exceptions are at least reported when
caught, while still rethrowing the first stored exception at the end; use the
existing runCallbacks() and $exception handling to locate the change.
In `@src/testing/src/PHPUnit/TestStateRegistrars.php`:
- Around line 80-95: The `rootRegistrars()` method is duplicating the
composer.json read/validate flow already handled in
`PackageManifest::packagesToIgnoreFromComposer`. Refactor the shared
isFile/json_decode/is_array logic into a reusable helper on `PackageManifest`
(or similar) that reads a requested `extra.hypervel` key, then update
`rootRegistrars()` to use that helper for `test-state` so malformed JSON
handling lives in one place.
In `@tests/Testing/PHPUnit/TestStateRegistrarsTest.php`:
- Around line 276-338: Move the shared helper classes out of the generic
Hypervel\Tests\Testing\PHPUnit namespace into a test-specific namespace tied to
TestStateRegistrarsTest so they won’t collide with similarly named helpers in
other test files. Update the declarations for TestStateRegistrarRecorder,
PackageTestStateRegistrar, ReplacementPackageTestStateRegistrar,
RootTestStateRegistrar, RegistrarWithoutRegisterMethod, and
TestStateRegistrarsProbe together, and keep their usage within
TestStateRegistrarsTest aligned with the new namespace.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 62f57a96-ec58-41b6-8439-0dc492385f62
📒 Files selected for processing (16)
AGENTS.mddocs/plans/2026-07-04-testing-after-each-cleanup-registrars.mdsrc/boost/docs/packages.mdsrc/boost/docs/testing.mdsrc/foundation/src/PackageManifest.phpsrc/testing/composer.jsonsrc/testing/src/PHPUnit/AfterEachTestCleanup.phpsrc/testing/src/PHPUnit/AfterEachTestExtension.phpsrc/testing/src/PHPUnit/AfterEachTestSubscriber.phpsrc/testing/src/PHPUnit/TestStateRegistrars.phptests/Foundation/FoundationPackageManifestTest.phptests/Testbench/Foundation/Fixtures/PackageManifest/composer.jsontests/Testbench/Foundation/PackageManifestTest.phptests/Testing/PHPUnit/AfterEachTestCleanupTest.phptests/Testing/PHPUnit/AfterEachTestSubscriberTest.phptests/Testing/PHPUnit/TestStateRegistrarsTest.php
Handle malformed Composer installed package metadata without crashing package discovery during PHPUnit extension bootstrap. Invalid package lists now degrade to an empty manifest, malformed package entries are skipped before mapping, and non-array extra.hypervel metadata is treated as empty package metadata while preserving package versions. Centralize root extra.hypervel reads through PackageManifest::rootHypervelExtra so root dont-discover and test-state metadata share the same composer.json parsing behavior. Make TestStateRegistrars fail fast when Composer runtime metadata does not provide a valid root install path instead of guessing with getcwd. The root path validation is separated from the Composer lookup and covered through the test probe, including invalid values and realpath normalization. Declare composer-runtime-api ^2.2 for split packages that use Composer\InstalledVersions directly: foundation, testbench, and testing. Permission remains unchanged because its usage is optional and guarded. Document the first-exception-wins cleanup callback behavior and add focused tests for the malformed metadata and root path validation cases.
|
@greptileai @coderabbitai thanks for the review. Follow-up commit d2cefa9 addresses the actionable issues. Fixed:
Intentionally unchanged:
Validation: |
|
The rationale for the intentionally-unchanged items (skeleton docs matching generated app wiring, first-exception-wins over Thanks for the thorough validation run as well. I'll take another pass over the diff to confirm everything lines up. (=^･ω･^=) ✏️ Learnings added
✅ Action performedReview finished.
|
|
Tip For best results, initiate chat on the files or code changes.
(=^･ω･^=) |
|
The follow-up commit looks solid. Here's a quick verification of each fix: Malformed metadata hardening — Fail-fast root path —
Test coverage — The new No remaining concerns. This is ready. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/Testing/PHPUnit/TestStateRegistrarsTest.php (1)
211-225: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a
#[DataProvider]instead of a manual loop.Functionally correct, but a failed assertion inside the loop won't clearly indicate which of
null/''/123triggered it. A#[DataProvider]gives per-case isolation and clearer failure output, consistent with using PHPUnit attributes for parameterized cases.♻️ Proposed refactor using a data provider
- public function testResolveInstalledRootPathThrowsForInvalidComposerRootInstallPath(): void - { - foreach ([null, '', 123] as $installPath) { - try { - TestStateRegistrarsProbe::resolveInstalledRootPath($installPath); - - $this->fail('Expected a runtime exception for invalid Composer root install path.'); - } catch (RuntimeException $exception) { - $this->assertSame( - 'Composer runtime metadata is missing the root package install path.', - $exception->getMessage() - ); - } - } - } + #[DataProvider('invalidInstallPaths')] + public function testResolveInstalledRootPathThrowsForInvalidComposerRootInstallPath(mixed $installPath): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Composer runtime metadata is missing the root package install path.'); + + TestStateRegistrarsProbe::resolveInstalledRootPath($installPath); + } + + public static function invalidInstallPaths(): array + { + return [ + 'null' => [null], + 'empty string' => [''], + 'non-string' => [123], + ]; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Testing/PHPUnit/TestStateRegistrarsTest.php` around lines 211 - 225, Refactor the `testResolveInstalledRootPathThrowsForInvalidComposerRootInstallPath` test to use a PHPUnit `#[DataProvider]` instead of the manual `foreach` loop. Move the invalid inputs (`null`, `''`, `123`) into a dedicated provider method and have the test method accept the install path as a parameter, keeping the same `TestStateRegistrarsProbe::resolveInstalledRootPath` and `RuntimeException` assertions so each case runs independently with clearer failure output.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/Testing/PHPUnit/TestStateRegistrarsTest.php`:
- Around line 211-225: Refactor the
`testResolveInstalledRootPathThrowsForInvalidComposerRootInstallPath` test to
use a PHPUnit `#[DataProvider]` instead of the manual `foreach` loop. Move the
invalid inputs (`null`, `''`, `123`) into a dedicated provider method and have
the test method accept the install path as a parameter, keeping the same
`TestStateRegistrarsProbe::resolveInstalledRootPath` and `RuntimeException`
assertions so each case runs independently with clearer failure output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d2683a8a-4947-4540-a4b1-bb95b6ca14c6
📒 Files selected for processing (8)
src/foundation/composer.jsonsrc/foundation/src/PackageManifest.phpsrc/testbench/composer.jsonsrc/testing/composer.jsonsrc/testing/src/PHPUnit/AfterEachTestCleanup.phpsrc/testing/src/PHPUnit/TestStateRegistrars.phptests/Foundation/FoundationPackageManifestTest.phptests/Testing/PHPUnit/TestStateRegistrarsTest.php
✅ Files skipped from review due to trivial changes (2)
- src/foundation/composer.json
- src/testbench/composer.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/testing/src/PHPUnit/AfterEachTestCleanup.php
- src/foundation/src/PackageManifest.php
Replace the manual invalid-install-path loop in TestStateRegistrarsTest with a PHPUnit DataProvider so each malformed Composer root install path is reported as its own dataset. This keeps the same guard coverage for null, empty string, and non-string install_path values while matching the repository preference for PHPUnit attributes over inline parameter loops. Validation run before commit: - ./vendor/bin/phpunit --no-progress tests/Testing/PHPUnit/TestStateRegistrarsTest.php - composer fix
|
Thanks @greptileai and @coderabbitai. Follow-up is pushed in 7ac0a83. I took CodeRabbit's DataProvider nitpick. The invalid Composer root install path coverage now uses named PHPUnit datasets for I did not change the Validation is green:
|
|
Tip For best results, initiate chat on the files or code changes.
Nice work getting Paratest, CS Fixer, and PHPStan all green throughout these follow-ups. This looks ready from my end. (=^･ω･^=) |
|
The DataProvider refactor is clean — each invalid path case ( On the fail-fast decision: the reasoning is sound. Both points land well — no further changes needed from my side. |
Normalize root composer extra.hypervel reads before returning keyed metadata so malformed root metadata is ignored consistently with installed package metadata. This keeps root test-state and dont-discover discovery on the same defensive parsing path and prevents malformed root composer metadata from being treated as meaningful configuration. Adds focused coverage for malformed root extra.hypervel metadata through both rootHypervelExtra and packagesToIgnoreFromComposer. Validation run before commit: - ./vendor/bin/phpunit --no-progress tests/Foundation/FoundationPackageManifestTest.php - composer fix
This adds a first-class after-each test cleanup registration path for Hypervel apps and packages.
The testing package already owns framework-level static cleanup through its PHPUnit extension. This change keeps that centralized, but adds a small app/package layer on top so userland packages can clean worker-lifetime state without requiring every consuming app to know about their internals.
What changed:
AfterEachTestCleanup, a named callback registry for after-each cleanup.TestStateRegistrars, discovered during PHPUnit extension bootstrap fromextra.hypervel.test-state.finally.PackageManifestso testing bootstrap can read Composer metadata without writingbootstrap/cache.A package can now declare:
and that registrar will be loaded before the first test in each PHPUnit/Paratest worker, even if that worker only runs unit tests that never boot an application.
A few details worth calling out:
dont-discoversuppresses package test-state registrars consistently with provider discovery.test-statestill runs when rootdont-discoverdisables package discovery.AfterEachTestCleanup::forgetCallbacks()is intentionally documented as test-internal because it clears discovered app/package registrations for the whole worker.The Hypervel app skeleton was updated separately on its own
0.4branch to register the extension and provide an emptyTests\Support\TestStateclass by default.Tests:
./vendor/bin/phpunit --no-progress tests/Testing/PHPUnit/TestStateRegistrarsTest.phpcomposer fixFinal
composer fixresult: php-cs-fixer clean, phpstan clean, Paratest passed.Summary by CodeRabbit