MD Settings: Fix and re-enable settings-main tests.

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

 // Copyright 2016 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.
 
 cr.define('settings_main_page', function() {
   /**
    * Extending TestBrowserProxy even though SearchManager is not a browser proxy
    * itself. Essentially TestBrowserProxy can act as a "proxy" for any external
    * dependency, not just "browser proxies" (and maybe should be renamed to
    * TestProxy).
@@ skipping 151 matching lines @@
           Polymer.dom.flush();
           assertTrue(noSearchResults.hidden);
           assertToggleContainerVisible(true);
         });
       });
 
       /**
        * Asserts the visibility of the basic and advanced pages.
        * @param {string} Expected 'display' value for the basic page.
        * @param {string} Expected 'display' value for the advanced page.
+       * @return {!Promise}
        */
       function assertPageVisibility(expectedBasic, expectedAdvanced) {
         Polymer.dom.flush();
         var page = settingsMain.$$('settings-basic-page');
         assertEquals(
             expectedBasic, page.$$('#basicPage').style.display);
-        assertEquals(
-            expectedAdvanced,
-            page.$$('#advancedPageTemplate').get().style.display);
+
+        return page.$$('#advancedPageTemplate').get().then(
+            function(advancedPage) {
+              assertEquals(
+                  expectedAdvanced || 'block',

[michaelpg, 2017/05/08 18:00:04]

why the || 'block'? (is it related to callers invoking this function with empty
strings?)

[dpapad, 2017/05/08 19:07:52]

Updated all callers to pass 'block' instead of '', and removed from here. Before
we were using |style.display| (and assuming that worked at the time the tests
were originally authored), whereas now the test is using
getComputedStyle(element).display, which is more robust.

[michaelpg, 2017/05/08 20:01:32]

Oh, I see. Thanks for updating!

+                  getComputedStyle(advancedPage).display);
+        });
       }
 
       // TODO(michaelpg): It would be better not to drill into
       // settings-basic-page. If search should indeed only work in Settings
       // (as opposed to Advanced), perhaps some of this logic should be
       // delegated to settings-basic-page now instead of settings-main.
 
       /**
        * Asserts the visibility of the basic and advanced pages after exiting
        * search mode.
        * @param {string} Expected 'display' value for the basic page.
        * @param {string} Expected 'display' value for the advanced page.
        * @return {!Promise}
        */
       function assertPageVisibilityAfterSearch(
           expectedBasic, expectedAdvanced) {
         searchManager.setMatchesFound(true);
         return settingsMain.searchContents('Query1').then(function() {
           searchManager.setMatchesFound(false);
           return settingsMain.searchContents('');
         }).then(function() {
-          assertPageVisibility(expectedBasic, expectedAdvanced);
+          return assertPageVisibility(expectedBasic, expectedAdvanced);
         });
       }
 
       test('exiting search mode, advanced collapsed', function() {
         // Simulating searching while the advanced page is collapsed.
         settingsMain.currentRouteChanged(settings.Route.BASIC);
         Polymer.dom.flush();
         return assertPageVisibilityAfterSearch('', 'none');
       });
 
@@ skipping 15 matching lines @@
 
         searchManager.setMatchesFound(true);
         return settingsMain.searchContents('Query1').then(function() {
           // Simulate navigating into a subpage.
           settings.navigateTo(settings.Route.SEARCH_ENGINES);
           settingsMain.$$('settings-basic-page').fire('subpage-expand');
           Polymer.dom.flush();
 
           // Simulate clicking the left arrow to go back to the search results.
           settings.navigateTo(settings.Route.BASIC);
-          assertPageVisibility('', '');
+          return assertPageVisibility('', '');
         });
       });
 
       // TODO(michaelpg): Move these to a new test for settings-basic-page.
       test('can collapse advanced on advanced section route', function() {
         settings.navigateTo(settings.Route.PRIVACY);
         Polymer.dom.flush();
 
-        var advancedToggle =
-            getToggleContainer().querySelector('#advancedToggle');
-        assertTrue(!!advancedToggle);
+        var basicPage = settingsMain.$$('settings-basic-page');
+        var advancedPage = null;
+        return basicPage.$$('#advancedPageTemplate').get().then(
+            function(advanced) {
+              advancedPage = advanced;
+              return assertPageVisibility('', '');
+            }).then(function() {
+              var whenHidden = test_util.whenAttributeIs(
+                  advancedPage, 'hidden', true);

[michaelpg, 2017/05/08 18:00:03]

The 'hidden' attribute can never be a boolean; it will be |null| or a string.

Seems like test_util.whenAttributeIs is written incorrectly:

    return target[attributeName] === attributeValue;

That is testing a property, not an attribute. Can you add a TODO (here or there)
to fix it or rename it?

[dpapad, 2017/05/08 19:07:52]

Added TODO.

 
-        MockInteractions.tap(advancedToggle);
-        Polymer.dom.flush();
+              var advancedToggle =
+                  getToggleContainer().querySelector('#advancedToggle');
+              assertTrue(!!advancedToggle);
+              MockInteractions.tap(advancedToggle);
 
-        assertPageVisibility('', 'none');
+              return whenHidden;
+            }).then(function() {
+              return assertPageVisibility('', 'none');
+            });
       });
 
       test('navigating to a basic page does not collapse advanced', function() {
         settings.navigateTo(settings.Route.PRIVACY);
         Polymer.dom.flush();
 
         assertToggleContainerVisible(true);
 
         settings.navigateTo(settings.Route.PEOPLE);
         Polymer.dom.flush();
 
-        assertPageVisibility('', '');
+        return assertPageVisibility('', '');
       });
     });
   }
 
   return {
     registerTests: registerTests,
   };
 });