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

third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.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.
 
 """A script to modify TestExpectations lines based layout test failures in try jobs.
 
 This script outputs a list of test expectation lines to add to a 'TestExpectations' file
 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
 
 
 _log = logging.getLogger(__name__)
 
 
 def main(host, port):
     expectations_file = port.path_to_generic_test_expectations_file()
     expectations_line_adder = W3CExpectationsLineAdder(host)
     issue_number = expectations_line_adder.get_issue_number()
     try_bots = expectations_line_adder.get_try_bots()
     try_jobs = rietveld.latest_try_jobs(issue_number, try_bots, host.web)
     test_expectations = {}
     if not try_jobs:
         print 'No Try Job information was collected.'
         return 1
     for job in try_jobs:
         platform_results = expectations_line_adder.get_failing_results_dict(BuildBot(), job.builder_name, job.build_number)
         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)
 
 
 class W3CExpectationsLineAdder(object):
 
     def __init__(self, host):
         self._host = host
         self.filesystem = host.filesystem
 
@@ skipping 216 matching lines @@
             all_lines += str(line) + '\n'
         all_lines = all_lines[:-1]
         if w3c_comment_line_index == -1:
             file_contents += '\n%s\n' % comment_line
             file_contents += all_lines
         else:
             end_of_comment_line = (file_contents[w3c_comment_line_index:].find('\n')) + w3c_comment_line_index
             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