Create test expectations line automatically from failing test results

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.
+
+Platform_Results_dict structure:
+ {Test Name(s): { Platform(s): {'actual': ACTUAL_RESULTS, 'expected': EXPECTED_RESULTS, 'bug': BUG_URL}}}
+ each Test Name and Platform are keys in their respective dictionaries.
+"""
+
+import logging
+
+from webkitpy.common.checkout.scm.git import Git
+from webkitpy.common.host import Host
+from webkitpy.common.net import buildbot
+from webkitpy.common.net import rietveld
+from webkitpy.common.net.web import Web
+from webkitpy.layout_tests.builder_list import BuilderList
+from webkitpy.common.config.builders import BUILDERS as Builders_list_dict

[qyearsley, 2016/07/18 18:31:42]

You can keep the name BUILDERS, since that's simpler than renaming it as
Builders_list_dict here.

[dcampb, 2016/07/18 20:50:04]

I have removed this import as well as builder_list. Refactored the script to use
the builder_list on the host object. 

+
+
+_log = logging.getLogger(__name__)
+
+
+TRY_BOTS = ['linux_chromium_rel_ng', 'mac_chromium_rel_ng', 'win_chromium_rel_ng']
+
+
+def main():
+    host = Host()
+    port = host.port_factory.get()
+    expectations_file = port.path_to_generic_test_expectations_file()
+    counter = 0
+    builderlist = BuilderList(Builders_list_dict)

[qyearsley, 2016/07/18 18:31:42]

builderlist -> builder_list

+    git = Git('.')
+    web = Web()
+    expectations_line_adder = W3CExpectationsLineAdder(git, web, builderlist)
+    issue_number = expectations_line_adder._git.get_issue_number()

[qyearsley, 2016/07/18 18:31:42]

Here a private attribute of another object is accessed -- you can either move
some logic from main into a method in W3CExpectationsLineAdder, or if the logic
is kept in main, then git should be marked as public.

[dcampb, 2016/07/18 20:50:04]

I moved this line and the line 42 into their own functions that return the issue
number and the try_bots names. 

+    try_bots = expectations_line_adder._builder_list.all_try_builder_names()
+    try_jobs_info = expectations_line_adder.get_try_jobs_information(issue_number, try_bots)
+    line_expectations_dict = {}
+    while counter < len(try_bots):
+        try:
+            builder_name = try_jobs_info[counter][0]
+            build_number = try_jobs_info[counter][1]
+            builder = buildbot.Builder(builder_name, buildbot.BuildBot())
+            build = buildbot.Build(builder, build_number)
+            platform_results_dict = expectations_line_adder.get_failing_results_dict(builder, build)
+            line_expectations_dict = expectations_line_adder.merge_dicts(line_expectations_dict, platform_results_dict)
+            counter += 1
+        except IndexError:
+            print "no tryjob info was collected"

[qyearsley, 2016/07/18 18:31:42]

You could add capitalization and punctuation to this message, and put a space in
"try job".

+            return 1
+    for platform_results_dicts in line_expectations_dict.values():
+        platform_results_dicts = expectations_line_adder.merge_same_valued_keys(platform_results_dicts)
+    line_list = expectations_line_adder.create_line_list(line_expectations_dict)
+    expectations_line_adder.write_to_testexpectations(host, expectations_file, line_list)
+
+
+class W3CExpectationsLineAdder(object):

[qyearsley, 2016/07/18 18:31:42]

Formatting note: Usually there's a blank line before the first method of a
class.

+    def __init__(self, git, web, builderlist):

[qyearsley, 2016/07/18 18:31:42]

builderlist -> builder_list

+        self._git = git
+        self._web = web
+        self._builder_list = builderlist
+
+    # This function will return only tryjobs with failing results.
+    # If no failing results, try_job info will not be returned. If one try server
+    # has two try jobs, it will return both.

[qyearsley, 2016/07/18 18:31:42]

This comment can go inside a docstring, and updated when that CL that changes
latest_try_jobs is landed.

+    def get_try_jobs_information(self, issue_number, try_bots):
+        info = rietveld.latest_try_jobs(issue_number, try_bots, self._web)
+        return info

[qyearsley, 2016/07/18 18:31:41]

The variable info doesn't provide any extra information here; you can just
write:

return rietveld.latest_try_jobs(issue_number, try_bots, self._web)

+
+    def _generate_results_dict(self, platform, result_list):
+        test_dict = {}
+        dash = platform.find("-")
+        if dash != -1:

[qyearsley, 2016/07/18 18:31:42]

To find if a character exists in a string you can also use the keyword `in`:

if '-' in platform:
    ...

[dcampb, 2016/07/18 20:50:04]

done

+            platform = platform[dash + 1:].capitalize()
+        for result in result_list:
+            test_dict[result.test_name()] = {platform: {'expected': result.expected_results(), 'actual': result.actual_results(), 'bug': 'crbug.com/626703'}}

[qyearsley, 2016/07/18 18:31:42]

This line is a bit long; reformatting it may make it more readable.

[dcampb, 2016/07/18 20:50:04]

done

+        return test_dict
+
+    def get_failing_results_dict(self, builder, build):
+        layout_test_results = builder.fetch_layout_test_results(build.results_url())
+        builder_name = layout_test_results.builder_name()
+        platform = self._builder_list.port_name_for_builder_name(builder_name)
+        result_list = layout_test_results.didnt_run_as_expected_results()
+        failing_results_dict = self._generate_results_dict(platform, result_list)
+        return failing_results_dict
+
+    def merge_dicts(self, final, temp, path=None):
+        path = path or []
+        for key in temp:
+            if key in final:
+                if (isinstance(final[key], dict)) and isinstance(temp[key], dict):
+                    self.merge_dicts(final[key], temp[key], path + [str(key)])
+                elif final[key] == temp[key]:
+                    pass
+                else:
+                    raise Exception('conflict at %s' % '.'.join(path))
+            else:
+                final[key] = temp[key]
+        return final
+
+    def merge_same_valued_keys(self, dictionary):
+        matching_value_keys = set()
+        keys = dictionary.keys()
+        isLastItem = False
+        for index, item in enumerate(keys):
+            if isLastItem:
+                break
+            for i in range(index + 1, len(keys)):
+                next_item = keys[i]
+                if dictionary[item] == dictionary[next_item]:
+                    matching_value_keys.update([item, next_item])
+                    dictionary[tuple(matching_value_keys)] = dictionary[item]
+                    isLastItem = next_item == keys[-1]
+                    del dictionary[item]
+                    del dictionary[next_item]
+        return dictionary
+
+    def get_expectations(self, results):
+        expectations = []
+        failure_expectations = ['TEXT', 'FAIL', 'IMAGE+TEXT', 'IMAGE']
+        pass_crash_timeout = ['TIMEOUT', 'CRASH', 'PASS']
+        if results['expected'] in pass_crash_timeout and results['actual'] in failure_expectations:
+            expectations.append('Failure')
+        if results['expected'] in failure_expectations and results['actual'] in pass_crash_timeout:
+            expectations.append(results['actual'].capitalize())
+        if results['expected'] in pass_crash_timeout and results['actual'] in pass_crash_timeout:
+            expectations.append(results['actual'].capitalize())
+            expectations.append(results['expected'].capitalize())
+        return expectations
+
+    def create_line_list(self, dictionary):
+        line_list = []
+        for key, value in dictionary.iteritems():
+            test_name = key
+            for key2 in value:
+                platform = []
+                bug = []
+                expectations = []
+                if isinstance(key2, tuple):
+                    platform = list(key2)
+                else:
+                    platform.append(key2)
+                bug.append(value[key2]['bug'])
+                expectations = self.get_expectations(value[key2])
+                line = '%s [ %s ] %s [ %s ]' % (bug[0], ' '.join(platform), test_name, ' '.join(expectations))
+                line_list.append(str(line))
+        return line_list
+
+    def write_to_testexpectations(self, host, path, line_list):
+        file_contents = host.filesystem.read_text_file(path)
+        w3c_comment_line_index = file_contents.find("# Tests added from W3C auto import bot")
+        all_lines = ""
+        for line in line_list:
+            all_lines += str(line) + "\n"
+        all_lines = all_lines[:-1]
+        if w3c_comment_line_index == -1:
+            file_contents += "\n\n# Tests added from W3C auto import bot\n"
+            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)