Add switch to update-test-expectations to show removed lines in dashboard

third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/update_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.
 
 """Updates TestExpectations based on results in builder bots.
 
 Scans the TestExpectations file and uses results from actual builder bots runs
 to remove tests that are marked as flaky but don't fail in the specified way.
 
 E.g. If a test has this expectation:
     bug(test) fast/test.html [ Failure Pass ]
 
 And all the runs on builders have passed the line will be removed.
 
 Additionally, the runs don't all have to be Passing to remove the line;
 as long as the non-Passing results are of a type not specified in the
 expectation this line will be removed. For example, if this is the
 expectation:
 
     bug(test) fast/test.html [ Crash Pass ]
 
 But the results on the builders show only Passes and Timeouts, the line
 will be removed since there's no Crash results.
 """
 
 import argparse
 import logging
+import webbrowser
 
 from webkitpy.layout_tests.models.test_expectations import TestExpectations
+from webkitpy.tool.commands.flaky_tests import FlakyTests
 
 _log = logging.getLogger(__name__)
 
 
 def main(host, bot_test_expectations_factory, argv):
     parser = argparse.ArgumentParser(epilog=__doc__, formatter_class=argparse.RawTextHelpFormatter)
     parser.add_argument('--verbose', '-v', action='store_true', default=False, help='enable more verbose logging')
+    parser.add_argument('--show-results',
+                        '-s',
+                        action='store_true',
+                        default=False,
+                        help='Open results dashboard for all removed lines')
     args = parser.parse_args(argv)
 
     logging.basicConfig(level=logging.DEBUG if args.verbose else logging.INFO, format="%(levelname)s: %(message)s")
 
     port = host.port_factory.get()
     expectations_file = port.path_to_generic_test_expectations_file()
     if not host.filesystem.isfile(expectations_file):
         _log.warning("Didn't find generic expectations file at: " + expectations_file)
         return 1
 
     remove_flakes_o_matic = RemoveFlakesOMatic(host,
                                                port,
                                                bot_test_expectations_factory)
 
     test_expectations = remove_flakes_o_matic.get_updated_test_expectations()
 
+    if args.show_results:
+        remove_flakes_o_matic.show_removed_results()
+
     remove_flakes_o_matic.write_test_expectations(test_expectations,
                                                   expectations_file)
     return 0
 
 
 class RemoveFlakesOMatic(object):
 
     def __init__(self, host, port, bot_test_expectations_factory):
         self._host = host
         self._port = port
         self._expectations_factory = bot_test_expectations_factory
         self._builder_results_by_path = {}
+        self._expectations_to_remove = None
 
     def _can_delete_line(self, test_expectation_line):
         """Returns whether a given line in the expectations can be removed.
 
         Uses results from builder bots to determine if a given line is stale and
         can safely be removed from the TestExpectations file. (i.e. remove if
         the bots show that it's not flaky.) There are also some rules about when
         not to remove lines (e.g. never remove lines with Rebaseline
         expectations, don't remove non-flaky expectations, etc.)
 
@@ skipping 173 matching lines @@
         # Remove all comments above the removed line.
         while removed_index > 0 and expectations[removed_index - 1].is_comment():
             removed_index -= 1
             expectations.pop(removed_index)
 
         # Remove all whitespace above the comments.
         while removed_index > 0 and expectations[removed_index - 1].is_whitespace():
             removed_index -= 1
             expectations.pop(removed_index)
 
+    def _ensure_expectations_to_remove(self):
+        """Ensures the _expectations_to_remove member is filled in.
+
+        This member starts uninitialized. Several public methods need this member to be filled in to
+        operate but we don't want to build it multiple times and using the constructor would make
+        testing inconvenient. Instead, each calls this method which early-outs if the work has
+        already been done.
+        """
+        if self._expectations_to_remove is not None:
+            return

[qyearsley, 2016/10/05 21:06:08]

Another possible approach to consider here may be to make
_expectations_to_remove a memoized property (using the memoized decorator in
webkitpy/common/memoized.py, OR by doing something like:

def _expectations_to_remove(self):
    if self._expectations_to_remove is not None:
        return self._expectations_to_remove

    # logic to populate self._expectations_to_remove
    return self._expectations_to_remove

But this as-is is good too.

[bokan, 2016/10/05 22:35:24]

Done.

+
+        self._builder_results_by_path = self._get_builder_results_by_path()
+        self._expectations_to_remove = []
+        test_expectations = TestExpectations(self._port, include_overrides=False).expectations()
+
+        for expectation in test_expectations:
+            if self._can_delete_line(expectation):
+                self._expectations_to_remove.append(expectation)
+
     def get_updated_test_expectations(self):
         """Filters out passing lines from TestExpectations file.
 
         Reads the current TestExpectations file and, using results from the
         build bots, removes lines that are passing. That is, removes lines that
         were not needed to keep the bots green.
 
         Returns:
             A TestExpectations object with the passing lines filtered out.
         """
+        self._ensure_expectations_to_remove()
 
-        self._builder_results_by_path = self._get_builder_results_by_path()
-
-        expectations_to_remove = []
         test_expectations = TestExpectations(self._port, include_overrides=False).expectations()
-
-        for expectation in test_expectations:
-            if self._can_delete_line(expectation):
-                expectations_to_remove.append(expectation)
-
-        for expectation in expectations_to_remove:
+        for expectation in self._expectations_to_remove:
             index = test_expectations.index(expectation)
             test_expectations.remove(expectation)
 
             # Remove associated comments and whitespace if we've removed the last expectation under
             # a comment block. Only remove a comment block if it's not separated from the test
             # expectation line by whitespace.
             self._remove_associated_comments_and_whitespace(test_expectations, index)
 
         return test_expectations
 
+    def show_removed_results(self):
+        """Opens removed lines in the results dashboard.
+
+        Opens the results dashboard in the browser, showing all the tests for lines that the script
+        removed from the TestExpectations file and allowing the user to manually confirm the
+        results.
+        """
+        self._ensure_expectations_to_remove()
+        removed_test_names = ','.join(x.name for x in self._expectations_to_remove)
+        url = FlakyTests.FLAKINESS_DASHBOARD_URL % removed_test_names
+
+        _log.info('Opening results dashboard: ' + url)
+        webbrowser.open(url)
+
     def write_test_expectations(self, test_expectations, test_expectations_file):
         """Writes the given TestExpectations object to the filesystem.
 
         Args:
             test_expectations: The TestExpectations object to write.
             test_expectations_file: The full file path of the Blink
                 TestExpectations file. This file will be overwritten.
         """
         self._host.filesystem.write_text_file(
             test_expectations_file,
             TestExpectations.list_to_string(test_expectations, reconstitute_only_these=[]))