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

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

Index: Tools/Scripts/webkitpy/tool/commands/flakytests.py
diff --git a/Tools/Scripts/webkitpy/tool/commands/flakytests.py b/Tools/Scripts/webkitpy/tool/commands/flakytests.py
index 3a97cd61cd7bf5a9462306a076dd759d3b5dd842..b976e8edb125b572f2b883b89363e65ce9b6ebfc 100644
--- a/Tools/Scripts/webkitpy/tool/commands/flakytests.py
+++ b/Tools/Scripts/webkitpy/tool/commands/flakytests.py
@@ -26,6 +26,7 @@
 # (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
@@ -37,7 +38,26 @@ class FlakyTests(AbstractDeclarativeCommand):
     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)