[Media Router] Further split media_router_container tests.

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

 // Copyright 2015 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.
 
 /** @fileoverview Runs the Media Router Polymer elements tests. */
 
 /** @const {string} Path to source root. */
 var ROOT_PATH = '../../../../../';
 
 // Polymer BrowserTest fixture.
@@ skipping 21 matching lines @@
 
   commandLineSwitches: [{
     switchName: 'media-router', switchValue: '1'
   }],
 
   // List tests for individual elements. The media_router_container tests are
   // split between media_router_container_tests.js and
   // 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',
     'route_details_tests.js',
   ]),
 
   /** @override */
   setUp: function() {
     PolymerTest.prototype.setUp.call(this);
 
@@ skipping 31 matching lines @@
 
 TEST_F('MediaRouterElementsBrowserTest', 'MediaRouterElementsTestIssueBanner',
     function() {
   // Register mocha tests for the issue banner.
   issue_banner.registerTests();
 
   // Run all registered tests.
   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();
 });
 
 TEST_F('MediaRouterElementsBrowserTest',
     'MediaRouterElementsTestMediaRouterContainerFilter',
     function() {
   // Register mocha tests for the container filter.
   media_router_container_filter.registerTests();
 
   // Run all registered tests.
   mocha.run();
@@ skipping 21 matching lines @@
 
 TEST_F('MediaRouterElementsBrowserTest',
     'MediaRouterElementsTestMediaRouterRouteDetails',
     function() {
   // Register mocha tests for the route details.
   route_details.registerTests();
 
   // Run all registered tests.
   mocha.run();
 });