selenium icon indicating copy to clipboard operation
selenium copied to clipboard

[🚀 Feature]: [py] A common interface/mixin for element location `find_element(...)` & `find_elements(...)`

Open symonk opened this issue 2 years ago • 3 comments

Feature and motivation

There is some duplication throughout the following areas that are (largely) carrying out the same tasks:

grep -rE "def find_elements?\(" * 
selenium/webdriver/remote/webdriver.py:    def find_element(self, by=By.ID, value=None) -> WebElement:
selenium/webdriver/remote/webdriver.py:    def find_elements(self, by=By.ID, value=None) -> List[WebElement]:
selenium/webdriver/remote/webelement.py:    def find_element(self, by=By.ID, value=None):
selenium/webdriver/remote/webelement.py:    def find_elements(self, by=By.ID, value=None):
selenium/webdriver/remote/shadowroot.py:    def find_element(self, by: By = By.ID, value: str = None):
selenium/webdriver/remote/shadowroot.py:    def find_elements(self, by: By = By.ID, value: str = None):
selenium/webdriver/support/event_firing_webdriver.py:    def find_element(self, by=By.ID, value=None):
selenium/webdriver/support/event_firing_webdriver.py:    def find_elements(self, by=By.ID, value=None):
selenium/webdriver/support/event_firing_webdriver.py:    def find_element(self, by=By.ID, value=None):
selenium/webdriver/support/event_firing_webdriver.py:    def find_elements(self, by=By.ID, value=None):

Note: EventFiringWebDriver is just dynamically dispatching calls at runtime and implements nothing itself.

These areas can likely be simplified by exposing some sort of LocatesElementsMixin (or whatever is better named in line with other bindings; a mixin class is just an idea as it has some drawbacks too, but this issue is more just to figure out what would be a good approach to unifying it.

A few of the problems:

  • Keeping backwards compatibility
  • The actual webdriver Command dispatch varies throughout
  • instance attributes vary in naming which makes a mixin class a little tougher and unclean

Curious on your thoughts to this? @AutomatedTester has a comment in ShadowRoot.py here; I assume it was also aiming to move towards something like this?

https://github.com/SeleniumHQ/selenium/blob/575b878db2efccdde95ea8d0bdf6b8db4bea1fa0/py/selenium/webdriver/remote/shadowroot.py#L26

Python is not java of course so we should keep that in mind and leverage duck typing etc where applicable.

symonk avatar May 15 '22 17:05 symonk

@symonk, thank you for creating this issue. We will troubleshoot it as soon as we can.


Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C), add the applicable G-* label, and it will provide the correct link and auto-close the issue.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

github-actions[bot] avatar May 15 '22 17:05 github-actions[bot]

This part of the deprecation plan in my head for the find_element_* method to get us to this point and then we can move to either using Mixins here or have a pure object (like .NET/Java).

AutomatedTester avatar May 18 '22 11:05 AutomatedTester

@titusfortner cleared the milestone here; don't think it's a good one, let's focus on clearing down what needs removed first, then we can look at consolidating the APIs

symonk avatar Jun 08 '22 19:06 symonk

This issue is stale because it has been open 280 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] avatar Mar 15 '23 20:03 github-actions[bot]

This issue is stale because it has been open 280 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] avatar Dec 21 '23 20:12 github-actions[bot]

@AutomatedTester were you thinking of having a SearchContext class that each of these methods can call? (because I know you don't like mix-ins)

titusfortner avatar Dec 22 '23 00:12 titusfortner

This issue is looking for contributors.

Please comment below or reach out to us through our IRC/Slack/Matrix channels if you are interested.

github-actions[bot] avatar Dec 29 '23 19:12 github-actions[bot]