Script for exporting WPT

third_party/WebKit/Tools/Scripts/webkitpy/w3c/chromium_wpt.py

+# Copyright 2016 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+CR_WPT_DIR = 'third_party/WebKit/LayoutTests/imported/wpt/'

[qyearsley, 2016/11/04 17:40:32]

Just so it's explicit and clear for future readers, we could avoid use the full
word Chromium rather than Cr in names -- e.g.

 CR_WPT_DIR -> CHROMIUM_WPT_DIR
 cr_commits -> chromium_commits
 cr_commits_since -> chromium_commits_since

[qyearsley, 2016/11/04 17:42:04]

Scratch out the word "avoid" in this comment...

+
+from webkitpy.common.memoized import memoized
+from webkitpy.common.webkit_finder import WebKitFinder
+
+
+class ChromiumWPT(object):
+    """This is a utility class for interacting with the Chromium git tree
+    for use cases relating to the Web Platform Tests.
+    """
+
+    def __init__(self, host):
+        self.host = host
+
+    def exportable_commits_since(self, sha):

[qyearsley, 2016/11/04 17:40:32]

`chromium_commit` might be clearer than `sha`, since this would express that
it's expected to be a commit in the Chromium repo.

+        cr_commits = self.cr_commits_since(sha)
+
+        def is_exportable(sha):
+            return self.has_changes_in_wpt(sha) and not self.is_import_commit(sha)
+
+        return filter(is_exportable, cr_commits)
+
+    def is_import_commit(self, sha):
+        return self.subject(sha).startswith('Import wpt@')
+
+    def has_changes_in_wpt(self, sha):
+        """Detects if a Chromium revision has modified files in the WPT directory."""
+
+        assert sha

[qyearsley, 2016/11/04 17:40:32]

Just a random note: I think that this use of assert is good :-) I think I don't
normally see/use as many asserts as I could.

+        files = self.host.executive.run_command([
+            'git', 'diff-tree', '--no-commit-id',
+            '--name-only', '-r', sha
+        ]).splitlines()
+
+        # TODO(jeffcarp): Use DepsUpdater.is_baseline
+        return any([f.startswith(CR_WPT_DIR) and '-expected' not in f for f in files])

[qyearsley, 2016/11/04 17:40:32]

Python any() and all() can take any iterable including generator expressions, so
in expressions like this, the square brackets are optional; this can also be:

  return any(f.startswith(CR_WPT_DIR) and '-expected' not in f for f in files)

+
+    def cr_commits_since(self, sha):
+        return self.host.executive.run_command([
+            'git', 'rev-list', '--reverse', '{}..HEAD'.format(sha)
+        ]).splitlines()
+
+    def subject(self, sha):
+        return self.host.executive.run_command([
+            'git', 'show', '--format=%s', '--no-patch', sha
+        ])
+
+    def commit_position(self, sha):
+        return self.host.executive.run_command([
+            'git', 'footers', '--position', sha
+        ])
+
+    def message(self, sha):
+        """This returns both the subject and body of a given revision."""

[qyearsley, 2016/11/04 17:40:32]

Usually docstrings start with just the verb in third-person singular, e.g.

  """Returns a string with the subject and body for a given commit."""

+        return self.host.executive.run_command([
+            'git', 'show', '--format=%B', '--no-patch', sha
+        ])
+
+    def format_patch(self, sha):
+        """Get patch but only for files in LayoutTests/imported/wpt"""

[qyearsley, 2016/11/04 17:40:32]

Likewise:

  """Makes a patch with just changes in files in the WPT directory in a given
commit."""

+        # TODO(jeffcarp): do not include expectations files
+        return self.host.executive.run_command([
+            'git', 'format-patch', '-1', '--stdout',
+            sha, self.absolute_chromium_wpt_dir()
+        ])
+
+    @memoized
+    def absolute_chromium_wpt_dir(self):
+        finder = WebKitFinder(self.host.filesystem)
+        return finder.path_from_webkit_base('LayoutTests', 'imported', 'wpt')