New issue
Advanced search Search tips

Issue 681406 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[WPT Import/Export] Rename modules in webkitpy/w3c for consistency/clarity

Project Member Reported by qyears...@chromium.org, Jan 15 2017

Issue description

From an email thread in November:

Currently in webkitpy/w3c we have:

deps_updater (nothing to do with DEPS - this imports latest wpt and optionally tries to update baselines)
 -> test_importer (assumes wpt repo is already checked out; only does part of the process)
   -> test_converter (used by test_importer, should be unnecessary at some point)
 -> update_w3c_test_expectations (updates baselines and expectations files, part of auto-import)
sync_wpt (currently this is the initial version of the exporter script)
 -> local_wpt (for operations on upstream copy)
 -> chromium_wpt (for operations on downstream copy)
wpt_bridge_bot (temporary, will be deleted)

Regardless of whether we want to compose the different parts by invoking them as separate scripts or importing them as Python modules, we may want to consider re-organizing. Possible naming:

wpt_sync
 -> wpt_import
   -> wpt_update_expectations
 -> wpt_export
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/57fa7c2cd73975510b7018f8470a67bad5f2e544

commit 57fa7c2cd73975510b7018f8470a67bad5f2e544
Author: qyearsley <qyearsley@chromium.org>
Date: Thu Jan 19 00:30:37 2017

Directly use TestImporter in DepsUpdater instead of invoking script.

Earlier, when importing W3C tests was in a more experimental phase,
import-w3c-tests was useful as a separate script because it offered
more flexibility in how and what was imported.

Now, however, it's easier to update web-platform-tests or csswg-test,
or import new w3c tests, by editing W3CImportExpectations if necessary
and then running update-w3c-deps.

We'd like to rename update-w3c-deps to make the name clearer, but this
is impeded by confusingly also having a script called import-w3c-tests.

BUG= 681406 

Review-Url: https://codereview.chromium.org/2633933002
Cr-Commit-Position: refs/heads/master@{#444565}

[delete] https://crrev.com/a5e49a2b2acf7ef4a79030c06589dde1e2ca3684/third_party/WebKit/Tools/Scripts/import-w3c-tests
[modify] https://crrev.com/57fa7c2cd73975510b7018f8470a67bad5f2e544/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py
[modify] https://crrev.com/57fa7c2cd73975510b7018f8470a67bad5f2e544/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py
[modify] https://crrev.com/57fa7c2cd73975510b7018f8470a67bad5f2e544/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer_unittest.py

That new structure looks cleaner, sgtm. The longer we work on this the more I think we shouldn't have a script named wpt_sync. I think the two scripts, wpt_import and wpt_export will always be invoked separately. So we could even do something like this?

-> wpt_import
  -> wpt_update_expectations
-> wpt_export
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4c7dc89449103d77aad52cdd11a737fad0f01ab9

commit 4c7dc89449103d77aad52cdd11a737fad0f01ab9
Author: qyearsley <qyearsley@chromium.org>
Date: Tue Jan 24 01:39:01 2017

Remove sync_wpt and add wpt-export to replace it.

This CL basically renames "sync-wpt" to "wpt-export" and makes
a couple of additional minor clean-up changes:
 - Remove use of configure_logging from test_importer which is now removed.
 - Remove --no-fetch option from the script.
 - Expand docstring and use docstring as --help description.

BUG= 681406 

Review-Url: https://codereview.chromium.org/2650433002
Cr-Commit-Position: refs/heads/master@{#445581}

[delete] https://crrev.com/144398d6839eae64076550a0e39a9a20ffe43ff6/third_party/WebKit/Tools/Scripts/sync-wpt
[delete] https://crrev.com/144398d6839eae64076550a0e39a9a20ffe43ff6/third_party/WebKit/Tools/Scripts/webkitpy/w3c/sync_wpt.py
[add] https://crrev.com/4c7dc89449103d77aad52cdd11a737fad0f01ab9/third_party/WebKit/Tools/Scripts/wpt-export

Status: Fixed (was: Available)
I think this fixed now
Owner: qyears...@chromium.org
Status: Assigned (was: Fixed)
I'd still like to rename update-w3c-deps to wpt-import, and update-w3c-test-expectations to wpt-update-expectations (and rename the deps_updater and maybe test_importer modules).
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6fe839799893f3d82b5ab6ccfe0e759f8044c091

commit 6fe839799893f3d82b5ab6ccfe0e759f8044c091
Author: qyearsley <qyearsley@chromium.org>
Date: Wed Jan 25 17:10:52 2017

Rename w3c test importer script update-w3c-deps -> wpt-import.

Reason: This name is meant to be clearer and more consistent with related scripts.

Note that I want to call it "wpt-import" instead of "w3c-import" because there are plans to merge csswg-test into wpt, and I want to call it "wpt-import" instead of "wpt-update" because this makes its relationship with "wpt-export" clearer.

BUG= 681406 

Review-Url: https://codereview.chromium.org/2650923003
Cr-Commit-Position: refs/heads/master@{#446044}

[modify] https://crrev.com/6fe839799893f3d82b5ab6ccfe0e759f8044c091/third_party/WebKit/LayoutTests/external/README
[modify] https://crrev.com/6fe839799893f3d82b5ab6ccfe0e759f8044c091/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py
[modify] https://crrev.com/6fe839799893f3d82b5ab6ccfe0e759f8044c091/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater_unittest.py
[rename] https://crrev.com/6fe839799893f3d82b5ab6ccfe0e759f8044c091/third_party/WebKit/Tools/Scripts/wpt-import

Comment 7 by foolip@chromium.org, Jan 25 2017

Yay, it's all coming together, so exciting!
Status: Fixed (was: Assigned)
Components: Blink>Infra>Ecosystem
Components: -Blink>Infra>Predictability

Sign in to add a comment