-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Remove testharness from reftest #43310
Conversation
There was a problem hiding this 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 Chromium project.
I somehow made this both a testharness test and a reftest. This CL converts this to just a reftest, to avoid breaking things [1]. [1] #43303 Bug: 1236777 Change-Id: I89fc76e68a24ab12c4a9efeaac36183dfb85b746 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5055427 Commit-Queue: Mason Freed <masonf@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1228158}
9b3e062
to
a51e6fc
Compare
@@ -24,5 +30,5 @@ | |||
assert_equals(shadow.adoptedStyleSheets.length,0); | |||
shadow.adoptedStyleSheets = shadow.adoptedStyleSheets.concat([sheet]); | |||
assert_equals(shadow.adoptedStyleSheets.length,1); | |||
}, "adoptedStyleSheets should allow .concat on empty starting values"); | |||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mfreed7 You probably want to use reftest-wait
in order for this to work like you expect? Otherwise it might take a screenshot before the errors are shown.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing in the test is async though - all of it should render synchronously as part of window load, so there shouldn’t be a need to do reftest-wait
, right? This passes on Chromium.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the error show up in Chromium when you do assert_equals(false, true)
? I'm concerned about false passes rather false fails. It's possible that reftest-wait
is enough, but looking at: https://web-platform-tests.org/writing-tests/reftests.html#controlling-when-comparison-occurs . I don't see anything that mentions explicitly that it waits before all the load event listeners have executed before screenshotting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Chromium, assert_equals(false,true) does in fact fail this test.
So I think it makes sense to enforce this behavior - WDYT? I've put this infra test up for review:
Several good comments here - I'll take another crack at this test... |
This adds a WPT infrastructure test to enforce that the harness waits for window.load before capturing reftest images. That allows testing like this: ``` <link rel=match href=green.html> <script> window.onload = () => document.body.append('test'); </script> ``` See [1] for conversation. [1] #43310 Change-Id: I1f3f9836ca880350b294538dd2f2d58689485f7b
See [1] for comments, but this CL adds reftest-wait to ensure the test finishes before the image capture, and adds PASS lines for all passed tests. This makes no difference in Chromium, but ensures this test works correctly in other browsers. [1] #43310 Bug:1236777 Change-Id: I849284eafdd0afb630a11b0a9e6b73bc0264fd6b
|
This adds a WPT infrastructure test to enforce that the harness waits for window.load before capturing reftest images. That allows testing like this: ``` <link rel=match href=green.html> <script> window.onload = () => document.body.append('test'); </script> ``` See [1] for conversation. [1] #43310 Change-Id: I1f3f9836ca880350b294538dd2f2d58689485f7b
This adds a WPT infrastructure test to enforce that the harness waits for window.load before capturing reftest images. That allows testing like this: ``` <link rel=match href=green.html> <script> window.onload = () => document.body.append('test'); </script> ``` See [1] for conversation. [1] #43310 Change-Id: I1f3f9836ca880350b294538dd2f2d58689485f7b
See [1] for comments, but this CL adds reftest-wait to ensure the test finishes before the image capture, and adds PASS lines for all passed tests. This makes no difference in Chromium, but ensures this test works correctly in other browsers. [1] #43310 Bug:1236777 Change-Id: I849284eafdd0afb630a11b0a9e6b73bc0264fd6b
See [1] for comments, but this CL adds reftest-wait to ensure the test finishes before the image capture, and adds PASS lines for all passed tests. This makes no difference in Chromium, but ensures this test works correctly in other browsers. [1] #43310 Bug: 1236777 Change-Id: I849284eafdd0afb630a11b0a9e6b73bc0264fd6b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5062333 Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1229509}
See [1] for comments, but this CL adds reftest-wait to ensure the test finishes before the image capture, and adds PASS lines for all passed tests. This makes no difference in Chromium, but ensures this test works correctly in other browsers. [1] web-platform-tests/wpt#43310 Bug: 1236777 Change-Id: I849284eafdd0afb630a11b0a9e6b73bc0264fd6b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5062333 Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1229509}
See [1] for comments, but this CL adds reftest-wait to ensure the test finishes before the image capture, and adds PASS lines for all passed tests. This makes no difference in Chromium, but ensures this test works correctly in other browsers. [1] #43310 Bug: 1236777 Change-Id: I849284eafdd0afb630a11b0a9e6b73bc0264fd6b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5062333 Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1229509}
This adds a WPT infrastructure test to enforce that the harness waits for window.load before capturing reftest images. That allows testing like this: ``` <link rel=match href=green.html> <script> window.onload = () => document.body.append('test'); </script> ``` See [1] for conversation. [1] web-platform-tests/wpt#43310 Change-Id: I1f3f9836ca880350b294538dd2f2d58689485f7b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5064229 Reviewed-by: Weizhong Xia <weizhong@google.com> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1229618}
This adds a WPT infrastructure test to enforce that the harness waits for window.load before capturing reftest images. That allows testing like this: ``` <link rel=match href=green.html> <script> window.onload = () => document.body.append('test'); </script> ``` See [1] for conversation. [1] #43310 Change-Id: I1f3f9836ca880350b294538dd2f2d58689485f7b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5064229 Reviewed-by: Weizhong Xia <weizhong@google.com> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1229618}
This adds a WPT infrastructure test to enforce that the harness waits for window.load before capturing reftest images. That allows testing like this: ``` <link rel=match href=green.html> <script> window.onload = () => document.body.append('test'); </script> ``` See [1] for conversation. [1] #43310 Change-Id: I1f3f9836ca880350b294538dd2f2d58689485f7b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5064229 Reviewed-by: Weizhong Xia <weizhong@google.com> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1229618}
…structable-concat.html, a=testonly Automatic update from web-platform-tests Address concerns about CSSStyleSheet-constructable-concat.html See [1] for comments, but this CL adds reftest-wait to ensure the test finishes before the image capture, and adds PASS lines for all passed tests. This makes no difference in Chromium, but ensures this test works correctly in other browsers. [1] web-platform-tests/wpt#43310 Bug: 1236777 Change-Id: I849284eafdd0afb630a11b0a9e6b73bc0264fd6b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5062333 Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1229509} -- wpt-commits: 2f79845b3d3e3a0244c8512df9b6c430e5a1d2f7 wpt-pr: 43377
…before reftest capture, a=testonly Automatic update from web-platform-tests Add infra test that window.load happens before reftest capture This adds a WPT infrastructure test to enforce that the harness waits for window.load before capturing reftest images. That allows testing like this: ``` <link rel=match href=green.html> <script> window.onload = () => document.body.append('test'); </script> ``` See [1] for conversation. [1] web-platform-tests/wpt#43310 Change-Id: I1f3f9836ca880350b294538dd2f2d58689485f7b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5064229 Reviewed-by: Weizhong Xia <weizhong@google.com> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1229618} -- wpt-commits: aff9e54e2b07b57885c89e8d2175570946a9e5bc wpt-pr: 43376
…structable-concat.html, a=testonly Automatic update from web-platform-tests Address concerns about CSSStyleSheet-constructable-concat.html See [1] for comments, but this CL adds reftest-wait to ensure the test finishes before the image capture, and adds PASS lines for all passed tests. This makes no difference in Chromium, but ensures this test works correctly in other browsers. [1] web-platform-tests/wpt#43310 Bug: 1236777 Change-Id: I849284eafdd0afb630a11b0a9e6b73bc0264fd6b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5062333 Reviewed-by: Joey Arhar <jarhar@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1229509} -- wpt-commits: 2f79845b3d3e3a0244c8512df9b6c430e5a1d2f7 wpt-pr: 43377
…before reftest capture, a=testonly Automatic update from web-platform-tests Add infra test that window.load happens before reftest capture This adds a WPT infrastructure test to enforce that the harness waits for window.load before capturing reftest images. That allows testing like this: ``` <link rel=match href=green.html> <script> window.onload = () => document.body.append('test'); </script> ``` See [1] for conversation. [1] web-platform-tests/wpt#43310 Change-Id: I1f3f9836ca880350b294538dd2f2d58689485f7b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5064229 Reviewed-by: Weizhong Xia <weizhong@google.com> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1229618} -- wpt-commits: aff9e54e2b07b57885c89e8d2175570946a9e5bc wpt-pr: 43376
…structable-concat.html, a=testonly Automatic update from web-platform-tests Address concerns about CSSStyleSheet-constructable-concat.html See [1] for comments, but this CL adds reftest-wait to ensure the test finishes before the image capture, and adds PASS lines for all passed tests. This makes no difference in Chromium, but ensures this test works correctly in other browsers. [1] web-platform-tests/wpt#43310 Bug: 1236777 Change-Id: I849284eafdd0afb630a11b0a9e6b73bc0264fd6b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5062333 Reviewed-by: Joey Arhar <jarharchromium.org> Commit-Queue: Mason Freed <masonfchromium.org> Commit-Queue: Joey Arhar <jarharchromium.org> Auto-Submit: Mason Freed <masonfchromium.org> Cr-Commit-Position: refs/heads/main{#1229509} -- wpt-commits: 2f79845b3d3e3a0244c8512df9b6c430e5a1d2f7 wpt-pr: 43377 UltraBlame original commit: fc535649c0bf241509629e174a6e2bd92270e783
…before reftest capture, a=testonly Automatic update from web-platform-tests Add infra test that window.load happens before reftest capture This adds a WPT infrastructure test to enforce that the harness waits for window.load before capturing reftest images. That allows testing like this: ``` <link rel=match href=green.html> <script> window.onload = () => document.body.append('test'); </script> ``` See [1] for conversation. [1] web-platform-tests/wpt#43310 Change-Id: I1f3f9836ca880350b294538dd2f2d58689485f7b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5064229 Reviewed-by: Weizhong Xia <weizhonggoogle.com> Commit-Queue: Mason Freed <masonfchromium.org> Cr-Commit-Position: refs/heads/main{#1229618} -- wpt-commits: aff9e54e2b07b57885c89e8d2175570946a9e5bc wpt-pr: 43376 UltraBlame original commit: 12454e2d818196a790f2711897bc1424a98ecb86
…structable-concat.html, a=testonly Automatic update from web-platform-tests Address concerns about CSSStyleSheet-constructable-concat.html See [1] for comments, but this CL adds reftest-wait to ensure the test finishes before the image capture, and adds PASS lines for all passed tests. This makes no difference in Chromium, but ensures this test works correctly in other browsers. [1] web-platform-tests/wpt#43310 Bug: 1236777 Change-Id: I849284eafdd0afb630a11b0a9e6b73bc0264fd6b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5062333 Reviewed-by: Joey Arhar <jarharchromium.org> Commit-Queue: Mason Freed <masonfchromium.org> Commit-Queue: Joey Arhar <jarharchromium.org> Auto-Submit: Mason Freed <masonfchromium.org> Cr-Commit-Position: refs/heads/main{#1229509} -- wpt-commits: 2f79845b3d3e3a0244c8512df9b6c430e5a1d2f7 wpt-pr: 43377 UltraBlame original commit: fc535649c0bf241509629e174a6e2bd92270e783
…before reftest capture, a=testonly Automatic update from web-platform-tests Add infra test that window.load happens before reftest capture This adds a WPT infrastructure test to enforce that the harness waits for window.load before capturing reftest images. That allows testing like this: ``` <link rel=match href=green.html> <script> window.onload = () => document.body.append('test'); </script> ``` See [1] for conversation. [1] web-platform-tests/wpt#43310 Change-Id: I1f3f9836ca880350b294538dd2f2d58689485f7b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5064229 Reviewed-by: Weizhong Xia <weizhonggoogle.com> Commit-Queue: Mason Freed <masonfchromium.org> Cr-Commit-Position: refs/heads/main{#1229618} -- wpt-commits: aff9e54e2b07b57885c89e8d2175570946a9e5bc wpt-pr: 43376 UltraBlame original commit: 12454e2d818196a790f2711897bc1424a98ecb86
…structable-concat.html, a=testonly Automatic update from web-platform-tests Address concerns about CSSStyleSheet-constructable-concat.html See [1] for comments, but this CL adds reftest-wait to ensure the test finishes before the image capture, and adds PASS lines for all passed tests. This makes no difference in Chromium, but ensures this test works correctly in other browsers. [1] web-platform-tests/wpt#43310 Bug: 1236777 Change-Id: I849284eafdd0afb630a11b0a9e6b73bc0264fd6b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5062333 Reviewed-by: Joey Arhar <jarharchromium.org> Commit-Queue: Mason Freed <masonfchromium.org> Commit-Queue: Joey Arhar <jarharchromium.org> Auto-Submit: Mason Freed <masonfchromium.org> Cr-Commit-Position: refs/heads/main{#1229509} -- wpt-commits: 2f79845b3d3e3a0244c8512df9b6c430e5a1d2f7 wpt-pr: 43377 UltraBlame original commit: fc535649c0bf241509629e174a6e2bd92270e783
…before reftest capture, a=testonly Automatic update from web-platform-tests Add infra test that window.load happens before reftest capture This adds a WPT infrastructure test to enforce that the harness waits for window.load before capturing reftest images. That allows testing like this: ``` <link rel=match href=green.html> <script> window.onload = () => document.body.append('test'); </script> ``` See [1] for conversation. [1] web-platform-tests/wpt#43310 Change-Id: I1f3f9836ca880350b294538dd2f2d58689485f7b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5064229 Reviewed-by: Weizhong Xia <weizhonggoogle.com> Commit-Queue: Mason Freed <masonfchromium.org> Cr-Commit-Position: refs/heads/main{#1229618} -- wpt-commits: aff9e54e2b07b57885c89e8d2175570946a9e5bc wpt-pr: 43376 UltraBlame original commit: 12454e2d818196a790f2711897bc1424a98ecb86
I somehow made this both a testharness test and a reftest. This
CL converts this to just a reftest, to avoid breaking things [1].
[1] #43303
Bug: 1236777
Change-Id: I89fc76e68a24ab12c4a9efeaac36183dfb85b746
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5055427
Commit-Queue: Mason Freed <masonf@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1228158}