Create test expectations line automatically from failing test results

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

+# Coppyright 2016 The Chromium Authors. All rights reserved.

[qyearsley, 2016/07/12 23:00:15]

Coppyright -> Copyright

+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+
+from webkitpy.common.checkout.scm.git import Git
+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
+import logging
+
+
+# This script is designed to output a list of Test Expectations lines to be added to TestExpectations.py by
+# retrieving the latest try_job results for the current cl issue connected to the local branch and parsing it
+# to develop a dictionary of dictionaries that contain:
+# {Test Name: {Platform: {'actual': 'foo', 'expected': 'foo', 'bug':'crbug.com/#####'}}}

[qyearsley, 2016/07/12 23:00:15]

Good - module-level descriptions with examples of input/output are very helpful.
In this case, it looks like the format used internally is an implementation
detail though, so it may belong somewhere else rather than in the module-level
docstring.

The usual convention is to put comments like this in a docstring between the
file header and the imports. As with other docstrings, the ideal is to have one
short summary followed by details. For example:

# 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.checkout.scm.git import Git
from webkitpy.common.net.buildbot import BuildBot, Builder
...

+
+_log = logging.getLogger(__name__)
+
+
+TRY_BOTS = ['linux_chromium_rel_ng', 'mac_chromium_rel_ng', 'win_chromium_rel_ng']
+
+
+def main():
+    line_expectations_dict = {}
+    line_list = []
+    counter = 0
+    web = Web()
+    try_jobs_info = get_try_jobs_information(web)
+    while counter < len(TRY_BOTS):
+        try:
+            platform_results_dict = get_try_job_results(try_jobs_info, counter)
+            line_expectations_dict = merge_dicts(line_expectations_dict, platform_results_dict)
+            counter += 1
+        except IndexError:
+            print "no tryjob info was collected"
+            return 1
+    line_expectations_dict = update_platforms(line_expectations_dict)
+    line_list = create_line_list(line_expectations_dict)
+    return line_list
+
+# 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.
+
+
+def get_try_jobs_information(web):
+    git = Git('.')
+    issue_number = git.get_issue_number()
+    info = rietveld.latest_try_jobs(issue_number, TRY_BOTS, web)
+    return info
+
+
+def _generate_results_dict(layout_test_list):
+    test_dict = {}
+    builder_list = BuilderList(Builders_list_dict)
+    builder_name = layout_test_list.builder_name()
+    platform = builder_list.port_name_for_builder_name(builder_name)
+    dash = platform.find("-")
+    if dash != -1:
+        platform = platform[dash + 1:].capitalize()
+    for result in layout_test_list.didnt_run_as_expected_results():
+        test_dict[result.test_name()] = {platform: {'expected': result.expected_results(), 'actual': result.actual_results(), 'bug': 'crbug.com/626703'}}
+    return test_dict
+
+
+def get_try_job_results(try_jobs_data, index):
+    builder_name = try_jobs_data[index][0]
+    build_number = try_jobs_data[index][1]
+    builder = buildbot.Builder(builder_name, buildbot.BuildBot())
+    build = buildbot.Build(builder, build_number)
+    layout_test_results = builder.fetch_layout_test_results(build.results_url())
+    failing_results_dict = _generate_results_dict(layout_test_results)
+    return failing_results_dict
+
+
+def _remove_duplicates(failing_paths):
+    seen = set()
+    new_list = []
+    for dictionary in failing_paths:
+        path = tuple(dictionary.items())
+    if path not in seen:
+        seen.add(path)
+        new_list.append(dict(path))
+    return new_list
+
+
+def merge_dicts(final, temp, path=None):
+    if path is None:
+        path = []
+    for key in temp:
+        if key in final:
+            if (isinstance(final[key], dict)) and isinstance(temp[key], dict):
+                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
+
+# note this function may break if there are more than 3 inner_keys.
+# as of right now, the inner_keys should only be [win, linux, mac]
+
+
+def update_platforms(final):
+    for value in final.values():
+        platform = [set()]
+        inner_keys = value.keys()
+        tuple_result = {}
+        for i, item in enumerate(inner_keys):
+            if (i + 1) < len(inner_keys):
+                if value[item] == value[inner_keys[i + 1]]:
+                    tuple_result = value[item]
+                    platform[0].add(item)
+                    if (i + 2) == len(inner_keys):
+                        platform[0].add(inner_keys[i + 1])
+                elif (i == 0) and (value[item] == value[inner_keys[len(inner_keys) - 1]]):
+                    platform[0].add(item)
+                    platform[0].add(inner_keys[len(inner_keys) - 1])
+        if len(platform[0]) == 0:
+            continue
+        else:
+            value[tuple(platform[0])] = tuple_result
+            for i in platform[0]:
+                value.pop(i)
+    return final

[qyearsley, 2016/07/12 23:00:15]

On first look, I'm having difficulty understanding what this function does -- I
think we'll want to find a way to re-factor this function and extract helper
functions to make this easier to understand.

As a general rule, easy-to-understand functions have the following properties:
 - function name, argument names and variable names are descriptive and clear
 - less than 15 lines
 - less than 3 levels of indentation

+
+
+def get_expectations(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(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 = get_expectations(value[key2])
+            line = '%s [ %s ] %s [ %s ]' % (bug[0], ' '.join(platform), test_name, ' '.join(expectations))
+            line_list.append(line)
+    return line_list