Skip to content

Add tests for thenables returned from async script #57160

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.

Sign up for GitHub

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

Open
jugglinmike wants to merge 6 commits into
base: master
Choose a base branch
from
+34 −0
Open

Add tests for thenables returned from async script #57160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
160437b
Add tests for thenables returned from async script
jugglinmike Jan 14, 2026
240ebfc
Incorporate review feedback
jugglinmike Jan 24, 2026
1c51e92
Merge remote-tracking branch 'upstream/master' into webdriver-classic…
jugglinmike Jan 25, 2026
192af48
Remove assertions for response data
jugglinmike Jan 25, 2026
b6904bc
Clarify test
jugglinmike Jan 25, 2026
c678159
Remove assertion for response data
jugglinmike Jan 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions webdriver/tests/classic/execute_async_script/promise.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,29 @@ def test_promise_reject_timeout(session):
resolve(promise);
""")
assert_error(response, "script timeout")


def test_returned_promise_fulfilled_over_callback(session):
session.timeouts.script = 1
response = execute_async_script(session, """
let resolve = arguments[0];
setTimeout(() => resolve('callback'), 200);
return Promise.resolve('promise');
""")
assert_success(response, "promise")


def test_returned_promise_rejected_over_callback(session):
session.timeouts.script = 1
response = execute_async_script(session, """
return Promise.reject(new Error('my error'));
Copy link
Copy Markdown
Contributor

@whimboo whimboo Jan 30, 2026

Choose a reason for hiding this comment

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

Shouldn't we do it actually similar to the above case for fulfillment and resolve the promise with a different error? That way Promise.reject should still have precedence, right?

Copy link
Copy Markdown
Contributor Author

@jugglinmike jugglinmike Jan 30, 2026

Choose a reason for hiding this comment

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

Shouldn't we do it actually similar to the above case for fulfillment and resolve the promise with a different error? That way Promise.reject should still have precedence, right?

Ah, yes. I didn't recognize how renaming the test made the callback relevant here. I've made that change and pushed it to this branch.

Doing so helped me recognize that we're not being completely systematic here. There are three variables: the callback result (which should be considered an error if it is a rejected promise), the promise result, and the sequence in which they occur.

I think we actually need 8 tests, not just two:

callback result promise result sequence subtest title expected
"callback" "promise" callback then promise test_callback_success_then_promise_success "callback"
"callback" "promise" promise then callback test_promise_success_then_callback_success "promise"1
error "promise" callback then promise test_callback_error_then_promise_success error
error "promise" promise then callback test_promise_success_then_callback_error "promise"1
"callback" error callback then promise test_callback_success_then_promise_error "callback"
"callback" error promise then callback test_promise_error_then_callback_success error
error error callback then promise test_callback_error_then_promise_error error
error error promise then callback test_callback_error_then_promise_error error

Though, given that we can't differentiate between script errors, we might skip those final two tests.

What do you think?

Footnotes

  1. We'll change these if-and-when https://github.com/w3c/webdriver/pull/1939 lands ↩ ↩2

""")
assert_error(response, "javascript error")


def test_returned_poisoned_thenable(session):
session.timeouts.script = .1
response = execute_async_script(session, """
return { get then() { thow new Error('my error'); } };
""")
assert_error(response, "javascript error")
8 changes: 8 additions & 0 deletions webdriver/tests/classic/execute_script/promise.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,3 +100,11 @@ def test_promise_reject_timeout(session):
);
""")
assert_error(response, "script timeout")


def test_returned_poisoned_thenable(session):
session.timeouts.script = .1
response = execute_script(session, """
return { get then() { thow new Error('my error'); } };
""")
assert_error(response, "javascript error")