[Media Router] Further split media_router_container tests.

chrome/test/data/webui/media_router/media_router_elements_browsertest.js

Index: chrome/test/data/webui/media_router/media_router_elements_browsertest.js
diff --git a/chrome/test/data/webui/media_router/media_router_elements_browsertest.js b/chrome/test/data/webui/media_router/media_router_elements_browsertest.js
index eae673a463ab3d7c99c215b128c612e2b6b0a859..f7ffc2a8b39e00e194435103a2ea25baf4d17238 100644
--- a/chrome/test/data/webui/media_router/media_router_elements_browsertest.js
+++ b/chrome/test/data/webui/media_router/media_router_elements_browsertest.js
@@ -39,7 +39,8 @@ MediaRouterElementsBrowserTest.prototype = {
   // media_router_container_filter_tests.js.
   extraLibraries: PolymerTest.getLibraries(ROOT_PATH).concat([
     'issue_banner_tests.js',
-    'media_router_container_tests.js',
+    'media_router_container_part1_tests.js',
+    'media_router_container_part2_tests.js',

[imcheng, 2016/03/04 00:26:58]

I see you have moved most of the cast mode list related tests into one file.
Would it make sense for call this media_router_container_cast_mode_list.js, or
something similar?

[btolsch, 2016/03/04 02:14:38]

Well, this isn't really a logical split.  It was based on evenly splitting the
execution time. If more cast mode tests were going to be added, they might need
to go in a new file. So I don't think I want to suggest that file as a place
that cast mode tests go.

[apacible, 2016/03/04 17:21:33]

Would it make sense to split tests based on views or specific things they're
testing? That'll hopefully bring the execution time down by a larger margin for
each test while making it more organized.

For example, media_router_container_first_run_flow.js,
media_router_container_cast_mode_list.js, etc...

Right now, anyone looking for tests will have to look at both files.

[btolsch, 2016/03/04 20:00:37]

Initially I was hesitant to do that because of the duplication in the suite
setup that would cause. I think that would be worth it though, especially since
the timings are implying that this split might still be flaky on bots. And if it
turns out it isn't a timing issue, the logical split would help narrow down the
issue.

[apacible, 2016/03/07 17:59:04]

Let's pull out all of the properties / setup / custom checks into another JS
file so it's all in one place. I agree with the duplication concern; it would be
tedious to update every single file (e.g. |hiddenCheckElementIdList|) for some
changes.

We could probably do something similar to how settings pulls in additional
files. Example:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/data/w...

[btolsch, 2016/03/07 20:20:15]

Done.

     'media_router_container_filter_tests.js',
     'media_router_header_tests.js',
     'media_router_search_highlighter.js',
@@ -91,12 +92,21 @@ TEST_F('MediaRouterElementsBrowserTest', 'MediaRouterElementsTestIssueBanner',
   mocha.run();
 });
 
-// See bugs.chromium.org issue 591227
 TEST_F('MediaRouterElementsBrowserTest',

[imcheng, 2016/03/04 00:26:59]

Since we are at the point where it is necessary to split tests within a logical
component to prevent timing out, it may be worthwhile to put in a comment here
explaining that. 

[btolsch, 2016/03/04 02:14:37]

Done.

-    'DISABLED_MediaRouterElementsTestMediaRouterContainer',
+    'MediaRouterElementsTestMediaRouterContainerPart1',
     function() {
   // Register mocha tests for the container.

[apacible, 2016/03/04 01:25:29]

This comment can also be removed from all test cases.

[btolsch, 2016/03/04 02:14:37]

Done.

-  media_router_container.registerTests();
+  media_router_container_part1.registerTests();
+
+  // Run all registered tests.

[imcheng, 2016/03/04 00:26:58]

This comment can be removed from all test cases.

[btolsch, 2016/03/04 02:14:37]

Done.

+  mocha.run();
+});
+
+TEST_F('MediaRouterElementsBrowserTest',
+    'MediaRouterElementsTestMediaRouterContainerPart2',
+    function() {
+  // Register mocha tests for the container.
+  media_router_container_part2.registerTests();
 
   // Run all registered tests.
   mocha.run();