Implement functionality to cc relevant owners.

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

Index: third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py
index 5e43fd1fadd0a88a907116f68af3efdc160fc045..dca27b0dcaf943bc9f298061bfc80eb698878ca9 100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py
@@ -5,6 +5,7 @@
 """Pull latest revisions of a W3C test repo and make a local commit."""
 
 import argparse
+import json
 import time
 
 from webkitpy.common.webkit_finder import WebKitFinder
@@ -65,8 +66,14 @@ class DepsUpdater(object):
         self.commit_changes_if_needed(chromium_commitish, import_commitish)
         if self.auto_update:
             try_bots = self.host.builders.all_try_builder_names()
+            data_file_path = self.finder.path_from_webkit_base('Tools', 'Scripts', 'webkitpy', 'w3c', 'directory_owners.json')
+            with open(data_file_path) as data_file:
+                directory_dict = parse_directory_owners(json.load(data_file))
+            self.print_('Gathering directory owners email to CC')

[qyearsley, 2016/07/28 23:46:33]

To be consistent with other lines that are printed by deps_updater, we could
prepend this with "##" and add a period.

I actually don't know why "## " is prepended to the printed lines, besides to
make them stand out more, I guess, but we may as well be consistent.

+            _, out = self.run(['git', 'diff', 'master', '--name-only'])
+            email_list = self.generate_email_list(out, directory_dict)
             self.print_('## Uploading change list.')
-            self.check_run(['git', 'cl', 'upload', '-f', '-m', 'W3C auto test importer'])
+            self.check_run(['git', 'cl', 'upload', '-f', '-m', 'W3C auto test importer'] + ['--cc' + email for email in email_list])
             self.print_('## Triggering try jobs.')
             for try_bot in try_bots:
                 self.run(['git', 'cl', 'try', '-b', try_bot])
@@ -88,7 +95,7 @@ class DepsUpdater(object):
                 self.check_run(['git', 'commit', '-a', '-m', '\'Modified Test Expectations from W3C Test Auto-roller\''])
                 self.check_run(['git', 'cl', 'upload', '-m', '\'Wrote lines to TestExpectations\''])
             else:
-                self.print_('No Failures, committing patch.')
+                self.print_('No Failures, landing patch.')
             quit()
             self.run(['git', 'cl', 'land', '-f'])
         return 0
@@ -297,8 +304,43 @@ class DepsUpdater(object):
                 sets[result_type].add(line.split()[0])
         return sets
 
+    def generate_email_list(self, changed_files, directory_dict):
+        """Generates a list of emails to be cc'd for current import
+
+           Turns the output from git cl try-results into a usable format.

[qyearsley, 2016/07/28 23:46:33]

Nit: The body of the docstring should be lined up with the start of the """,
rather than with the First sentence.

+
+           Args:
+               changed_files: The files that have been modified at some
+                              point in the import process.
+               directory_dict: A mapping of directories in imported to the
+                               email address of the point of contact of it

[qyearsley, 2016/07/28 23:46:33]

Nit: missing period. Also, you can probably remove "of it".

+
+           Returns:
+               A list of the email addresses to be notified for the current
+               import.
+        """
+        email_list = []
+        directories = set()
+        for line in changed_files.splitlines():
+            layout_tests_relative_path = self.fs.relpath(self.finder.layout_tests_dir(), self.finder.chromium_base())
+            test_path = self.fs.relpath(line, layout_tests_relative_path)
+            test_path = self.fs.dirname(test_path)
+            test_path = test_path.strip('../')

[qyearsley, 2016/07/28 23:46:33]

Side note: Be careful with strip, it probably doesn't behave quite as you expect
it to. The argument to strip is a "character set" and as much as possible is
stripped off. For example:

  >>> '../../.././foo/x..'.strip('./')
  'foo/x'

+            if test_path in directory_dict and test_path not in directories:
+                email_list.append(directory_dict[test_path])
+                directories.add(test_path)
+        return email_list
+
     def check_run(self, command):
         return_code, out = self.run(command)
         if return_code:
             raise Exception('%s failed with exit code %d.' % ' '.join(command), return_code)
         return out
+
+
+def parse_directory_owners(decoded_data_file):
+    directory_dict = {}
+    for dict_set in decoded_data_file:
+        if dict_set['notification-email']:
+            directory_dict[dict_set['directory']] = dict_set['notification-email']
+    return directory_dict

[qyearsley, 2016/07/28 23:46:33]

BTW, if you wanted to keep this in the class (for organizational purposes), you
can make it a static method (with the @staticmethod decorator), and then it
doesn't get a self argument.

This is no difference in behavior, just a difference in organization.