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

third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/update_test_expectations_unittest.py

Index: third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/update_test_expectations_unittest.py
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/update_test_expectations_unittest.py b/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/update_test_expectations_unittest.py
index 942edfae58b5240db8b8b2ff4a4aa282306deace..065f30c3a2f0f994431cca28a683f3bfc5dc11dd 100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/update_test_expectations_unittest.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/update_test_expectations_unittest.py
@@ -2,8 +2,11 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-from collections import OrderedDict
 import logging
+import webkitpy.layout_tests.update_test_expectations

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

Formatting/style nit: Would this still work if this were moved down and changed
to: from webkitpy.layout_tests import update_test_expectations?

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

Yes but it's no longer needed due to your suggestion below.

+
+from collections import OrderedDict
+from functools import partial
 
 from webkitpy.common.host_mock import MockHost
 from webkitpy.common.system.filesystem_mock import MockFileSystem
@@ -13,6 +16,7 @@ from webkitpy.layout_tests.port.factory import PortFactory
 from webkitpy.layout_tests.port.test import LAYOUT_TEST_DIR
 from webkitpy.layout_tests.update_test_expectations import main
 from webkitpy.layout_tests.update_test_expectations import RemoveFlakesOMatic
+from webkitpy.tool.commands.flaky_tests import FlakyTests
 
 
 class FakeBotTestExpectations(object):
@@ -75,10 +79,25 @@ class FakePortFactory(PortFactory):
         return port
 
 
+class MockWebBrowser(object):
+
+    def __init__(self):
+        self.opened_url = None
+
+    def open(self, url):
+        self.opened_url = url
+
+
 class UpdateTestExpectationsTest(LoggingTestCase):
 
     def setUp(self):
         super(UpdateTestExpectationsTest, self).setUp()
+        self._mock_web_browser = MockWebBrowser()
+        self._saved_webbrowser_open = webkitpy.layout_tests.update_test_expectations.webbrowser.open
+        webkitpy.layout_tests.update_test_expectations.webbrowser.open = (
+            partial(MockWebBrowser.open, self._mock_web_browser)
+        )

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

One way to refactor to make this more similar to other tests in webkitpy is:
RemoveFlakesOMatic could have a "webbrowser" attribute which is set in the
constructor, and replaced in the tests. What do you think?

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

Done.

+
         self._host = MockHost()
         self._port = self._host.port_factory.get('test', None)
         self._expectation_factory = FakeBotTestExpectationsFactory()
@@ -93,6 +112,10 @@ class UpdateTestExpectationsTest(LoggingTestCase):
         filesystem = self._host.filesystem
         self._write_tests_into_filesystem(filesystem)
 
+    def tearDown(self):
+        super(UpdateTestExpectationsTest, self).tearDown()
+        webkitpy.layout_tests.update_test_expectations.webbrowser.open = self._saved_webbrowser_open
+
     def _write_tests_into_filesystem(self, filesystem):
         test_list = ['test/a.html',
                      'test/b.html',
@@ -975,3 +998,40 @@ class UpdateTestExpectationsTest(LoggingTestCase):
 
         self.assertTrue(host.filesystem.isfile(test_expectation_path))
         self.assertEqual(host.filesystem.files[test_expectation_path], '')
+
+    def test_show_results(self):
+        """Tests that passing --show-results shows the removed results.
+
+        --show-results opens the removed tests in the layout dashboard using the default browser.
+        This tests mocks the webbrowser.open function and checks that it was called with the correct
+        url.
+        """
+        test_expectations_before = (
+            """# Remove this since it's passing all runs.
+            Bug(test) test/a.html [ Failure Pass ]
+            # Remove this since, although there's a failure, it's not a timeout.
+            Bug(test) test/b.html [ Pass Timeout ]
+            # Keep since we have both crashes and passes.
+            Bug(test) test/c.html [ Crash Pass ]""")
+
+        self._define_builders({
+            "WebKit Linux": {
+                "port_name": "linux-precise",
+                "specifiers": ['Precise', 'Release']
+            },
+        })
+        self._port.all_build_types = ('release',)
+        self._port.all_systems = (('precise', 'x86_64'),)
+
+        self._parse_expectations(test_expectations_before)
+        self._expectation_factory._all_results_by_builder = {
+            'WebKit Linux': {
+                "test/a.html": ["PASS", "PASS", "PASS"],
+                "test/b.html": ["PASS", "IMAGE", "PASS"],
+                "test/c.html": ["PASS", "CRASH", "PASS"],
+            }
+        }
+        self._flake_remover.show_removed_results()
+        self.assertEqual(
+            FlakyTests.FLAKINESS_DASHBOARD_URL % 'test/a.html,test/b.html',
+            self._mock_web_browser.opened_url)