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

added whitelist #49

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

added whitelist #49

wants to merge 4 commits into from

Conversation

andreabisello
Copy link

Now you can pass a whitelist of css selector.
If css selectors matches somethings, those elements visibility will be set to "hidden" in order to avoid screenshot comparison failures on elements that can, by nature, change.

@jphalip
Copy link
Collaborator

jphalip commented May 30, 2016

Nice, I really like this feature. Thanks for the suggestion.

Here are a few comments:

  • Could the parameter be named "ignore" instead of "whitelist". I think that would make the intent a bit clearer.
  • Could you make that parameter follow the same type as element_or_selector, i.e. in this case be a "list of NeedleWebElement objects or of CSS selectors"?
  • It'd be best to make use of the Selenium API and use find_elements_by_css_selector() instead of executing the querySelector() Javascript call, in order to make it compatible across more browsers.
  • Could you add a unit test for this?

Thanks!

@andreabisello
Copy link
Author

@jphalip

How i can set css properties or alter visibility of WebElement using WebElement api? in documentation http://selenium-python.readthedocs.io/api.html?highlight=javascript#module-selenium.webdriver.remote.webelement i find only getter method and in stack overflow everyone use execute_script api, so i used execute_script api to make element hidden using javascript.

any suggestion?

thanks.

@jphalip
Copy link
Collaborator

jphalip commented Jun 1, 2016

My recommendation is specifically to use find_elements_by_css_selector() to retrieve the elements, since querySelector() has some limited support in old browsers like IE 8 (granted, that's not a huge deal).

Then to set a CSS property on a WebElement you should be able to reference it with arguments[0] when running execute_script. Something like: driver.execute_script('arguments[0].style.visibility="hidden";')

2)element to be ignored now can be css selector or needle web element
3)querySelector is not used anymore in order to find the element
@andreabisello
Copy link
Author

@jphalip i updated the code and now i think your bullets suggestions 1,2,3 are fine with 8887618 .

i never created a unit test. any suggestion for this use case?

thanks

@jphalip
Copy link
Collaborator

jphalip commented Jun 10, 2016

Thanks for the updates. First, one small comment: I think we should assume that the provided CSS selectors might potentially return multiple elements. So, could you use find_elements_by_css_selector() (note the plural) instead of find_element_by_css_selector() and then loop through the resulting list of elements?

Also, a note about the ignore parameter's default value. You have it as a list, which is a common pitfall in Python programming. See this article for more information: http://effbot.org/zone/default-values.htm Instead you should give it a default value of None and then follow the same approach as described in the article.

Regarding the tests. You could have the tests create a page with two elements in it, then take a screenshot while ignoring one of them, and then compare the resulting screenshot with a pre-generated control image. You should take a look at the existing tests for some inspiration.

@andreabisello
Copy link
Author

@jphalip thanks for the suggestions :-) 👍

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