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

Set custom default timeouts for WebDriver classic session #39034

Merged
merged 1 commit into from
Mar 20, 2023

Conversation

whimboo
Copy link
Contributor

@whimboo whimboo commented Mar 16, 2023

The default wdspec timeout for a complete file is set to 25s. Having a page_load timeout of 300s and a script timeout of 30s here doesn't make sense. As such I propose to use custom values for our wdspec tests which load pages from a local server anyway and that shouldn't take longer than around 3s.

To incorporate slow running builds we will also use the timeout multiplier to account for that.

@whimboo
Copy link
Contributor Author

whimboo commented Mar 16, 2023

@gsnedders and @patrickangle could you please have a look at this proposed change? I would appreciate your review.

Copy link
Member

@gsnedders gsnedders left a comment

Choose a reason for hiding this comment

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

Somewhat to my surprise, the only regressions wpt.fyi shows for Safari are pre-existing flakiness. I'm amazed we don't have more things relying on longer timeouts!

I'd be happier with wpt.fyi reporting the results for the Chrome and Firefox runs—filed web-platform-tests/wpt.fyi#3210 for the fact it didn't do that.

webdriver/tests/get_timeouts/get.py Show resolved Hide resolved
Copy link
Contributor

@jgraham jgraham left a comment

Choose a reason for hiding this comment

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

Seems fine to me; I agree with @gsnedders that it's a bit unfortunate to lose the test coverage for defaults, but I can't think of an easy way to work around that. Otherwise, as long as it doesn't show regressions in Gecko CI then I'm happy.

@whimboo
Copy link
Contributor Author

whimboo commented Mar 20, 2023

Seems fine to me; I agree with @gsnedders that it's a bit unfortunate to lose the test coverage for defaults, but I can't think of an easy way to work around that. Otherwise, as long as it doesn't show regressions in Gecko CI then I'm happy.

As mentioned as part of the inline comment we do not loose coverage. The default values are checked in /new_session/timeouts.py.

Here the results from the Gecko CI which as well look all fine: https://treeherder.mozilla.org/jobs?repo=try&revision=2d00abc97d081486070bebf0c39b43efe96243ed

@jgraham jgraham merged commit 1a44d85 into web-platform-tests:master Mar 20, 2023
@whimboo whimboo deleted the wdspec_timeouts branch March 20, 2023 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra webdriver wg-testing_browser wptrunner The automated test runner, commonly called through ./wpt run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants