Removes build and builder object refrences along with minor bug fixes

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 f74cfec2b08689fa8df3ffc05eaaa330ce405045..1b6e4cbd203edd5adae9b78c8466cf80ed2489b6 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
@@ -10,7 +10,7 @@ by retrieving the try job results for the current CL.
 
 import logging
 
-from webkitpy.common.net import buildbot
+from webkitpy.common.net.buildbot import BuildBot
 from webkitpy.common.net import rietveld
 
 
@@ -23,21 +23,17 @@ def main(host, port):
     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)
-    line_expectations_dict = {}
+    test_expectations = {}
     if not try_jobs:
         print 'No Try Job information was collected.'
         return 1
-    for try_job in try_jobs:
-        builder_name = try_job[0]
-        build_number = try_job[1]
-        builder = buildbot.Builder(builder_name, host.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)
-    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)
+    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_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):
@@ -65,7 +61,7 @@ class W3CExpectationsLineAdder(object):
                 }}
         return test_dict
 
-    def get_failing_results_dict(self, builder, build):
+    def get_failing_results_dict(self, buildbot, builder_name, build_number):
         """Returns a nested dict of failing test results.
 
         Retrieves a full list of layout test results from a builder result URL. Collects
@@ -84,8 +80,8 @@ class W3CExpectationsLineAdder(object):
                 }
             }
         """
-        layout_test_results = builder.fetch_layout_test_results(build.results_url())
-        builder_name = layout_test_results.builder_name()
+        results_url = buildbot.results_url(builder_name, build_number)
+        layout_test_results = buildbot.fetch_layout_test_results(results_url)
         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)
@@ -129,29 +125,38 @@ class W3CExpectationsLineAdder(object):
             dictionary: A dictionary with a dictionary as the value.
 
         Returns:
