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

Allow Dusk selectors anywhere in the selector string #1105

Closed rabrowne85 closed 2 months ago

rabrowne85 commented 2 months ago

The current implementation requires that the dusk selector is the first selector in a string of selectors, but will replace any number of dusk selectors with the appropriate data attribute syntax, regardless of where they occur in the selector string as long as the first is a dusk selector.

I can't see any reason that the string has to start with the @ symbol from the dusk perspective, and whilst I can also appreciate that each dusk selector should perhaps be unique, I feel that there should be some form of duplication as if they are meant to be unique then why not use standard id attributes.

Therefore the change from Str::startsWith() to Str::contains() is generally more flexible, e.g., in cases where the dusk selectors are not unique on a page (for whatever reason) and you want to use css combinators to get the one you want without having to define the full css selector, which could be very brittle.

I know that scoping would provide a similar outcome, but I feel there would be scenarios where this might still be insufficient.

There's nothing in the documentation that states it has to be the first selector, which means it's a little confusing at first if you try and use a dusk selector as the 2nd or 3rd part of the full selector and the browser test fails because it's an invalid selector.

From what I can tell, there shouldn't be any breaking changes associated with it:

github-actions[bot] commented 2 months ago

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

taylorotwell commented 2 months ago

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions!