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

Leverage WebDriver built-in implicit wait #153

Conversation

tristanlabelle
Copy link
Contributor

We've been emulating implicit wait functionality by repeated requests when webdriver offers it as a built-in mechanism. Using the built-in mechanism when available has the advantage of making logs less verbose. This change preserves the repeated request emulation mechanism as a fallback.

This change aligns better with web driver concepts. We use "implicit wait timeout" instead of "default retry timeout", and make it only apply to findElement, introducing a separate "implicit interaction retry timeout" for interactions (which is not a webdriver protocol built-in functionality). The implicit wait timeout now defaults to 0 seconds, as per spec.

Individual operations still offer to specify a wait timeout, and this is implemented by temporarily changing the implicit wait timeout, since that is the only mechanism offered by webdriver.

Copy link
Contributor

@jeffdav jeffdav left a comment

Choose a reason for hiding this comment

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

Looks good, I think. I would maybe update the README to talk about the various timeouts and how they interact?

@tristanlabelle
Copy link
Contributor Author

@jeffdav good call, I updated the readme.

@tristanlabelle tristanlabelle merged commit 62d5b06 into main Jul 5, 2024
4 checks passed
@tristanlabelle tristanlabelle deleted the tristan/leverage-webdriver-implicit-wait-split-interaction-retry branch July 5, 2024 13:01
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.

2 participants