search

laravel / dusk

Laravel Dusk provides simple end-to-end testing and browser automation.
https://laravel.com/docs/dusk
MIT License
1.87k stars 320 forks source link

New Assertions: assertPathEndsWith and assertPathContains #1088

Closed shawnhooper closed 5 months ago

shawnhooper commented 6 months ago

Adds two new assertions to go with the existing assertPathBeginsWith:

taylorotwell commented 6 months ago

I would maybe use Str::contains for the contains one.

shawnhooper commented 6 months ago

Hi @taylorotwell, thanks for the feedback.

Looking at the other assertions in the MakesUrlAssertions trait, and they all call assertions from the PHPUnit framework. I think using PHPUnit::assertStringContainsString() is more consistent, and cleaner than checking Str::contains and then trying to throw a PHPUnit assertion error if it doesn't.

u01jmg3 commented 5 months ago

@driesvints: any chance we could get this PR progressed? Would be great to see assertPathEndsWith() in Dusk

driesvints commented 5 months ago

Taylor doesn't sees PR's in draft state. If a new review is needed, the PR needs to be marked as such. I'll do this for now.

u01jmg3 commented 5 months ago

Thanks @driesvints - I can't change the state of a draft PR and I'm not sure it was clear to @shawnhooper that they weren't going to get a reply. Either way, this PR will get looked at now 👍🏻

shawnhooper commented 5 months ago

@u01jmg3 Correct. I assumed Taylor has been busy and would get to it eventually. First time contributing into the official packages. :)

u01jmg3 commented 4 months ago

@driesvints - I see this was merged into 7.x and with today's v8.1.2 release, this PR hasn't made it in. Should this PR have gone into the 8.x branch? Perhaps because this PR was created back in February.

Shall I raise a new PR to add these changes to 8.x or can you fix it?

driesvints commented 4 months ago

This should indeed have gone into 8.x since 7.x is no longer maintained. If you could send in a new PR I'll merge and tag it. Thanks!