Add Filament admin CRUD tests for all resources and the Settings page - #2564
Merged
Merged
Conversation
Every admin resource now has create, edit, delete, validation, and
negative-authorization coverage: Servers, Nodes, Users, Eggs, Mounts,
BackupHosts, DatabaseHosts, Roles, Webhooks, and ApiKeys, plus render,
authorization, and save-persistence tests for the Settings page.
Wings calls are mocked through DaemonServerRepository like the existing
service tests, and the database host connection check is faked with a
proxied partial of the database manager so the panel connection stays
real. EnvironmentWriterTrait now resolves the env file through
app()->environmentFilePath() instead of a hardcoded base_path('.env'),
which behaves identically in production and lets the settings test point
writes at a scratch file.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe change makes environment writes use the configured application environment path. It adds Filament tests for settings and admin CRUD flows, including validation, persistence, deletion, external-call handling, and permission enforcement. ChangesAdmin panel coverage
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to Environment settings now use the configured application environment-file path, while the change adds admin and settings coverage. No concrete merge-blocking risk is identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
The custom PHPStan rule forbids the app() global.
rmartinoscar
approved these changes
Sep 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ListNodesTestpattern.EnvironmentWriterTraitnow writes toapp()->environmentFilePath()instead of hardcodedbase_path('.env'), which is identical in production but lets the settings test redirect writes to a scratch file viauseEnvironmentPath().DaemonServerRepository(same pattern asServerCreationServiceTest); the database host connection check is faked with a proxied partial of the database manager so the panel's own connection stays real.Raises
tests/Filamentfrom 43 to 92 tests. Part of the W5 critical-path test coverage work; a follow-up PR covers the Application API endpoints.Test plan
vendor/bin/pest tests/Filament --parallelpasses (92 tests).vendor/bin/pest tests/Integration --parallelstill passes (the trait change is exercised by the installer tests).