[WPT Export] "Import all remaining cookie tests into WPT" change not exported, then reverted |
||
Issue descriptionhttps://chromium-review.googlesource.com/c/chromium/src/+/1021577 landed on Aug 16, with a lot of changes in third_party/WebKit/LayoutTests/external/wpt/. https://ci.chromium.org/buildbot/chromium.infra.cron/wpt-exporter/96310 is the last wpt-exporter build before the commit, and passed. https://ci.chromium.org/buildbot/chromium.infra.cron/wpt-exporter/96311 is the first wpt-exporter build including the commit, and failed. However, "cookie" doesn't appear in the log, the failures was because https://chromium.googlesource.com/chromium/src/+/c3dade0056 (the immediately previous commit) couldn't be applied. The exporter kept failing for the same reason until https://ci.chromium.org/buildbot/chromium.infra.cron/wpt-exporter/96318, and then began passing in https://ci.chromium.org/buildbot/chromium.infra.cron/wpt-exporter/96319. "cookie" still doesn't appear at the log then. https://chromium-review.googlesource.com/c/chromium/src/+/1180077 is the import that ended up reverting the changes. The same fate probably awaits https://chromium-review.googlesource.com/c/chromium/src/+/1196543 given that no export PR has been created.
,
Aug 30
robertma@, I have a fix in progress.
,
Aug 30
,
Aug 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/34d106c9b3aae447635559612a96d718b61d2beb commit 34d106c9b3aae447635559612a96d718b61d2beb Author: Philip Jägenstedt <foolip@chromium.org> Date: Thu Aug 30 14:49:57 2018 [WPT Export] Allow commits with "Import" in subject to be exported These conditions are no longer needed, as real import commits have 'No-Export: true' in their commit message. Bug: 879128 Change-Id: I829373c595931d8b221066b4edbb55bcb4db8cb7 Reviewed-on: https://chromium-review.googlesource.com/1196447 Reviewed-by: Robert Ma <robertma@chromium.org> Commit-Queue: Robert Ma <robertma@chromium.org> Cr-Commit-Position: refs/heads/master@{#587571} [modify] https://crrev.com/34d106c9b3aae447635559612a96d718b61d2beb/third_party/blink/tools/blinkpy/w3c/chromium_exportable_commits.py [modify] https://crrev.com/34d106c9b3aae447635559612a96d718b61d2beb/third_party/blink/tools/blinkpy/w3c/chromium_exportable_commits_unittest.py [modify] https://crrev.com/34d106c9b3aae447635559612a96d718b61d2beb/third_party/blink/tools/blinkpy/w3c/gerrit.py [modify] https://crrev.com/34d106c9b3aae447635559612a96d718b61d2beb/third_party/blink/tools/blinkpy/w3c/gerrit_unittest.py
,
Aug 30
https://ci.chromium.org/buildbot/chromium.infra.cron/wpt-exporter/98525 is the first export to run after these changes.
,
Aug 30
Alright! It worked: https://github.com/web-platform-tests/wpt/pull/12762
,
Aug 31
From mkwst@ on chat: > https://chromium-review.googlesource.com/c/chromium/src/+/1196543 is in a weird state where it looks like the importer bot reimported the old CL (in https://chromium-review.googlesource.com/c/chromium/src/+/1196908), but didn't push it to WPT. > So I can't land the chromium patch, because it's already landed. Which means the bot won't auto-land the patch against WPT. Which possibly means that we should just land the PR against WPT? And hope that the bot doesn't remove things in the meantime? What's happened is that when I fixed the definition of what is exportable, that affected not just the exporter but also the importer. The importer, to avoid reverting changes which have not yet been exported, applies all such commit on top of a clean import. From the commit message: > With Chromium commits locally applied on WPT: > 29a89cda66 "Import all remaining cookie tests into WPT" I didn't predict this, but it is the intended behavior of import. I've applied https://chromium-review.googlesource.com/c/chromium/src/+/1196543 to the commit before https://chromium-review.googlesource.com/c/chromium/src/+/1021577 was landed. The only difference is that these files were added and still exist on master: third_party/WebKit/LayoutTests/external/wpt/cookies/http-state/chromium-tests-expected.txt third_party/WebKit/LayoutTests/external/wpt/cookies/http-state/domain-tests-expected.txt third_party/WebKit/LayoutTests/external/wpt/cookies/http-state/mozilla-tests-expected.txt third_party/WebKit/LayoutTests/external/wpt/cookies/http-state/name-tests-expected.txt third_party/WebKit/LayoutTests/external/wpt/cookies/http-state/ordering-tests-expected.txt third_party/WebKit/LayoutTests/external/wpt/cookies/http-state/path-tests-expected.txt third_party/WebKit/LayoutTests/external/wpt/cookies/http-state/value-tests-expected.txt Since those are the only differences, the export PRs are identical. I'll get https://github.com/web-platform-tests/wpt/pull/12762 merged.
,
Aug 31
I listed all commits containing "Import" in LayoutTests/external/wpt (going back to 2017-01-17) except for proper wpt import and with some ad-hoc changes test them for exportability using the updated definition, but ignoring the _is_commit_exported condition. These are the ones that were listed: Import all remaining cookie tests into WPT 29a89cda6627b5e4c4b1682501b84c9022dccd1a Import MediaStreamTrack-getSettings and add expectation 2e3125b7dff512325a9a704b8d9f36483422d59c Import //fetch from Web Platform Tests. 09fdd7696d0f20abd9984f23e0fd1aadd5e2f93b Import web-platform-tests/editing/. a2d8556be5121f4c6640443f5a5c549ba450c770 The first one is the CL this bug is about. The remaining three are all actually imports from wpt into Chromium that depended on beginning with "Import" to not be exported. In other words, it appears as though "Import all remaining cookie tests into WPT" was the first and last case of data loss due to this. The instructions at https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_platform_tests.md#Enabling-import-for-a-new-directory suggest depending on auto-import for importing new directories, so I'm not too concerned that people will try to use "Import foo" commits without No-Export going forward. |
||
►
Sign in to add a comment |
||
Comment 1 by foolip@chromium.org
, Aug 30