[🐛 Bug]: [py] Mypy error - WebDriverWait.driver has attribute

[MarcelWilson]

Description

The following error from mypy occurs for WebDriverWait.__repr__

error: Item "WebElement" of the upper bound "WebDriver | WebElement" of type variable "D" has no attribute "session_id" [union-attr]

Explanation:
If you pass a WebElement into WebDriverWait, the repr will not be able to properly access the session_id attribute. Mypy is detecting this.

    def __repr__(self):
        return f'<{type(self).__module__}.{type(self).__name__} (session="{self._driver.session_id}")>'

My guess is there likely needs to be additional logic to access driver from WebElement.parent in those cases, or session_id needs to be exposed as an attribute of WebElement.

Reproducible Code

def __repr__(self):
        return f'<{type(self).__module__}.{type(self).__name__} (session="{self._driver.session_id}")>'

[selenium-ci]

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

Selenium Triage Team: remember to follow the Triage Guide

[cgoldberg]

Not surprising. I just ran our Mypy configuration across the codebase and we currently have 120 errors in 30 files! :)

If by chance you are interested in cleaning up our type checking, a PR would be very welcomed (If not, I'll get to it eventually).

You can run the Mypy job with:

tox -c py/tox.ini -e mypy

[selenium-ci]

This issue is looking for contributors.

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

[cgoldberg]

For the larger issue: #15697

[MarcelWilson]

If by chance you are interested in cleaning up our type checking, a PR would be very welcomed (If not, I'll get to it eventually).

I wouldn't mind giving it a go. The question then becomes about approach. Is it wise to expose the sessionid in WebElement?
Or is it preferred to do a check in the WebDriverWait and pull out the driver from there?

[cgoldberg]

You need access to it from whichever class the __repr__ is on, right?

I don't have the code in front of me at the moment, but... Is driver referenced anywhere else in that module? If not, I'm not sure it belongs there at all.

[MarcelWilson]

Under the hood, WebDriverWait needs a WebDriver to pass into the method.

selenium/py/selenium/webdriver/support/wait.py

Lines 134 to 146 in 255749b

	end_time = time.monotonic() + self._timeout
	while True:
	try:
	value = method(self._driver)
	if value:
	return value
	except self._ignored_exceptions as exc:
	screen = getattr(exc, "screen", None)
	stacktrace = getattr(exc, "stacktrace", None)
	if time.monotonic() > end_time:
	break
	time.sleep(self._poll)
	raise TimeoutException(message, screen, stacktrace)

However, it also accepts passing in WebElement since they also have the ability to make calls like find_element - as seen in several expected_condition functions.

selenium/py/selenium/webdriver/support/expected_conditions.py

Lines 222 to 228 in 255749b

	def _predicate(driver: WebDriverOrWebElement):
	try:
	return _element_if_visible(driver.find_element(*locator))
	except StaleElementReferenceException:
	return False
	return _predicate

The only real issue here is when WebDriver.__repr__ tries to access the attribute of session_id which currently only exists on the remote.WebDriver.

I see a couple of possibilities here:

1. WebElement could expose session_id as a property. Since it already utilizes the attribute for it's own __repr__ it would be a small step
   

   selenium/py/selenium/webdriver/remote/webelement.py

   Lines 79 to 80 in 255749b

   	def __repr__(self):
   	return f'<{type(self).__module__}.{type(self).__name__} (session="{self._parent.session_id}", element="{self._id}")>'

2. WebDriverWait would need to pull out the session_id from both objects in separate logic.

if driver:
    ...
elif element:
    ...

My hunch is the first approach makes the most sense (also has the smallest code change). But I fear this opinion may be biased as a user.

[MarcelWilson]

I tried to submit a PR with the code change but it appears to have been automatically closed. I'm not sure I understand the objection. Maybe you can do something with the submission.

[cgoldberg]

I just re-opened your PR... Our bot is a little aggressive with closing things :)

I'll review it later today. Thanks.

[titusfortner]

Our bot is a little aggressive with closing things

When I close something instead of letting a bot do it, I'm mean
When a bot closes something and I re-open it, I'm helpful

😂