MD Settings: split browser tests into smaller chunks

chrome/test/data/webui/settings/cr_settings_browsertest.js

Index: chrome/test/data/webui/settings/cr_settings_browsertest.js
diff --git a/chrome/test/data/webui/settings/cr_settings_browsertest.js b/chrome/test/data/webui/settings/cr_settings_browsertest.js
index fd0c2f6173c3a62912ad3d1df9d0b5c078f64b34..0f54682ee4911d007b9f32fef8ac4b0b3247e255 100644
--- a/chrome/test/data/webui/settings/cr_settings_browsertest.js
+++ b/chrome/test/data/webui/settings/cr_settings_browsertest.js
@@ -54,21 +54,23 @@ CrSettingsBrowserTest.prototype = {
 // 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();
+  mocha.run();
+});
+
+TEST_F('CrSettingsBrowserTest', 'DropdownMenu', function() {
   settings_dropdown_menu.registerTests();
+  mocha.run();
+});
+
+TEST_F('CrSettingsBrowserTest', 'PrefUtil', function() {
   settings_prefUtil.registerTests();
-  settings_prefs.registerTests();
+  mocha.run();
+});
 
-  // Run all registered tests.
+TEST_F('CrSettingsBrowserTest', 'Prefs', function() {
+  settings_prefs.registerTests();
   mocha.run();
 });