Improve flakiness logic for print-flaky-tests.

Tools/Scripts/webkitpy/layout_tests/layout_package/bot_test_expectations.py

 # Copyright (C) 2013 Google Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
 #
 #     * Redistributions of source code must retain the above copyright
 # notice, this list of conditions and the following disclaimer.
 #     * Redistributions in binary form must reproduce the above
 # copyright notice, this list of conditions and the following disclaimer
@@ skipping 18 matching lines @@
 """Generates a fake TestExpectations file consisting of flaky tests from the bot
 corresponding to the give port."""
 
 import json
 import logging
 import os.path
 import urllib
 import urllib2
 
 from webkitpy.layout_tests.port import builders
-from webkitpy.layout_tests.models.test_expectations import TestExpectations
+from webkitpy.layout_tests.models.test_expectations import TestExpectations, PASS
 from webkitpy.layout_tests.models.test_expectations import TestExpectationLine
 
 
 _log = logging.getLogger(__name__)
 
 
 # results.json v4 format:
 # {
 #  'version': 4,
 #  'builder name' : {
@@ skipping 91 matching lines @@
     def expectations_for_builder(self, builder):
         results_json = self._results_json_for_builder(builder)
         if not results_json:
             return None
         return BotTestExpectations(results_json)
 
 class BotTestExpectations(object):
     # FIXME: Get this from the json instead of hard-coding it.
     RESULT_TYPES_TO_IGNORE = ['N', 'X', 'Y']  # NO_DATA, SKIP, NOTRUN
 
     # TODO(ojan): Remove this once crbug.com/514378 is fixed.

[joelo, 2015/08/17 23:43:29]

should be able to remove now?

     # The JSON can contain results for expectations, not just actual result types.
     NON_RESULT_TYPES = ['S', 'X']  # SLOW, SKIP
 
-    PASS_EXPECTATION = 'PASS'
-
     # specifiers arg is used in unittests to avoid the static dependency on builders.
     def __init__(self, results_json, specifiers=None):
         self.results_json = results_json
         self.specifiers = specifiers or set(builders.specifiers_for_builder(results_json.builder_name))
 
     def _line_from_test_and_flaky_types(self, test_path, flaky_types):
         line = TestExpectationLine()
         line.original_string = test_path
         line.name = test_path
         line.filename = test_path
         line.path = test_path  # FIXME: Should this be normpath?
         line.matching_tests = [test_path]
         line.bugs = ["crbug.com/FILE_A_BUG_BEFORE_COMMITTING_THIS"]
-        line.expectations = sorted(map(self.results_json.expectation_for_type, flaky_types))
+        line.expectations = sorted(flaky_types)
         line.specifiers = self.specifiers
         return line
 
     def flakes_by_path(self, only_ignore_very_flaky):
         """Sets test expectations to bot results if there are at least two distinct results."""
         flakes_by_path = {}
         for test_path, entry in self.results_json.walk_results():
             flaky_types = self._flaky_types_in_results(entry, only_ignore_very_flaky)
             if len(flaky_types) <= 1:
                 continue
-            flakes_by_path[test_path] = sorted(map(self.results_json.expectation_for_type, flaky_types))
+            flakes_by_path[test_path] = sorted(flaky_types)
         return flakes_by_path
 
     def unexpected_results_by_path(self):
         """For tests with unexpected results, returns original expectations + results."""
         def exp_to_string(exp):
             return TestExpectations.EXPECTATIONS_TO_STRING.get(exp, None).upper()
 
         def string_to_exp(string):
             # Needs a bit more logic than the method above,
             # since a PASS is 0 and evaluates to False.
@@ skipping 48 matching lines @@
 
         for result_item in run_length_encoded_results:
             _, result_types = self.results_json.occurances_and_type_from_result_item(result_item)
 
             for result_type in result_types:
                 if result_type not in self.RESULT_TYPES_TO_IGNORE:
                     results.add(result_type)
 
         return results
 
+    def _result_to_enum(self, result):
+        return TestExpectations.EXPECTATIONS[result.lower()]
+
     def _flaky_types_in_results(self, results_entry, only_ignore_very_flaky):
         flaky_results = set()
 
-        latest_expectations = [self.PASS_EXPECTATION]
+        # Always include pass as an expected result. Passes will never turn the bot red.
+        # This fixes cases where the expectations have an implicit Pass, e.g. [ Slow ].
+        latest_expectations = [PASS]
         if self.results_json.EXPECTATIONS_KEY in results_entry:
-            latest_expectations = results_entry[self.results_json.EXPECTATIONS_KEY].split(' ')
+            expectations_list = results_entry[self.results_json.EXPECTATIONS_KEY].split(' ')
+            latest_expectations += [self._result_to_enum(expectation) for expectation in expectations_list]
 
         for result_item in results_entry[self.results_json.RESULTS_KEY]:
             _, result_types_str = self.results_json.occurances_and_type_from_result_item(result_item)
 
             result_types = []
             for result_type in result_types_str:
                 # TODO(ojan): Remove this if-statement once crbug.com/514378 is fixed.

[joelo, 2015/08/17 23:43:30]

same here

[ojan, 2015/08/19 00:42:54]

Last I checked, some of the slower bots still had old values in here.

                 if result_type not in self.NON_RESULT_TYPES:
-                    result_types.append(result_type)
+                    result_types.append(self.results_json.expectation_for_type(result_type))
 
             # It didn't flake if it didn't retry.
             if len(result_types) <= 1:
                 continue
 
             # If the test ran as expected after only one retry, it's not very flaky.
             # It's only very flaky if it failed the first run and the first retry
             # and then ran as expected in one of the subsequent retries.
-            if only_ignore_very_flaky and self.results_json.expectation_for_type(result_types[1]) in latest_expectations:
-                # Once we get a pass, we don't retry again. So if the first retry passed,
-                # then there should only be 2 entries because we don't do another retry.
-                # TODO(ojan): Reenable this assert once crbug.com/514378 is fixed.
-                # assert(len(result_types) == 2)
+            # If there are only two entries, then that means it failed on the first
+            # try and ran as expected on the second because otherwise we'd have
+            # a third entry from the next try.
+            second_result_type_enum_value = self._result_to_enum(result_types[1])
+            if only_ignore_very_flaky and len(result_types) == 2:
                 continue
 
-            flaky_results = flaky_results.union(set(result_types))
+            has_unexpected_results = False
+            for result_type in result_types:
+                result_enum = self._result_to_enum(result_type)
+                # TODO(ojan): We really should be grabbing the expected results from the time
+                # of the run instead of looking at the latest expected results. That's a lot
+                # more complicated though. So far we've been looking at the aggregated
+                # results_small.json off test_results.appspot, which has all the information
+                # for the last 100 runs. In order to do this, we'd need to look at the
+                # individual runs' full_results.json, which would be slow and more complicated.
+                # The only thing we lose by not fixing this is that a test that was flaky
+                # and got fixed will still get printed out until 100 runs have passed.
+                if not TestExpectations.result_was_expected(result_enum, latest_expectations, test_needs_rebaselining=False):
+                    has_unexpected_results = True
+                    break
+
+            if has_unexpected_results:
+                flaky_results = flaky_results.union(set(result_types))
 
         return flaky_results