Implement functionality to cc relevant owners.

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

 # Copyright 2014 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.
 
 """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
 
 # Import destination directories (under LayoutTests/imported/).
 WPT_DEST_NAME = 'wpt'
 CSS_DEST_NAME = 'csswg-test'
 
 POLL_DELAY_SECONDS = 300
 
@@ skipping 40 matching lines @@
         elif self.target == 'css':
             import_commitish = self.update(
                 CSS_DEST_NAME,
                 'https://chromium.googlesource.com/external/w3c/csswg-test.git')
         else:
             raise AssertionError("Unsupported target %s" % self.target)
 
         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 = self.parse_directory_owners(data_file)

[qyearsley, 2016/07/28 22:31:18]

To make `parse_directory_owners` more single-purpose (and probably easier to
test), maybe it could take the list of dicts parsed from the file, rather than
the file object -- so this here would be:

     with open(data_file_path) as data_file:
        directory_dict = self.parse_directory_owners(json.load(data_file))

+            self.print_('Gathering directory owners email to CC')
+            _, out = self.run(['git', 'diff', 'master', '--name-only'])
+            email_list = self.parse_imported_test_changes(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'] + email_list)
             self.print_('## Triggering try jobs.')
             for try_bot in try_bots:
                 self.run(['git', 'cl', 'try', '-b', try_bot])
             self.print_('## Waiting for Try Job Results')
             has_failing_results = False
             while True:
                 time.sleep(POLL_DELAY_SECONDS)
                 _, out = self.run(['git', 'cl', 'try-results'])
                 results = self.parse_try_job_results(out)
                 if results.get('Started') or results.get('Scheduled'):
                     continue
                 if results.get('Failures'):
                     has_failing_results = True
                 break
             if has_failing_results:
                 self.print_('## Adding test expectations lines to LayoutTests/TestExpectations')
                 script_path = self.path_from_webkit_base('Tools', 'Scripts', 'update-w3c-test-expectations')
                 self.run([self.host.executable, script_path])
                 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
 
     def parse_args(self, argv):
         parser = argparse.ArgumentParser()
         parser.description = __doc__
         parser.add_argument('-v', '--verbose', action='store_true',
                             help='log what we are doing')
         parser.add_argument('--allow-local-commits', action='store_true',
@@ skipping 188 matching lines @@
             line = line.strip()
             if line[-1] == ':':
                 result_type = line[:-1]
                 sets[result_type] = set()
             elif line.split()[0] == 'Total:':
                 break
             else:
                 sets[result_type].add(line.split()[0])
         return sets
 
+    def parse_imported_test_changes(self, results, directory_dict):

[qyearsley, 2016/07/28 22:31:18]

Some things which may make this function easier to read:
 - A docstring would be helpful
 - The argument `results` isn't very clear
 - The word "parse" in the method name makes it sound like you're return some
structured data based on some string. Which is somewhat true, but I'd say the
main thing that this function does is get email addresses to CC based on owners
of files.

Would it make sense to change this to something like
`get_emails_to_cc(changed_files, directory_to_email_map)` where changed_files is
a list of changed files?

+        email_list = []
+        directories = set()
+        for line in results.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)
+            print test_path
+            test_path = self.fs.dirname(test_path)
+            print test_path

[qyearsley, 2016/07/28 22:31:18]

Remember to remove debugging print lines :-)

+            test_path = test_path.strip('../')
+            if test_path in directory_dict and test_path not in directories:
+                email_list.append('--cc=' + directory_dict[test_path])

[qyearsley, 2016/07/28 22:31:18]

The name email_list sounds like it's just a list of emails, but it's actually a
list of extra args to `git cl upload` to add email addresses to CC.

It may be nice to change this to just return a list of email addresses, since
it's easy to add "--cc=" to each of them later; for example:

  self.check_run(['git', 'cl', 'upload', ...] +
                 ['--cc=' + email for email in email_list])

  

+                directories.add(test_path)
+        return email_list
+
+    def parse_directory_owners(self, data_file):
+        directory_dict = {}
+        data = json.load(data_file)
+        for dict_set in data:
+            if dict_set['notification-email']:
+                directory_dict[dict_set['directory']] = dict_set['notification-email']
+        return directory_dict
+
     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