Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove select and selected?/1 from Browser module. #240

Merged
merged 2 commits into from
May 15, 2017

Conversation

keathley
Copy link
Member

@keathley keathley commented May 15, 2017

This PR removes the select functions and selected?/1 from the Browser module. It would be very nice to have some sort of query composition for finding options inside of a given select. Because there are multiple "Option 2" options on the page you get a duplicate element error if you don't scope everything to the specific select.

|> selected?(Query.option("Option 2")) == true
|> find(select("My Select"))
|> click(option("Option 2"))
|> selected?(option("Option 2")) == true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this test call selected/2 and the one below calls Element.selected/2? Also would it be good to have a test to ensure this function returns false when the option is not selected?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test below should be calling Element.selected?/1. I'll add a test to make sure that it returns false. Thats a good catch.

@keathley
Copy link
Member Author

@aaronrenner Added the extra tests. Lemme know what you think.

@aaronrenner
Copy link
Member

Thanks, the tests look good. Is the reason wallaby has Browser.selected and Element.selected is because Browser.selected has retry logic and Element.selected does not?

@keathley
Copy link
Member Author

keathley commented May 15, 2017

Partially. In general the Browser module provides functions that accept queries and provide retrying, etc. The Element module accepts elements and doesn't provide any retry logic. In fact the Element module really can't provide any retry logic. Typically if a query needs to be retried its because an element wasn't able to be found or it was found and then went stale half-way through the element validation (checking for text or visibilty). But the big separation is that Browser accepts queries instead of elements.

@keathley keathley merged commit a60550d into master May 15, 2017
@keathley keathley deleted the chores/remove-select branch May 15, 2017 14:28
@aaronrenner
Copy link
Member

That makes sense. Thanks for the explanation.

graeme-defty pushed a commit to graeme-defty/wallaby that referenced this pull request May 16, 2017
* Remove select and selected?/1 from Browser module.

* Add negative tests for selected?/2 and selected?/1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants