Invoke webkit-patch in update-w3c-test-expectations

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

Index: third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.py
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.py b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.py
index 2a5ef4d82e380518af5a96f459fba4c41b05ec95..e7a975bd3b5ccce79dbefb7d5ec3147f5be58546 100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.py
@@ -11,6 +11,8 @@ by retrieving the try job results for the current CL.
 import logging
 
 from webkitpy.common.net.buildbot import BuildBot
+from webkitpy.w3c.test_parser import TestParser
+from webkitpy.common.webkit_finder import WebKitFinder

[qyearsley, 2016/07/29 20:59:44]

Remember to sort the new imports

[dcampb, 2016/08/02 18:02:12]

done

 from webkitpy.common.net import rietveld
 
 
@@ -32,6 +34,7 @@ def main(host, port):
         test_expectations = expectations_line_adder.merge_dicts(test_expectations, platform_results)
     for test_name, platform_result in test_expectations.iteritems():
         test_expectations[test_name] = expectations_line_adder.merge_same_valued_keys(platform_result)
+    test_expectations = expectations_line_adder.get_expected_txt_files(test_expectations)
     test_expectation_lines = expectations_line_adder.create_line_list(test_expectations)
     expectations_line_adder.write_to_test_expectations(host, expectations_file, test_expectation_lines)
 
@@ -268,3 +271,49 @@ class W3CExpectationsLineAdder(object):
             new_data = file_contents[: end_of_comment_line + 1] + all_lines + file_contents[end_of_comment_line:]
             file_contents = new_data
         host.filesystem.write_text_file(path, file_contents)
+
+    def get_expected_txt_files(self, tests_results):
+        """Gets -expected.txt files.
+
+        Invokes WebKit-Patch RebaselineFromTryJobs in order

[qyearsley, 2016/07/29 20:59:45]

WebKit-Patch RebaselineFromTryJobs -> webkit-patch rebaseline-from-try-jobs

[dcampb, 2016/08/01 17:26:02]

done

+        to download new -expected.txt files for testharness.js
+        tests that did not Crash or Timeout. Afterwords, the platform-

[qyearsley, 2016/07/29 20:59:45]

Afterwards -> Afterwards
or "Then, ..."

[dcampb, 2016/08/01 17:26:02]

done

+        specific test is removed from the overall failure test dictionary.
+
+        Args:
+            tests_results: A dictionary that maps test name to platforms to
+            test results.
+
+        Returns:
+            An updated tests_results dictionary without the platform-specific test-
+            harness.js tests that required new baselines to be downloaded from
+            WebKit-Patch RebaselineFromTryJobs.

[qyearsley, 2016/07/29 20:59:44]

WebKit-Patch RebaselineFromTryJobs -> webkit-patch rebaseline-from-try-jobs

[dcampb, 2016/08/01 17:26:02]

done

+        """
+        finder = WebKitFinder(self._host.filesystem)
+        root = finder.chromium_base()
+        layout_tests_relative_path = self._host.filesystem.relpath(finder.layout_tests_dir(), root)
+        webkit_base = finder.webkit_base()
+        crash_or_timeout = ['CRASH', 'TIMEOUT']

[qyearsley, 2016/07/29 20:59:45]

The fewer variables there are in a function, then easier the function is to
understand. So, if some value is used in one place and it's not a very
complicated expression, then the value can be used directly, e.g.

  webkit_patch = self._host.filesystem.join(root, finder.webkit_base(), ...)

  tests_results[test_path][platform]['actual'] not in ['CRASH', 'TIMEOUT']:

Exceptions: If the variable is used in lots of places, or if it gives a name to
a complex expression, then it can make the function easier to read.

[dcampb, 2016/08/01 17:26:02]

noted

[dcampb, 2016/08/01 17:26:02]

noted

+        tests_to_rebaseline = []
+        tests = self._host.executive.run_command(['git', 'diff', 'master', '--name-only'])
+        for test_dir in tests.splitlines():
+            absolute_path = self._host.filesystem.join(root, test_dir)
+            test_parser = TestParser(None, absolute_path)
+            is_js_test = test_parser.is_jstest()
+            if is_js_test:
+                test_path = self._host.filesystem.relpath(test_dir, layout_tests_relative_path)
+                try:
+                    for platform in tests_results[test_path]:
+                        if tests_results[test_path][platform]['actual'] not in crash_or_timeout:
+                            del tests_results[test_path][platform]
+                            tests_to_rebaseline.append(test_path)
+                # TODO(dcampb): Remove once trybots are all running layout
+                # Was used for testing

[qyearsley, 2016/07/29 20:59:45]

I'm not quite sure what this comment means now

[dcampb, 2016/08/01 17:26:02]

removed it now. It was because I had some newly imported tests on my local
machine that weren't run on the bots so their keys weren't in the
layouttestsresults. 

+                except KeyError:
+                    pass
+        if tests_to_rebaseline:
+            webkit_patch = self._host.filesystem.join(root, webkit_base, finder.path_to_script('webkit-patch'))
+            self._host.executive.run_command(['python', webkit_patch,
+                                              'rebaseline-from-try-jobs', '-v',
+                                              ' '.join(tests_to_rebaseline)])

[qyearsley, 2016/07/29 20:59:45]

Each of the tests is a separate arg, so the input to run_command can be:

    ['python', webkit_patch, 'rebaseline-from-try-jobs', '-v'] +
tests_to_rebaseline

[dcampb, 2016/08/01 17:26:02]

tests_to_rebaseline is a of test names. This way I can pass all of the tests as
a string. Its the only way I thought of to call rebaseline-from-try-jobs without
having a loop. 

[qyearsley, 2016/08/01 18:33:18]

Concatenating the start of the argument list with the list of test names with +
should still work -- Remember, list concatenation with + also works, e.g.
  >>> ['foo', 'bar'] + ['a', 'b', 'c']
  ['foo', 'bar', 'a', 'b', 'c']
And if that's being interpreted as a command to run, then `foo bar a b c` would
be run.

[dcampb, 2016/08/02 18:02:12]

okay, I see. Changes made

+        return tests_results