-            A dictionary with updated keys to reflect matching values of keys.
+            A new dictionary with updated keys to reflect matching values of keys.
             Example: {
                 'one': {'foo': 'bar'},
                 'two': {'foo': 'bar'},
-                'three': {'foo': bar'}
+                'three': {'foo': 'bar'}
             }
-            is converted to {('one', 'two', 'three'): {'foo': 'bar'}}
+            is converted to a new dictionary with that contains
+            {('one', 'two', 'three'): {'foo': 'bar'}}
         """
+        merged_dict = {}
         matching_value_keys = set()
-        keys = dictionary.keys()
-        is_last_item = False
-        for index, item in enumerate(keys):
-            if is_last_item:
+        keys = sorted(dictionary.keys())
+        while keys:
+            current_key = keys[0]
+            found_match = False
+            if current_key == keys[-1]:
+                merged_dict[current_key] = dictionary[current_key]
+                keys.remove(current_key)
                 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]
-                    is_last_item = next_item == keys[-1]
-                    del dictionary[item]
-                    del dictionary[next_item]
-        return dictionary
+            for next_item in keys[1:]:
+                if dictionary[current_key] == dictionary[next_item]:
+                    found_match = True
+                    matching_value_keys.update([current_key, next_item])
+                if next_item == keys[-1]:
+                    if found_match:
+                        merged_dict[tuple(matching_value_keys)] = dictionary[current_key]
+                        keys = [k for k in keys if k not in matching_value_keys]
+                    else:
+                        merged_dict[current_key] = dictionary[current_key]
+                        keys.remove(current_key)
+            matching_value_keys = set()
+        return merged_dict
 
     def get_expectations(self, results):
         """Returns a list of test expectations for a given test dict.

[qyearsley, 2016/07/26 23:59:43]

In particular, it's a list of a expectations which should be added as a new line
in TestExpectations, right?

[dcampb, 2016/07/27 16:26:36]

This function returns a list of actual test expectations for a given test. for
example, 'Timeout', 'Failure', 'Pass'. 
This list is used in the function that creates the test-expectations line list.
Example, ['crbug.com/626777 [  Win7 ] fast/new/foo/bar.html [ Failure ]'] 

[qyearsley, 2016/07/27 17:25:39]

Right, so the output of this function determines the expectation type(s) in the
new test expectations line. This could optionally be added to the docstring
(although this is unnecessary since how the function is used should usually be
understood by looking at the code).

[dcampb, 2016/07/27 18:17:50]

agreed. 

@@ -172,16 +177,17 @@ class W3CExpectationsLineAdder(object):
             A list of one or more test expectations with the first letter capitalized. Example:
             ['Failure', 'Timeout']

[qyearsley, 2016/07/26 23:59:43]

This comment can be updated now that this function returns a set.

[dcampb, 2016/07/27 18:17:49]

done

         """
-        expectations = []
-        failure_expectations = ['TEXT', 'FAIL', 'IMAGE+TEXT', 'IMAGE']
+        expectations = set()
+        failure_expectations = ['SLOW', 'TEXT', 'FAIL', 'IMAGE+TEXT', 'IMAGE']

[qyearsley, 2016/07/26 23:59:43]

I'm not sure if SLOW counts as a failure, and also I don't think SLOW is ever in
results["actual"] -- if SLOW is in results["expected"] for a test, I think that
also means it's expected to pass. So, I think maybe SLOW can be removed from
here.

Also, in theory there's one more "baseline mismatch" failure type: AUDIO. I'm
pretty sure there are no tests with audio baselines in web-platform-tests, but
you could add it here for completeness I think.

[dcampb, 2016/07/27 16:26:36]

I do remember seeing an example of an expected that was SLOW TIMEOUT CRASH FAIL,
however in TestExpectations the only expectations are 'Failure, Pass, Timeout'.
Which is why I added SLOW to the list. 

I will look around for all the possible failure types and expected types and see
which ones results in which expectation.

[qyearsley, 2016/07/27 17:25:39]

Alright; here shouldn't be any more "baseline mismatch" types besides those
four. Note, the "official" documentation for the results JSON format is
https://www.chromium.org/developers/the-json-test-results-format.

         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())
+        for expected in results['expected'].split():
+            for actual in results['actual'].split():

[qyearsley, 2016/07/26 23:59:43]

This morning I was saying that if there are multiple words in results["actual"]
then it means the test was retried. If there are multiple words in
results["expected"], though, I believe that this means that any of those
expected results are OK. For example, if expected is "PASS CRASH" and actual is
"CRASH" OR "PASS", then that's OK, and no new expectation line needs to be
added.

Would it also make sense to rewrite this function to take a LayoutTestResult
object (from
https://cs.chromium.org/chromium/src/third_party/WebKit/Tools/Scripts/webkitp...)

[dcampb, 2016/07/27 16:26:36]

Would that result in a mismatch result? or wouldn't that test pass?

 If so, it would not be ran through this script. This script only deals with
results with mismatch results. 

[dcampb, 2016/07/27 16:44:24]

Correction: This script only works with tests that did not run as expected.
However, I don't think that made a difference, as if it was expected to 'Crash'
or 'Pass' and Passes, then it would not have been reported as a failure. 

[qyearsley, 2016/07/27 17:25:39]

What I meant was, since the dict passed in contains basically the same
information as a LayoutTestResult object, I was wondering whether it would make
sense later to refactor the rest of the code to use LayoutTestResult objects --
but now I see that this isn't a simple refactoring if it were done, and
shouldn't be done in this CL.

[dcampb, 2016/07/27 18:17:49]

I see what your saying. I agree, it could be refactored in a later cl. 

+                if expected in pass_crash_timeout and actual in failure_expectations:
+                    expectations.add('Failure')
+                if expected in failure_expectations and actual in pass_crash_timeout:
+                    expectations.add(actual.capitalize())
+                if expected in pass_crash_timeout and actual in pass_crash_timeout:
+                    expectations.add(actual.capitalize())

[qyearsley, 2016/07/26 23:59:43]

What if expected is "PASS" and actual is "PASS" -- then we don't want to add any
new expectation line, right?

[dcampb, 2016/07/27 16:26:36]

That wouldn't happen as this script only parses test that have mismatch results.

[dcampb, 2016/07/27 16:44:24]

Correction: This script parses tests that did not run as expected. therefore,
the expected and pass would never be the same.

Note: There is a function in LayoutTestResults called
unexpected_mismatch_results().. not exactly sure how different that is from
didnt_run_as_expected().

[qyearsley, 2016/07/27 17:25:39]

Yeah, those names aren't super clear. "Mismatch" was supposed to mean "mismatch
with existing baselines", i.e. TEXT, IMAGE, etc.

[dcampb, 2016/07/27 18:17:50]

understood. I think it would be wise to get this cl up first, then take into
account tests that need -expected.txt files added later. 

         return expectations
 
     def create_line_list(self, merged_results):
@@ -210,17 +216,19 @@ class W3CExpectationsLineAdder(object):
         line_list = []
         for test_name, platform_results in merged_results.iteritems():
             for platform in platform_results:
-                platform_list = []
-                bug = []
-                expectations = []
-                if isinstance(platform, tuple):
-                    platform_list = list(platform)
-                else:
-                    platform_list.append(platform)
-                bug.append(platform_results[platform]['bug'])
-                expectations = self.get_expectations(platform_results[platform])
-                line = '%s [ %s ] %s [ %s ]' % (bug[0], ' '.join(platform_list), test_name, ' '.join(expectations))
-                line_list.append(str(line))
+                if test_name.startswith('imported'):
+                    print platform_results
+                    platform_list = []
+                    bug = []
+                    expectations = []
+                    if isinstance(platform, tuple):
+                        platform_list = list(platform)
+                    else:
+                        platform_list.append(platform)
+                    bug.append(platform_results[platform]['bug'])
+                    expectations = self.get_expectations(platform_results[platform])
+                    line = '%s [ %s ] %s [ %s ]' % (bug[0], ' '.join(platform_list), test_name, ' '.join(expectations))
+                    line_list.append(str(line))
         return line_list
 
     def write_to_test_expectations(self, host, path, line_list):