Update testharness.js, testdriver.js, idlharness.js and webidl2.js separately (not in auto-imports) |
|||||||||||||
Issue descriptionThere have been a multiple times when updating web-platform-tests updates testharness.js (and other resources which are copied to LayoutTests/resources) and this modifies the behavior of tests outside of external/wpt. To make auto-imports more reliable or simpler, it might be better if the import process didn't update LayoutTests/resources/, and if those were updated separately (and less often), probably manually? This would enable the auto-importer to be sure to only affect imported tests, and the updating of expectations and baselines could then ignore any flaky failures outside of the directory being updated. I'm not entirely sure whether this would be be the best way though. Are there any big disadvantages to having LayoutTests/resources/testharness.js and LayoutTests/external/wpt/resources/testharness.js out of sync sometimes?
,
Feb 14 2017
I think that what we actually want to do is probably what is proposed in bug 679742 -- somehow make it so that console messages don't affect whether testharness.js tests pass.
,
Jun 19 2017
Looking at this again, I think that updating testharness.js (and idlharness.js) separately would be a good choice, because it could mean that we could say with certainty that the importer should never update expectations outside of wpt. As it is now, the importer can sometimes change unrelated expectations outside of wpt, and this sometimes happens, e.g. in https://chromium.googlesource.com/chromium/src/+/60cc55805fd10b00fcd9472b17ae9517d7f942c9.
,
Jun 19 2017
,
Jun 19 2017
Updating separately sgtm
,
Jun 19 2017
Awesome -- uploaded a CL to do this: https://chromium-review.googlesource.com/c/539818/ The main question now of course is how we should update them -- I was thinking that for now, they could be updated manually, and later there could be a separate script that: 1. Copies the files over 2. Runs try jobs and downloads new baselines. This script could also be run on the wpt-importer bot. Does that sound reasonable?
,
Jun 19 2017
Also sgtm!
,
Jun 20 2017
Allowing the copies to drift apart to make import more straightforward SGTM, but who will be responsible for updating testharness.js so that they get back in sync regularly?
,
Jun 20 2017
I was thinking that in the immediate future, I would update it when I notice a change in testharness.js in an import :-/ It seems like testharness.js usually changes a few times per month, but being one or two months out of sync probably won't cause a lot of trouble. File history: https://chromium.googlesource.com/chromium/src/+log/master/third_party/WebKit/LayoutTests/resources/testharness.js As mentioned in #6, we could have another recipe, or another step in the wpt-importer recipe, which copies the files from external/wpt/resources/ to resources/, then starts try jobs and rebaselines any files anywhere in LayoutTests *except* for wpt. It seems possible that this rebaselining could occasionally introduce incorrect baselines, but at least the CL would be smaller and focused on one change. I think we should keep this bug open until there's a process for keeping testharness (and idlharness) in sync.
,
Jun 21 2017
Keeping this open until we have a process SGTM. Adding idlharness.js to the title as well. It's not necessarily worth automating this, we could also make it part of a rotation that resolves export problems, perhaps.
,
Jun 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5bbc2bf8150d69cc30463a9040de0e97f5caad28 commit 5bbc2bf8150d69cc30463a9040de0e97f5caad28 Author: Quinten Yearsley <qyearsley@google.com> Date: Wed Jun 21 20:10:11 2017 Don't copy testharness.js and idlharness.js on wpt import. Background: In general we want to keep LayoutTests/resources/testharness.js in sync with LayoutTests/external/wpt/resources/testharness.js because we want testharness tests inside of and outside of wpt to behave similarly. However, the practice of keeping these files always in sync means that it's possible for results of testharness tests outside of wpt to change upon import. Currently, the wpt importer tries to download new baselines for any tests that fail on import. This works most of the time but sometimes means we download baselines that we didn't want to download. Rationale for this change: To simplify the import process, I think it's easier to make it so that imports can only affect tests in external/wpt. I think there should be a separate process to sync the testharness.js and idlharness.js files; this could be manual and occasional, or it could be automatic. It could be a separate "mode" of the wpt-importer. In this change: 1. Simplify importer so it doesn't copy files over 2. Simplify presubmit so it doesn't check that the files are in sync 3. Change wpt-update-expectations so that it ignores all failures outside of external/wpt. Bug: 685854 Change-Id: I8f39790313cd1429f9016418a76a0eecc3183016 Reviewed-on: https://chromium-review.googlesource.com/539818 Commit-Queue: Quinten Yearsley <qyearsley@chromium.org> Reviewed-by: Kent Tamura <tkent@chromium.org> Reviewed-by: Philip Jägenstedt <foolip@chromium.org> Cr-Commit-Position: refs/heads/master@{#481285} [modify] https://crrev.com/5bbc2bf8150d69cc30463a9040de0e97f5caad28/third_party/WebKit/LayoutTests/PRESUBMIT.py [modify] https://crrev.com/5bbc2bf8150d69cc30463a9040de0e97f5caad28/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py [modify] https://crrev.com/5bbc2bf8150d69cc30463a9040de0e97f5caad28/third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_expectations_updater.py [modify] https://crrev.com/5bbc2bf8150d69cc30463a9040de0e97f5caad28/third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_expectations_updater_unittest.py
,
Jul 3 2017
,
Jul 3 2017
,
Jul 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/724dbb5f562ebb4c62c6c52015f7a916b73c1923 commit 724dbb5f562ebb4c62c6c52015f7a916b73c1923 Author: Quinten Yearsley <qyearsley@google.com> Date: Wed Jul 19 01:21:06 2017 Reorganize the order of methods in wpt importer This CL reorganizes the (confusingly-named) update and do_auto_update methods in TestImporter. The main purpose of this is to prepare to re-use the code related to triggering try jobs, updating expectations and triggering in order to automate the updating of LayoutTests/resources/testharness.js. The other purpose is to try to reorganize the code to improve readability. Bug: 685854 Change-Id: I6aac4a0875763fbcc7f5fbdd16cc038798243f92 Reviewed-on: https://chromium-review.googlesource.com/576368 Commit-Queue: Quinten Yearsley <qyearsley@chromium.org> Reviewed-by: Jeff Carpenter <jeffcarp@chromium.org> Cr-Commit-Position: refs/heads/master@{#487705} [modify] https://crrev.com/724dbb5f562ebb4c62c6c52015f7a916b73c1923/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py [modify] https://crrev.com/724dbb5f562ebb4c62c6c52015f7a916b73c1923/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer_unittest.py
,
Aug 23 2017
,
Sep 30 2017
,
Oct 2 2017
I believe the next step here is to make a script which does this: 1. Copy LayoutTests/external/wpt/resources/testharness.js to LayoutTests/resources/. 2. Upload a CL. 3. Trigger and wait for try jobs. 4. Downloads new baselines. (There should generally be no new test expectation lines.) 5. Triggers CQ. There are other conceivable ways to do it, like making this an option of wpt-import, but that option seems more complicated somehow. This script would use the functionality of FileSystem (copying file) and GitCL (triggering try job, uploading CL, triggering CQ); the easiest way to rebaseline now is by invoking webkit-patch rebaseline-cl in a separate process just like how it's done in wpt_expectations_updater.py.
,
Oct 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5c6da120dc404e10f642e4540426999875b68017 commit 5c6da120dc404e10f642e4540426999875b68017 Author: Quinten Yearsley <qyearsley@chromium.org> Date: Tue Oct 03 08:53:22 2017 Manually update Chromium's copy of testharness.js and idlharness.js. Bug: 685854 Change-Id: I3eeaad133bf9a1830eb5ef8558e3f29966214872 Reviewed-on: https://chromium-review.googlesource.com/695662 Commit-Queue: Philip Jägenstedt <foolip@chromium.org> Reviewed-by: Philip Jägenstedt <foolip@chromium.org> Cr-Commit-Position: refs/heads/master@{#505988} [modify] https://crrev.com/5c6da120dc404e10f642e4540426999875b68017/third_party/WebKit/LayoutTests/compositing/overflow/composited-scroll-overlap-test-expected.txt [modify] https://crrev.com/5c6da120dc404e10f642e4540426999875b68017/third_party/WebKit/LayoutTests/fast/dom/custom/svg-use-shadow-tree-expected.txt [modify] https://crrev.com/5c6da120dc404e10f642e4540426999875b68017/third_party/WebKit/LayoutTests/http/tests/w3c/webperf/approved/navigation-timing/html/idlharness-expected.txt [modify] https://crrev.com/5c6da120dc404e10f642e4540426999875b68017/third_party/WebKit/LayoutTests/http/tests/webaudio/autoplay-crossorigin-expected.txt [modify] https://crrev.com/5c6da120dc404e10f642e4540426999875b68017/third_party/WebKit/LayoutTests/resources/idlharness.js [modify] https://crrev.com/5c6da120dc404e10f642e4540426999875b68017/third_party/WebKit/LayoutTests/resources/testharness.js
,
Nov 21 2017
,
Nov 21 2017
Adding testdriver.js to the title per issue 781322 .
,
Nov 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4c9a7b51c37e742e24f36b5d47680bce3361fb96 commit 4c9a7b51c37e742e24f36b5d47680bce3361fb96 Author: Philip Jägenstedt <foolip@chromium.org> Date: Fri Nov 24 15:17:17 2017 Sync LayoutTests/resources/ with LayoutTests/external/wpt/resources/ The change to assert_array_equals in testharness.js affected two tests: LayoutTests/bluetooth/descriptor/writeValue/write-updates-value.html LayoutTests/fast/canvas-api/canvas-lineDash-invalid.html The bluetooth test was previously trying to compare two ArrayBuffer instances, which would before silently fail to do something useful, as actual/expected.length were undefined and `0 < undefined` is false. The canvas test was just using the wrong assert method. The change to assert_array_equals came from here: (Thanks @Ms2ger!) https://github.com/w3c/web-platform-tests/pull/7560 Bug: 685854 (not a fix, just related) Change-Id: I12214518a6445fa6e52fa06a0a954e8acc8f289c Reviewed-on: https://chromium-review.googlesource.com/786016 Reviewed-by: Robert Ma <robertma@chromium.org> Commit-Queue: Philip Jägenstedt <foolip@chromium.org> Cr-Commit-Position: refs/heads/master@{#519116} [modify] https://crrev.com/4c9a7b51c37e742e24f36b5d47680bce3361fb96/third_party/WebKit/LayoutTests/bluetooth/descriptor/writeValue/write-updates-value.html [modify] https://crrev.com/4c9a7b51c37e742e24f36b5d47680bce3361fb96/third_party/WebKit/LayoutTests/fast/canvas-api/canvas-lineDash-invalid.html [modify] https://crrev.com/4c9a7b51c37e742e24f36b5d47680bce3361fb96/third_party/WebKit/LayoutTests/fast/dom/custom/svg-use-shadow-tree-expected.txt [modify] https://crrev.com/4c9a7b51c37e742e24f36b5d47680bce3361fb96/third_party/WebKit/LayoutTests/http/tests/security/subresourceIntegrity/subresource-integrity-style-allowed-expected.txt [modify] https://crrev.com/4c9a7b51c37e742e24f36b5d47680bce3361fb96/third_party/WebKit/LayoutTests/http/tests/webaudio/autoplay-crossorigin-expected.txt [modify] https://crrev.com/4c9a7b51c37e742e24f36b5d47680bce3361fb96/third_party/WebKit/LayoutTests/resources/WebIDLParser.js [modify] https://crrev.com/4c9a7b51c37e742e24f36b5d47680bce3361fb96/third_party/WebKit/LayoutTests/resources/idlharness.js [modify] https://crrev.com/4c9a7b51c37e742e24f36b5d47680bce3361fb96/third_party/WebKit/LayoutTests/resources/testdriver.js [modify] https://crrev.com/4c9a7b51c37e742e24f36b5d47680bce3361fb96/third_party/WebKit/LayoutTests/resources/testharness.js
,
Nov 28 2017
Changed the summary to include webidl2.js
,
Nov 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0578baafc77e7360e0027975b9436a4d9385fa03 commit 0578baafc77e7360e0027975b9436a4d9385fa03 Author: Robert Ma <robertma@chromium.org> Date: Tue Nov 28 21:08:27 2017 Create a copy of webidl2.js for non-WPT layout tests With this change, non-WPT layout tests are completely independent of WPT files and hence won't be affected by imports. The change also makes the behaviour more consistent as we already have copies of testharness.js, testdriver.js and idlharness.js, with webidl2.js being the only left. An apache alias rule is changed accordingly. See crbug.com/685854 for maintaining these Chromium copies. Besides, WebIDLParser.js (which is an old version of webidl2.js) is removed from resources/ to avoid confusion. The only three files using it are modified to use webidl2.js instead. Bug: 787829 , 685854 Change-Id: I2d9966646cc7db557d87ec403cd0c833ee262279 Reviewed-on: https://chromium-review.googlesource.com/785992 Commit-Queue: Robert Ma <robertma@chromium.org> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> Reviewed-by: Philip Jägenstedt <foolip@chromium.org> Cr-Commit-Position: refs/heads/master@{#519834} [modify] https://crrev.com/0578baafc77e7360e0027975b9436a4d9385fa03/third_party/WebKit/LayoutTests/installedapp/idl.html [modify] https://crrev.com/0578baafc77e7360e0027975b9436a4d9385fa03/third_party/WebKit/LayoutTests/resources/README.txt [rename] https://crrev.com/0578baafc77e7360e0027975b9436a4d9385fa03/third_party/WebKit/LayoutTests/resources/webidl2.js [modify] https://crrev.com/0578baafc77e7360e0027975b9436a4d9385fa03/third_party/WebKit/LayoutTests/webaudio/audio-worklet/audio-worklet-node-idl.html [modify] https://crrev.com/0578baafc77e7360e0027975b9436a4d9385fa03/third_party/WebKit/LayoutTests/webauth/idl.html [modify] https://crrev.com/0578baafc77e7360e0027975b9436a4d9385fa03/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/servers/apache_http.py
,
Feb 13 2018
Is the remaining work here just writing the script described in Comment 17?
,
Feb 14 2018
And run the script (or manually update the resources) periodically.
,
Mar 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b98895b9d2871d0365f4c5509d8371542df5897b commit b98895b9d2871d0365f4c5509d8371542df5897b Author: Robert Ma <robertma@chromium.org> Date: Thu Mar 01 22:39:41 2018 Sync LayoutTests/resources/ with LayoutTests/external/wpt/resources/ Roll in the most recent version of: testharness.js, testdriver.js, idlharness.js and webidl2.js Some baselines are adjusted accordingly (mostly due to https://github.com/w3c/web-platform-tests/pull/9051). Bug: 685854 Change-Id: Ic74431ad0bbbc13e0c9ab2fa8bb4701f69806d32 Reviewed-on: https://chromium-review.googlesource.com/941915 Reviewed-by: Philip Jägenstedt <foolip@chromium.org> Commit-Queue: Robert Ma <robertma@chromium.org> Cr-Commit-Position: refs/heads/master@{#540318} [modify] https://crrev.com/b98895b9d2871d0365f4c5509d8371542df5897b/third_party/WebKit/LayoutTests/animations/stability/keyframe-iteration-exception-crash-expected.txt [modify] https://crrev.com/b98895b9d2871d0365f4c5509d8371542df5897b/third_party/WebKit/LayoutTests/fast/dom/custom/svg-use-shadow-tree-expected.txt [modify] https://crrev.com/b98895b9d2871d0365f4c5509d8371542df5897b/third_party/WebKit/LayoutTests/fast/js/put-forwards-expected.txt [modify] https://crrev.com/b98895b9d2871d0365f4c5509d8371542df5897b/third_party/WebKit/LayoutTests/http/tests/webaudio/autoplay-crossorigin-expected.txt [modify] https://crrev.com/b98895b9d2871d0365f4c5509d8371542df5897b/third_party/WebKit/LayoutTests/resources/idlharness.js [modify] https://crrev.com/b98895b9d2871d0365f4c5509d8371542df5897b/third_party/WebKit/LayoutTests/resources/testharness.js [modify] https://crrev.com/b98895b9d2871d0365f4c5509d8371542df5897b/third_party/WebKit/LayoutTests/resources/webidl2.js [modify] https://crrev.com/b98895b9d2871d0365f4c5509d8371542df5897b/third_party/WebKit/LayoutTests/webaudio/audio-worklet/audio-worklet-node-idl-expected.txt
,
May 4 2018
I think the state we are in now is the right one so closing this. The files inside LayoutTests/external/wpt should be auto-updated and are, and the copies outside are not.
,
May 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e832355cf4019319cb55bebd11fa6533ec5a5143 commit e832355cf4019319cb55bebd11fa6533ec5a5143 Author: Philip Jägenstedt <foolip@chromium.org> Date: Fri May 18 16:48:20 2018 Sync LayoutTests/resources/ with LayoutTests/external/wpt/resources/ Roll in the most recent version of: testharness.js, testdriver.js, idlharness.js and webidl2.js Trailing whitespace in webidl2.js is removed to pass presubmit. This means it is not 100% in sync with upstream. Bug: 685854 Change-Id: Iac8177bd995eb13d545796ef91e1075692db70f0 Reviewed-on: https://chromium-review.googlesource.com/1059152 Reviewed-by: Robert Ma <robertma@chromium.org> Reviewed-by: Philip Jägenstedt <foolip@chromium.org> Commit-Queue: Robert Ma <robertma@chromium.org> Cr-Commit-Position: refs/heads/master@{#559924} [modify] https://crrev.com/e832355cf4019319cb55bebd11fa6533ec5a5143/third_party/WebKit/LayoutTests/fast/dom/custom/svg-use-shadow-tree-expected.txt [modify] https://crrev.com/e832355cf4019319cb55bebd11fa6533ec5a5143/third_party/WebKit/LayoutTests/resources/idlharness.js [modify] https://crrev.com/e832355cf4019319cb55bebd11fa6533ec5a5143/third_party/WebKit/LayoutTests/resources/testdriver.js [modify] https://crrev.com/e832355cf4019319cb55bebd11fa6533ec5a5143/third_party/WebKit/LayoutTests/resources/testharness.js [modify] https://crrev.com/e832355cf4019319cb55bebd11fa6533ec5a5143/third_party/WebKit/LayoutTests/resources/webidl2.js [delete] https://crrev.com/1cfbf9f53aa4eba9ed7fbbbc58e149667a463778/third_party/WebKit/LayoutTests/webaudio/audio-worklet/audio-worklet-node-idl-expected.txt [modify] https://crrev.com/e832355cf4019319cb55bebd11fa6533ec5a5143/third_party/WebKit/LayoutTests/webaudio/audio-worklet/audio-worklet-node-idl.html
,
May 20 2018
|
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by qyears...@chromium.org
, Jan 26 2017