MD Settings: split browser tests into smaller chunks

chrome/test/data/webui/settings/cr_settings_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 Polymer Settings tests. */
 
 /** @const {string} Path to source root. */
 var ROOT_PATH = '../../../../../';
 
 // Polymer BrowserTest fixture.
@@ skipping 36 matching lines @@
     // TODO(michaelpg): Re-enable after bringing in fix for
     // https://github.com/PolymerElements/paper-slider/issues/131.
     this.accessibilityAuditConfig.ignoreSelectors(
         'badAriaAttributeValue', 'paper-slider');
   },
 };
 
 // Have to include command_line.h manually due to GEN calls below.
 GEN('#include "base/command_line.h"');
 
-// Times out on memory bots. http://crbug.com/534718
-GEN('#if defined(MEMORY_SANITIZER)');

[dpapad, 2016/06/09 01:21:29]

Won't removing this line make our tests fail on memory bots again? If this is
not an issue anymore, then we should probably also update all other places were
we replicated this ifdef, see
https://cs.chromium.org/search/?q=MEMORY_SANITIZER+file:%5Esrc/chrome/test/da....

[Dan Beam, 2016/06/09 01:37:25]

the test previously failed for 3 reasons:

a) the GN migration accidentally used a debug version of v8 instead of an
optimized one (fixed: crbug.com/586511)

b) the test was doing too much (fixed after this CL and crrev.com/2050053002)

c) memory bots are slow cuz they're tracking memory (not fixed)

I'm guessing that fixing 2 out of 3 of these are enough to re-enable on msan
bots

[michaelpg, 2016/06/09 04:06:36]

this is the first bullet point under "advantages" here:

https://www.chromium.org/developers/updating-webui-for-material-design/testin...

if that philosophy was wrong, we should remove it. What are the timings actually
like here? I suspect the browser test overhead, including navigating to the
browsePreload, accounts for like 90% of the time, and the time taken for all
suites of unrelated mocha tests would be at most 10%.

Put another way: It seems very likely to me that a test with 4 mocha suites
which times out, will still time out if you disable 3 of the suites. Unless one
particular suite is just hella slow for some reason and needs to be addressed.

You're almost quadrupling the amount of time running these tests will take by
making us spin up more browsers. Of course that's not a problem from the
flakiness perspective*, because each one has its own timeout, it's just slow
overall... and since our intention with web components is to test more granular
elements, WebUI will ultimately have many many more browser_tests this way than
we've had before. Is the *total* running time of browser_tests not a concern?


* actually it is a problem for flakiness, as a flake is more likely to occur in
one of 4 correct browser_tests than in just 1 correct browser_tests.

[Dan Beam, 2016/06/09 04:37:08]

doing more work will always take more time, BUT I agree with you that we need
better per-step (setup, teardown, each suite) metrics.

I'll run some printf()s on the try bots before I go further so we're not blind.

-GEN('#define MAYBE_CrSettingsTest DISABLED_CrSettingsTest');
-GEN('#else');
-GEN('#define MAYBE_CrSettingsTest CrSettingsTest');
-GEN('#endif');
-
-// TODO(dbeam): these should be split into multiple TEST_F()s.
-TEST_F('CrSettingsBrowserTest', 'MAYBE_CrSettingsTest', function() {
+TEST_F('CrSettingsBrowserTest', 'Checkbox', function() {
   settings_checkbox.registerTests();
-  settings_dropdown_menu.registerTests();
-  settings_prefUtil.registerTests();
-  settings_prefs.registerTests();
-
-  // Run all registered tests.
   mocha.run();
 });
 
+TEST_F('CrSettingsBrowserTest', 'DropdownMenu', function() {
+  settings_dropdown_menu.registerTests();
+  mocha.run();
+});
+
+TEST_F('CrSettingsBrowserTest', 'PrefUtil', function() {
+  settings_prefUtil.registerTests();
+  mocha.run();
+});
+
+TEST_F('CrSettingsBrowserTest', 'Prefs', function() {
+  settings_prefs.registerTests();
+  mocha.run();
+});
+
 /**
  * @constructor
  * @extends {CrSettingsBrowserTest}
  */
 function CrSettingsAboutPageTest() {}
 
 CrSettingsAboutPageTest.prototype = {
   __proto__: CrSettingsBrowserTest.prototype,
 
   /** @override */
@@ skipping 464 matching lines @@
   extraLibraries: CrSettingsBrowserTest.prototype.extraLibraries.concat([
     'fake_language_settings_private.js',
     'fake_settings_private.js',
     'languages_page_tests.js',
   ]),
 };
 
 TEST_F('CrSettingsLanguagesPageTest', 'LanguagesPage', function() {
   mocha.run();
 });