sync with pest repo #4
Conversation
Ensure laravel namespaces are only used in code that is already specific to laravel.
…l host in visit chain
…ailable Make ->withHost available when using visit(..)
fix: allow use of assertScript without laravel
There was a problem hiding this comment.
Pull request overview
This pull request synchronizes the codebase with the pest repository by adding support for per-visit host configuration and removing the dependency on Laravel's Illuminate\Support\Str helper class.
Changes:
- Added
withHost()method toPendingAwaitablePagefor per-visit host configuration with automatic restoration of global host settings - Replaced
Illuminate\Support\Strusage with native PHP string functions to reduce dependencies - Added comprehensive test coverage for subdomain routing and host configuration scenarios
- Enhanced architecture tests to enforce dependency boundaries for Laravel/Illuminate/Livewire namespaces
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Browser/Visit/SubdomainTest.php | Adds 7 new test cases covering various host configuration scenarios including chaining, global vs local hosts, and host restoration |
| tests/ArchTest.php | Implements architecture test to restrict Illuminate/Laravel/Livewire usage to specific classes |
| src/Configuration.php | Updates withHost() parameter type to accept nullable string |
| src/Api/PendingAwaitablePage.php | Implements per-visit withHost() method with temporary host management and proper cleanup |
| src/Api/Concerns/MakesElementAssertions.php | Replaces Illuminate\Support\Str with native str_contains() and str_starts_with() functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ->assertSee('"host":"api.localhost"'); | ||
| }); | ||
|
|
||
| it('Chaining withHost will not override global host', function (): void { |
There was a problem hiding this comment.
Test description should start with a lowercase letter to be consistent with the existing test naming convention in this file (e.g., "can visit non-subdomain routes" on line 8, "works with Laravel subdomain style" on line 27).
| it('Chaining withHost will not override global host', function (): void { | |
| it('chaining withHost will not override global host', function (): void { |
| } | ||
|
|
||
| /** | ||
| * Return true if haystack contains any of the given needles |
There was a problem hiding this comment.
The documentation comment is missing a period at the end of the sentence, which is inconsistent with other documentation comments in the codebase (e.g., line 599 "Sets the host for the server.").
| * Return true if haystack contains any of the given needles | |
| * Return true if haystack contains any of the given needles. |
| ->assertSee('"host":"api.localhost"'); | ||
| }); | ||
|
|
||
| it('Can chain withHost on visit', function (): void { |
There was a problem hiding this comment.
Test description should start with a lowercase letter to be consistent with the existing test naming convention in this file (e.g., "can visit non-subdomain routes" on line 8, "works with Laravel subdomain style" on line 27).
4 participants
No description provided.