MD Settings: Fix site data details regression and add test.

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 645 matching lines @@
   extraLibraries: CrSettingsBrowserTest.prototype.extraLibraries.concat([
     'category_default_setting_tests.js',
     'category_setting_exceptions_tests.js',
     'site_details_tests.js',
     'site_details_permission_tests.js',
     'site_list_tests.js',
     'test_browser_proxy.js',
     'test_site_settings_prefs_browser_proxy.js',
     'zoom_levels_tests.js',
     'usb_devices_tests.js',
-    'protocol_handlers_tests.js'
+    'protocol_handlers_tests.js',
+    'site_data_details_subpage_tests.js',
   ]),
 };
 
 TEST_F('CrSettingsSiteSettingsTest', 'SiteSettings', function() {
   category_default_setting.registerTests();
   category_setting_exceptions.registerTests();
   site_details.registerTests();
   site_details_permission.registerTests();
   site_list.registerTests();
   zoom_levels.registerTests();
   usb_devices.registerTests();
   protocol_handlers.registerTests();
+  site_data_details.registerTests();

[dpapad, 2017/02/08 00:04:10]

It is unfortunate that I have to use registerTests() here, but I could not find
a good way to avoid this. The new test I am adding is appending a "site=foo"
parameter to the URL, which affects the behavior of the site_details_tests.js
above. So essentially to preserve the old behavior of site_details_test.js, I
needed a way to either

 1 register the new test last, done via an explicit call to registerTests(), so
that the URL has not changed yet, by the time site_details_test.js runs, OR
 2 revert the URL change in the setup() of site_details_test.js.

I chose the 1st one.

[tommycli, 2017/02/08 19:28:40]

I probably would revert the URL change in the teardown of the test. Or -- if
that URL change is only applicable to that one test, do it in the test directly
(and revert in the test directly) instead of the setup/teardown methods.

I don't like to nitpick, but having this test be last seems to be fairly
fragile. If someone adds another test after this, it's going to mysteriously not
work without any obvious reason why.

I don't care if you leave it as a separate js file or combine it... that's up to
you.

[dpapad, 2017/02/08 22:01:09]

Done. Thanks for the help.

 
   mocha.run();
 });
 
 GEN('#if defined(OS_CHROMEOS)');
 /**
  * Test fixture for device-page.
  * @constructor
  * @extends {CrSettingsBrowserTest}
  */
@@ skipping 508 matching lines @@
   extraLibraries: CrSettingsBrowserTest.prototype.extraLibraries.concat([
     'test_browser_proxy.js',
     'test_extension_control_browser_proxy.js',
     'extension_controlled_indicator_tests.js',
   ]),
 };
 
 TEST_F('CrSettingsExtensionControlledIndicatorTest', 'All', function() {
   mocha.run();
 });