dusk icon indicating copy to clipboard operation
dusk copied to clipboard

Published 20 hours ago • verified publisher icon laravel

New Assertions: assertPathEndsWith and assertPathContains

Open shawnhooper opened this issue 11 months ago • 2 comments

Adds two new assertions to go with the existing assertPathBeginsWith:

  • assertPathEndsWith
  • assertPathContains

shawnhooper avatar Feb 27 '24 19:02 shawnhooper

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

taylorotwell avatar Feb 28 '24 02:02 taylorotwell

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.

shawnhooper avatar Mar 01 '24 02:03 shawnhooper

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

u01jmg3 avatar Apr 12 '24 14:04 u01jmg3

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.

driesvints avatar Apr 15 '24 12:04 driesvints

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 👍🏻

u01jmg3 avatar Apr 15 '24 13:04 u01jmg3

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

shawnhooper avatar Apr 15 '24 14:04 shawnhooper

@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?

u01jmg3 avatar Apr 16 '24 15:04 u01jmg3

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!

driesvints avatar Apr 16 '24 15:04 driesvints