search

nunomaduro / laravel-mojito

🍹 A lightweight package for testing Laravel views in isolation
http://nunomaduro.com/
MIT License
373 stars 16 forks source link

`in()` method result may be incomplete #10

Closed SimoTod closed 4 years ago

SimoTod commented 4 years ago

It may not be an issue but just my misunderstanding.

I was expecting the in() method to select all the DOM elements matching the given condition so we can assert against that subset but it seems to consider only the first match so there are no real differences between first() and in()

If we consider the welcome view in the test folder (https://github.com/nunomaduro/laravel-mojito/blob/master/tests/fixtures/welcome.html), I would expect $this->assertView('welcome')->in('.links a')->contains('Docs'); pass $this->assertView('welcome')->first('.links a')->contains('Docs'); pass $this->assertView('welcome')->in('.links a')->contains('Laracasts'); pass $this->assertView('welcome')->first('.links a')->contains('Laracasts'); fail but right now we get $this->assertView('welcome')->in('.links a')->contains('Docs'); pass $this->assertView('welcome')->first('.links a')->contains('Docs'); pass $this->assertView('welcome')->in('.links a')->contains('Laracasts'); fail??? $this->assertView('welcome')->first('.links a')->contains('Laracasts'); fail

I just wanted to get people's opinion since it could be just me misinterpreting the feature but, if we feel that in() should act as I described, I'm happy to PR that in.

nunomaduro commented 4 years ago

Honestly, I don't remember. 👍

SimoTod commented 4 years ago

I guess the real question is: if someone reads $this->assertView('welcome')->in('.links a')->contains('Laracasts'); for the first time, will they expect it to pass or fail? I can "fix" it, but I don't want to impose my view on this so I would love to know what other people think.

nunomaduro commented 4 years ago

Because $this->assertView('welcome')->in('.links a')->contains('Laracasts'); fail??? selects the first element right?

SimoTod commented 4 years ago

Yep, currently it just selects the first node matching the selector.

nunomaduro commented 4 years ago

Not sure if that is a bug or not, I think is the normal behaviour of this selector .links a

SimoTod commented 4 years ago

Cool, it's probably subjective, happy for you to close this issue. Thanks.

SimoTod commented 4 years ago

To give more context (sorry for the spamming), this would be my suggested solution if it becomes a problem in the future. https://github.com/nunomaduro/laravel-mojito/compare/master...SimoTod:feature/in-method-improvements

nunomaduro commented 4 years ago

I trust you to take a decision on this one, just add a couple of tests to make sure its not breaking.

SimoTod commented 4 years ago

@nunomaduro sounds good. It doesn't change the existing behaviour for old tests so it seems okay. I've drafted a release (Added more documentation as well), let me know if you are okay with me publishing it. Thanks.

nunomaduro commented 4 years ago

Sounds good for the release, don't forget to update the changelog.