Skip to content

webdriver: Wait indefinitely with None timeout#57403

Merged
servo-wpt-sync merged 1 commit intoweb-platform-tests:masterfrom
servo:servo_export_42184
Jan 30, 2026
Merged

webdriver: Wait indefinitely with None timeout#57403
servo-wpt-sync merged 1 commit intoweb-platform-tests:masterfrom
servo:servo_export_42184

Conversation

@servo-wpt-sync
Copy link
Collaborator

@servo-wpt-sync servo-wpt-sync commented Jan 29, 2026

So far for None timeout, we've treated it effectively as no wait which is wrong: #57332 (comment).

This PR makes all None timeouts wait indefinitely. The funny part is, we somehow have only done it correctly for pageload: https://w3c.github.io/webdriver/#dfn-timeouts-configuration

Testing: We add test for script execution with None script timeout.
Reviewed in servo/servo#42184

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

The review process for this patch is being conducted in the Servo project.

Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
@yezhizhen
Copy link
Contributor

Would you like to check the test? @whimboo

Chrome is passing but Firefox is failing them. But I think there's been something wrong with Firefox's promise handling before this:
https://github.com/web-platform-tests/wpt/pull/57403/checks?check_run_id=61828175740

assert_error(response, "script timeout")


def test_promise_resolve_timeout_none(session):
Copy link
Contributor

@yezhizhen yezhizhen Jan 29, 2026

Choose a reason for hiding this comment

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

This test is exactly same as

def test_promise_resolve_delayed(session):
response = execute_script(session, """
return new Promise(
(resolve) => setTimeout(
() => resolve('foobar'),
50
)
);
""")
assert_success(response, "foobar")
, except setTimeout changed from 50 to 200.

But somehow Firefox is passing test_promise_resolve_delayed but failing test_promise_resolve_timeout_none?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this is related to #57160 and other recently opened issues around promises.

Can this PR wait a bit until the others have been done?

Copy link
Contributor

@yezhizhen yezhizhen Jan 29, 2026

Choose a reason for hiding this comment

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

This has nothing to do with #57160 tho. The tests here are essentially same as existing ones.

@servo-wpt-sync servo-wpt-sync merged commit 5aa7861 into web-platform-tests:master Jan 30, 2026
25 checks passed
@servo-wpt-sync servo-wpt-sync deleted the servo_export_42184 branch January 30, 2026 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants