Make update-flaky-tests to work again and support all bots

Tools/Scripts/webkitpy/tool/commands/flakytests.py

 # Copyright (c) 2011 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
 # in the documentation and/or other materials provided with the
 # distribution.
 #     * Neither the name of Google Inc. nor the names of its
 # contributors may be used to endorse or promote products derived from
 # this software without specific prior written permission.
 #
 # THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
 # "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
 # LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
 # A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
 # OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
 # SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
 # LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
 # DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
 # THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+import operator
 from webkitpy.tool.multicommandtool import AbstractDeclarativeCommand
 from webkitpy.layout_tests.layout_package.bot_test_expectations import BotTestExpectationsFactory
 from webkitpy.layout_tests.models.test_expectations import TestExpectationParser, TestExpectationsModel, TestExpectations
 
 
 class FlakyTests(AbstractDeclarativeCommand):
     name = "flaky-tests"
     help_text = "Generate FlakyTests file from the flakiness dashboard"
     show_in_main_help = True
 
     def execute(self, options, args, tool):
+        fs = tool.filesystem

[ojan, 2014/05/29 18:51:20]

Nit: this is only used in two places on the same line. It should either be
inlined or moved before the line where it's used.

         port = tool.port_factory.get()
-        full_port_name = port.determine_full_port_name(tool, options, port.port_name)
-        expectations = BotTestExpectationsFactory().expectations_for_port(full_port_name)
-        print TestExpectations.list_to_string(expectations.expectation_lines())
+        current_expectations = TestExpectations(port, model_all_expectations=True)
+        model = TestExpectationsModel()
+        port_names = tool.port_factory.all_port_names()
+        # builders.all_builder_names includes an ASAN builder but
+        # those are in a separate TestExpectations file, so skip it.
+        port_names = filter(lambda name: 'ASAN' not in name, port_names)

[Dirk Pranke, 2014/05/29 18:25:21]

Perhaps 'ASAN' shouldn't be in all_builder_names() at all?

[ojan, 2014/05/29 18:51:20]

+1. I should have done this before when I made rebaseline-o-matic ignore the
ASAN bot. Removing the ASAN bot from _exact_matches would have been the better
fix.

+        for port_name in port_names:
+            expectations = BotTestExpectationsFactory().expectations_for_port(port_name)
+            for line in expectations.expectation_lines():
+                model.add_expectation_line(line)
+        # FIXME: We need an official API to get all the test names or all test lines.
+        lines = model._test_to_expectation_line.values()
+        lines.sort(key=operator.attrgetter('path'))

[Dirk Pranke, 2014/05/29 18:25:21]

Can you use key=lambda line: line.path or something like that instead?

[Dirk Pranke, 2014/05/29 21:06:46]

I think you missed this comment as well ...

+        # This is a hack, but prevents duplicates with existing TestExpectations
+        # (which would cause lint-test-expectations to fail). A better solution
+        # would be to update 'current_expectations' with the new data and
+        # write out the whole TestExpectations file.

[Dirk Pranke, 2014/05/29 18:25:21]

I don't think we do complain about duplicates across files?

(Though I do agree we should filter out duplicates ...)

[ojan, 2014/05/29 18:51:20]

This is adding to the regular TestExpectations file, so it's not across files.

[Dirk Pranke, 2014/05/29 18:59:56]

Oh. Right. I suppose the better question is, why wouldn't we keep this in a
dedicated FlakyTests file? Mixing hand-edited and automatically generated stuff
seems like a bad idea.

[ojan, 2014/05/29 19:37:04]

Yes! This would solve almost all my concerns I brought up on the blink-dev
thread. If we did this, the only concern I'd have left with turning this one is
being more conservative in when we consider a test flaky, which is a ~5 line
patch to fix. Happy to help with this part if you need it Eric.

With the dedicated FlakyTests file:
1. These lines would never conflict with NeedsRebaseline lines.
2. Tests that stop being flaky would automatically get remove from the file each
time we run the script since we'd always overwrite the whole file each time.
3. It would make it so that we could make it a goal to remove all the flaky
lines from TestExpectations and only be left with FlakyTests.

The only downside to this approach is that it's another file people need to
understand for figuring out which tests run where, but that left the barn ages
ago when we split up TestExpectations into a dozen files.

+        lines = filter(lambda line: not current_expectations.model().has_test(line.name), lines)
+        # Skip any tests which are mentioned in the dashboard but not in our checkout:
+        lines = filter(lambda line: fs.exists(fs.join(port.layout_tests_dir(), line.path)), lines)
+        print TestExpectations.list_to_string(lines)