Fix a bug in test_driver.bless() #49691

mfreed7 · 2024-12-14T00:58:41Z

The code was previously doing this:

  let wait_click = new Promise(resolve => {
    button.addEventListener("click", resolve));
  };
  return test_driver.click(button)
    .then(wait_click)
    .then(...

but the argument to .then(wait_click) isn't a function, it's the promise to return. Therefore .then() ignores its argument (see step #3 at https://tc39.es/ecma262/multipage/control-abstraction-objects.html#sec-performpromisethen) and proceeds immediately to the next .then() block.

Since the code wasn't actually waiting for the click event to finish getting fired, subsequent steps could happen out of order.

The code was previously doing this: let wait_click = new Promise(resolve => { button.addEventListener("click", resolve)); }; return test_driver.click(button) .then(wait_click) .then(... but the argument to `.then(wait_click)` isn't a function, it's the promise to return. Therefore .then() creates an already-resolved promise containing `wait_click` as its resolved value. Which the next `.then()` block ignores. So this wasn't actually waiting for the click to occur, meaning subsequent steps could happen before the click actually took place.

This code was previously doing this: let wait_click = new Promise(resolve => { button.addEventListener("click", resolve)); }; return test_driver.click(button) .then(wait_click) .then(... but the argument to `.then(wait_click)` isn't a function, it's the promise to return. Therefore .then() creates an already-resolved promise containing `wait_click` as its resolved value. Which the next `.then()` block ignores. So this wasn't actually waiting for the click to occur. This triggered a number of test bugs (caused by erroneous assumptions accidentally baked into the tests. I fixed a few, and filed a few bugs for the rest (after failing to figure out how to fix them). Note that the WPT version of testdriver.js is rolled into Chromium, so that change is being made here: #49691 Bug: 384009734,384050894 Change-Id: Ibdb8a97d23998ad89c5a48c23a7e780dc605283b

This code was previously doing this: let wait_click = new Promise(resolve => { button.addEventListener("click", resolve)); }; return test_driver.click(button) .then(wait_click) .then(... but the argument to `.then(wait_click)` isn't a function, it's the promise to return. Therefore .then() creates an already-resolved promise containing `wait_click` as its resolved value. Which the next `.then()` block ignores. So this wasn't actually waiting for the click to occur. This triggered a number of test bugs (caused by erroneous assumptions accidentally baked into the tests. I fixed a few, and filed a few bugs for the rest (after failing to figure out how to fix them). Note that the WPT version of testdriver.js is rolled into Chromium, so that change is being made here: web-platform-tests/wpt#49691 Bug: 384009734,384050894 Change-Id: Ibdb8a97d23998ad89c5a48c23a7e780dc605283b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6094526 Reviewed-by: Jonathan Lee <[email protected]> Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Mason Freed <[email protected]> Commit-Queue: Jonathan Lee <[email protected]> Cr-Commit-Position: refs/heads/main@{#1397010}

This code was previously doing this: let wait_click = new Promise(resolve => { button.addEventListener("click", resolve)); }; return test_driver.click(button) .then(wait_click) .then(... but the argument to `.then(wait_click)` isn't a function, it's the promise to return. Therefore .then() creates an already-resolved promise containing `wait_click` as its resolved value. Which the next `.then()` block ignores. So this wasn't actually waiting for the click to occur. This triggered a number of test bugs (caused by erroneous assumptions accidentally baked into the tests. I fixed a few, and filed a few bugs for the rest (after failing to figure out how to fix them). Note that the WPT version of testdriver.js is rolled into Chromium, so that change is being made here: #49691 Bug: 384009734,384050894 Change-Id: Ibdb8a97d23998ad89c5a48c23a7e780dc605283b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6094526 Reviewed-by: Jonathan Lee <[email protected]> Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Mason Freed <[email protected]> Commit-Queue: Jonathan Lee <[email protected]> Cr-Commit-Position: refs/heads/main@{#1397010}

mfreed7 · 2024-12-16T23:46:05Z

Closing this one in favor of #49719

This code was previously doing this: let wait_click = new Promise(resolve => { button.addEventListener("click", resolve)); }; return test_driver.click(button) .then(wait_click) .then(... but the argument to `.then(wait_click)` isn't a function, it's the promise to return. Therefore .then() creates an already-resolved promise containing `wait_click` as its resolved value. Which the next `.then()` block ignores. So this wasn't actually waiting for the click to occur. This triggered a number of test bugs (caused by erroneous assumptions accidentally baked into the tests. I fixed a few, and filed a few bugs for the rest (after failing to figure out how to fix them). Note that the WPT version of testdriver.js is rolled into Chromium, so that change is being made here: #49691 Bug: 384009734,384050894 Change-Id: Ibdb8a97d23998ad89c5a48c23a7e780dc605283b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6094526 Reviewed-by: Jonathan Lee <[email protected]> Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Mason Freed <[email protected]> Commit-Queue: Jonathan Lee <[email protected]> Cr-Commit-Position: refs/heads/main@{#1397010} Co-authored-by: Mason Freed <[email protected]>

…only Automatic update from web-platform-tests Fix a bug in test_driver.bless() (#49719) This code was previously doing this: let wait_click = new Promise(resolve => { button.addEventListener("click", resolve)); }; return test_driver.click(button) .then(wait_click) .then(... but the argument to `.then(wait_click)` isn't a function, it's the promise to return. Therefore .then() creates an already-resolved promise containing `wait_click` as its resolved value. Which the next `.then()` block ignores. So this wasn't actually waiting for the click to occur. This triggered a number of test bugs (caused by erroneous assumptions accidentally baked into the tests. I fixed a few, and filed a few bugs for the rest (after failing to figure out how to fix them). Note that the WPT version of testdriver.js is rolled into Chromium, so that change is being made here: web-platform-tests/wpt#49691 Bug: 384009734,384050894 Change-Id: Ibdb8a97d23998ad89c5a48c23a7e780dc605283b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6094526 Reviewed-by: Jonathan Lee <[email protected]> Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Mason Freed <[email protected]> Commit-Queue: Jonathan Lee <[email protected]> Cr-Commit-Position: refs/heads/main@{#1397010} Co-authored-by: Mason Freed <[email protected]> -- wpt-commits: ce746b469d6c2fe11cf9db769eb2b3426bad6466 wpt-pr: 49719

…only Automatic update from web-platform-tests Fix a bug in test_driver.bless() (#49719) This code was previously doing this: let wait_click = new Promise(resolve => { button.addEventListener("click", resolve)); }; return test_driver.click(button) .then(wait_click) .then(... but the argument to `.then(wait_click)` isn't a function, it's the promise to return. Therefore .then() creates an already-resolved promise containing `wait_click` as its resolved value. Which the next `.then()` block ignores. So this wasn't actually waiting for the click to occur. This triggered a number of test bugs (caused by erroneous assumptions accidentally baked into the tests. I fixed a few, and filed a few bugs for the rest (after failing to figure out how to fix them). Note that the WPT version of testdriver.js is rolled into Chromium, so that change is being made here: web-platform-tests/wpt#49691 Bug: 384009734,384050894 Change-Id: Ibdb8a97d23998ad89c5a48c23a7e780dc605283b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6094526 Reviewed-by: Jonathan Lee <jonathanjleegoogle.com> Auto-Submit: Mason Freed <masonfchromium.org> Commit-Queue: Mason Freed <masonfchromium.org> Commit-Queue: Jonathan Lee <jonathanjleegoogle.com> Cr-Commit-Position: refs/heads/main{#1397010} Co-authored-by: Mason Freed <masonfchromium.org> -- wpt-commits: ce746b469d6c2fe11cf9db769eb2b3426bad6466 wpt-pr: 49719 UltraBlame original commit: 337b58e389432dc74cdafad2aeb85b1e0ea5fc3c

…only Automatic update from web-platform-tests Fix a bug in test_driver.bless() (#49719) This code was previously doing this: let wait_click = new Promise(resolve => { button.addEventListener("click", resolve)); }; return test_driver.click(button) .then(wait_click) .then(... but the argument to `.then(wait_click)` isn't a function, it's the promise to return. Therefore .then() creates an already-resolved promise containing `wait_click` as its resolved value. Which the next `.then()` block ignores. So this wasn't actually waiting for the click to occur. This triggered a number of test bugs (caused by erroneous assumptions accidentally baked into the tests. I fixed a few, and filed a few bugs for the rest (after failing to figure out how to fix them). Note that the WPT version of testdriver.js is rolled into Chromium, so that change is being made here: web-platform-tests/wpt#49691 Bug: 384009734,384050894 Change-Id: Ibdb8a97d23998ad89c5a48c23a7e780dc605283b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6094526 Reviewed-by: Jonathan Lee <[email protected]> Auto-Submit: Mason Freed <[email protected]> Commit-Queue: Mason Freed <[email protected]> Commit-Queue: Jonathan Lee <[email protected]> Cr-Commit-Position: refs/heads/main@{#1397010} Co-authored-by: Mason Freed <[email protected]> -- wpt-commits: ce746b469d6c2fe11cf9db769eb2b3426bad6466 wpt-pr: 49719

mfreed7 marked this pull request as ready for review December 14, 2024 02:04

mfreed7 requested a review from a team as a code owner December 14, 2024 02:04

wpt-pr-bot added infra testdriver.js labels Dec 14, 2024

wpt-pr-bot assigned jgraham Dec 14, 2024

wpt-pr-bot requested a review from jgraham December 14, 2024 02:04

chromium-wpt-export-bot mentioned this pull request Dec 16, 2024

Fix a bug in test_driver.bless() #49719

Merged

mfreed7 closed this Dec 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix a bug in test_driver.bless() #49691

Fix a bug in test_driver.bless() #49691

mfreed7 commented Dec 14, 2024

mfreed7 commented Dec 16, 2024

Fix a bug in test_driver.bless() #49691

Fix a bug in test_driver.bless() #49691

Conversation

mfreed7 commented Dec 14, 2024

mfreed7 commented Dec 16, 2024