Bootstrap Foundation test applications through the kernel#426
Conversation
|
Warning Review limit reached
Next review available in: 20 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 (5)
📝 WalkthroughWalkthroughThe ChangesTestCase Bootstrap Refactor
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Test as FoundationTestCase
participant Bootstrap as bootstrap/app.php
participant App as Application
participant Kernel as KernelContract
Test->>Bootstrap: require file
Bootstrap-->>Test: return Application instance
Test->>App: make(KernelContract::class)
App-->>Kernel: resolve kernel
Test->>Kernel: bootstrap()
Kernel-->>Test: bootstrap complete
Test->>Test: return bootstrapped $app
🚥 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 fixes the missing kernel bootstrap step in
Confidence Score: 5/5Safe to merge — the change is a focused bug fix restoring one missing call, backed by a direct regression test. The core change is a single-line addition that restores the kernel bootstrap step the codebase had always intended to run. The Env repository fallback is well-guarded with is_string(), teardown in both new and updated tests properly isolates process environment state, and the regression test exercises the exact failure path described in the PR. No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "test(testing): harden app bootstrap clea..." | Re-trigger Greptile |
Restore the Laravel and Orchestra testing lifecycle for normal application tests by bootstrapping the created application through the console kernel inside Foundation\Testing\TestCase::createApplication(). The Hypervel port had kept the bootstrap/app.php require but dropped the kernel bootstrap call. That meant real app tests could construct an application object without running the bootstrappers, so the first facade access during the test lifecycle failed before user tests could run. This uses the full kernel bootstrap path rather than a testing-specific bootstrapper list. There is no Hypervel-specific reason to diverge here: the kernel already owns the bootstrap sequence, HandleExceptions is part of the normal framework bootstrap, and the testing teardown already flushes exception state. Also remove the temporary shared default bootstrapper constant introduced by the earlier PR draft and keep the HTTP and console bootstrapper lists local to their kernels, matching the framework shape used elsewhere.
Remove the stale skeleton CreatesApplication trait now that the base Foundation test case bootstraps applications directly. Starter-style applications no longer need to carry this Laravel skeleton trait for Hypervel tests to work. Keeping it in the testbench skeleton would make the supported path harder to read because it suggests app tests should override createApplication(), when the framework test case now owns that boot path.
Add a regression test for the base Foundation testing path used by real applications. The test creates a small temporary application fixture, points APP_BASE_PATH at it, and runs the real Foundation\Testing\TestCase setUp() lifecycle. That reproduces the path that previously failed before user tests could run, then asserts that the app is bootstrapped, the expected base path is used, facades are bound, and config is available. The fixture is intentionally self-contained instead of copying the Testbench skeleton. Testbench owns a different lifecycle for package tests, while this coverage is specifically for application tests that rely on the base Foundation test case.
acbd2ff to
97e584c
Compare
Teach Application::inferBasePath() to fall back to the Env repository after checking the explicit APP_BASE_PATH superglobals. This keeps the existing precedence for raw and values while making framework test helpers and worker environments that write through putenv visible to base path inference. The Env lookup is evaluated lazily so the repository is not touched when an explicit superglobal value is present. Add builder coverage for APP_BASE_PATH values set through the environment repository and for taking precedence over that fallback. While the builder test file is touched, align its test methods with the current : void convention.
Move APP_BASE_PATH restoration and facade reset for the Foundation app bootstrap regression test into the outer test lifecycle. The anonymous inner Foundation test case still tears itself down inside the test method, but process-global cleanup now runs from the enclosing test tearDown. That means facade state and APP_BASE_PATH are restored even when the inner teardown throws, avoiding cross-test contamination in a long-running parallel worker. This directly addresses Greptile's review comment without weakening the regression coverage for the real application test bootstrap path.
This fixes the base Foundation application test path.
A normal app test that extends
Hypervel\Foundation\Testing\TestCasecreates the application frombootstrap/app.php, then the framework test lifecycle immediately starts using framework services. The Hypervel port had kept thebootstrap/app.phprequire, but dropped Laravel's kernel bootstrap step. That leaves the app constructed but not bootstrapped, so facades, providers, and configuration are not ready when the test lifecycle touches them.The fix is to put that missing bootstrap back where upstream has it:
Foundation\Testing\TestCase::createApplication()now boots the created app through the console kernel before returning it.This keeps the test path boring in the best way:
HandleExceptionsremains part of the normal bootstrap pathI also removed the stale skeleton
Tests\CreatesApplicationtrait. With the base Foundation test case doing the right thing, new app skeletons should not need to overridecreateApplication()just to boot the framework.The regression test creates a tiny temporary app, points
APP_BASE_PATHat it, and runs the realFoundation\Testing\TestCase::setUp()path. It asserts that the app is bootstrapped, the base path is correct, facades are bound, and config is available. That covers the failure mode directly without involving Testbench's different lifecycle.Credit to Richard's PR #423 for surfacing the bug and the app-test failure path.
Validation run locally:
./vendor/bin/phpunit --no-progress tests/Foundation/Testing/ApplicationBootstrapTest.phpcomposer fixSummary by CodeRabbit