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.
+"""
+
+import logging
+
+from webkitpy.common.net import buildbot
+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_info = expectations_line_adder.get_try_jobs_information(issue_number, try_bots)
+    line_expectations_dict = {}
+    if not try_jobs_info:
+        print 'No Try Job information was collected.'
+        return 1
+    for try_job in try_jobs_info:
+        builder_name = try_job[0]
+        build_number = try_job[1]
+        builder = buildbot.Builder(builder_name, expectations_line_adder.get_build_bot)

[qyearsley, 2016/07/19 17:59:47]

Note: As-is, get_build_bot is a function, not a BuildBot object.

+        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)
+    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_test_expectations(host, expectations_file, line_list)
+
+
+class W3CExpectationsLineAdder(object):
+
+    def __init__(self, host):
+        self._host = host
+
+    def get_build_bot(self):
+        return self._host.buildbot

[qyearsley, 2016/07/19 17:59:48]

This method could be removed, and line 33 above could be changed to:

    builder = buildbot.Builder(builder_name, host.buildbot)

+
+    def get_try_jobs_information(self, issue_number, try_bots):

[qyearsley, 2016/07/19 17:59:48]

This method could also be called latest_try_jobs -- or it could be removed, and
line 25 above could be changed to:

    try_jobs = rietveld.latest_try_jobs(issue_number, try_bots, host.web)

+        return rietveld.latest_try_jobs(issue_number, try_bots, self._host.web)
+
+    def get_issue_number(self):
+        return self._host._scm.get_issue_number()
+
+    def get_try_bots(self):
+        return self._host.builders.all_try_builder_names()
+
+    def _generate_results_dict(self, platform, result_list):
+        test_dict = {}
+        if '-' in platform:
+            platform = platform[platform.find('-') + 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'
+                }}
+        return test_dict
+
+    def get_failing_results_dict(self, builder, build):
+        """ Returns a nested dict of failing test results.

[qyearsley, 2016/07/19 17:59:48]

Extra space before "Returns"

+
+        Retrieves a full list oflayout test results from a builder result url. Collects

[qyearsley, 2016/07/19 17:59:48]

"oflayout" -> "of layout"
"url" -> "URL"

+        the builder name, platform and a list of tests that did not run as expected.
+
+        Args:
+            builder: A Builder object.
+            build: A Build object.
+
+        Returns:
+            A dictionary with the structure: {
+            'key': {
+                'expected': 'TIMEOUT', 'actual': 'CRASH', 'bug': 'crbug.com/11111', ...}
+            }

[qyearsley, 2016/07/19 17:59:47]

Could be reformatted as:


            A dictionary with the structure: {
                'key': {
                    'expected': 'TIMEOUT',
                    'actual': 'CRASH',
                    'bug': 'crbug.com/11111',
                }
            }

+        """
+        layout_test_results = builder.fetch_layout_test_results(build.results_url())
+        builder_name = layout_test_results.builder_name()
+        platform = self._host.builders.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):
+        """Merges keys in dictionary with same value.
+
+        Traverses through a dict and compares the values of keys to one another.
+        If the values match, the keys are combined to a tuple and the previous keys
+        are removed from the dict.
+
+        Args:
+            dictionary: A dictionary with a dictionary as the value.
+
+        Returns:
+            A dictionary with updated keys to reflect matching values of keys.
+            Example: {
+                'one': {'foo': 'bar'},
+                'two': {'foo': 'bar'},
+                'three': {'foo': bar'}
+                }     is converted to

[qyearsley, 2016/07/19 17:59:48]

I'd dedent this line so that the closing curly brace is lined up with the start
of the line that has the opening curly brace.

+                {('one', 'two', 'three'): {'foo': 'bar'}}
+        """
+        matching_value_keys = set()
+        keys = dictionary.keys()
+        isLastItem = False

[qyearsley, 2016/07/19 17:59:48]

Variable names should be lower_with_underscores.

+        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):

[qyearsley, 2016/07/19 17:59:48]

This method could also use a docstring -- Is results a dict with results for one
test? If so, it's worth noting that results['actual'] can be a space-delimited
string with results for different runs, e.g. "FAIL PASS".

It may also be worth mentioning that the return value is a list which should
have one or more strings with the first letter capitalized, e.g. ["Failure"]

+        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):
+        """Creates list of test expectations lines.
+
+        Traverses through a dictionary and parses the value to create a test
+        expectations line per key.
+
+        Args:
+            dictionary: A dictionary with the format {

[qyearsley, 2016/07/19 17:59:48]

We should think of a name for the argument that describes the contents or
meaning of the input dictionary.

Here, this dictionary contains results for all platforms for all failed tests.
Obviously, you can't describe everything about it in the argument name, and just
"results" is also very vague.

Maybe merged_results?

[dcampb, 2016/07/20 03:29:43]

agreed, I used merged_results.

+                'test_name': {
+                    'platform': {
+                        'expected: 'PASS',
+                        'actual': 'FAIL',
+                        'bug': 'crbug.com/11111'
+                        }
+                    }
+                }

[qyearsley, 2016/07/19 17:59:47]

Indentation for closing braces usually lines up with the start of the line where
the opening brace is. For example:

            dictionary: A dictionary with the format {
                'test_name': {
                    'platform': {
                        'expected: 'PASS',
                        'actual': 'FAIL',
                        'bug': 'crbug.com/11111'
                    }
                }
            }

OR

            dictionary: A dictionary with the format:
                {
                    'test_name': {
                        'platform': {
                            'expected: 'PASS',
                            'actual': 'FAIL',
                            'bug': 'crbug.com/11111'
                        }
                    }
                }

+                It is possible for the dicitonary to have many test_name
+                keys.
+
+        Returns:
+            A list of test expectations lines with the format
+            ['BUG_URL [PLATFORM(S)] TEST_MAME [EXPECTATION(S)]']
+        """
+        line_list = []
+        for key, value in dictionary.iteritems():
+            test_name = key

[qyearsley, 2016/07/19 17:59:47]

We can also think of more descriptive names than key and value here, e.g. `for
test_name, platform_results in ...`

[dcampb, 2016/07/20 03:29:43]

done

+            for key2 in value:

[qyearsley, 2016/07/19 17:59:48]

Any time you find yourself adding digits after a variable name, that's a sign
that there's probably a clearer and more descriptive name that you could use.

The keys in the inner dicts are either single platforms or tuples of platforms,
right? Instead of key2, it could be called platform (and the code below should
make it clear that it can be either a string or tuple).

[dcampb, 2016/07/20 03:29:43]

agreed

+                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_test_expectations(self, host, path, line_list):
+        """Writes to TestExpectations.
+
+        Writes to the test expectations lines in line_list
+        to LayoutTest/TestExpectations. Checks the file for the string
+        '#Tests added from W3C auto import bot' and writes expectation

[qyearsley, 2016/07/19 17:59:48]

"#Tests" -> "# Tests"

+        lines directly under it. If not found, it writes to the end of
+        the file.
+
+        Args:
+            host: A Host object.
+            path: The path to the file LayoutTest/TestExpectations.
+            line_list: A list of w3c test expectations lines.
+
+        Returns:
+            Writes to a file on the filesystem called LayoutTests/TestExpectations.
+        """
+        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)