MD Settings: Manage search engines 3/3, hooking up UI.

chrome/test/data/webui/settings/search_engines_page_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_search_engines_page', function() {
+  /**
+   * TODO(dpapad): Similar class exists in webui_resource_async_browsertest.js.
+   * Move somewhere else and re-use.
+   * @constructor
+   */
+  var PromiseResolver = function() {
+    this.promise = new Promise(function(resolve, reject) {
+      this.resolve = resolve;
+      this.reject = reject;
+    }.bind(this));
+  };
+
+  // See |defineTestBrowserProxy| function below (this class can only be defined
+  // after search_engines_browser_proxy.html has been imported.
+  var TestSearchEnginesBrowserProxy = null;
+
+  /**
+   * @return {!Promise} A promise resolved after the
+   *     TestSearchEnginesBrowserProxy class is defined.
+   */
+  function defineTestBrowserProxy() {
+    if (TestSearchEnginesBrowserProxy != null)
+      return Promise.resolve();
+
+    var browserProxyImportPath =
+        'chrome://md-settings/search_engines_page/search_engines_browser_proxy.html';

[Dan Beam, 2016/02/18 03:08:19]

why doesn't the page that's visited via browsePreload have

  <link rel="import"
href="chrome://md-settings/search_engines_page/search_engines_browser_proxy.html">

to remove the need for this awkward code?

[dpapad, 2016/02/18 19:34:51]

For many reasons, see also reply at line 177.
I am unit-testing individual elements, and I need time to setup the test browser
proxy before the page is loaded, so browsePreload URL can't be
chrome://md-settings/searchEngine because that would not give me time to setup.

+    return PolymerTest.importHtml(browserProxyImportPath).then(function() {
+      /**
+       * A test version of SearchEnginesBrowserProxy. Provides helper methods for
+       * allowing tests to know when a method was called, as well as specifying
+       * mock responses.
+       *
+       * @constructor
+       * @extends {settings.SearchEnginesBrowserProxy}
+       */
+      TestSearchEnginesBrowserProxy = function() {
+        /** @private {!Map<string, !PromiseResolver>} */
+        this.resolverMap_ = this.buildResolverMap_();
+
+        /** @private {!SearchEnginesInfo} */
+        this.searchEnginesInfo_ = {};
+      };
+
+      TestSearchEnginesBrowserProxy.prototype = {
+        __proto__: settings.SearchEnginesBrowserProxy.prototype,
+
+        /**
+         * @return {!Map<string, !PromiseResolver>}
+         * @private
+         */
+        buildResolverMap_: function() {
+          var resolverMap = new Map();
+          var wrapperMethods = [
+            'getSearchEnginesList',
+            'removeSearchEngine',
+            'searchEngineEditCancelled',
+            'searchEngineEditCompleted',
+            'searchEngineEditStarted',
+            'setDefaultSearchEngine',
+            'validateSearchEngineInput',
+          ];
+          wrapperMethods.forEach(function(methodName) {
+            resolverMap.set(methodName, new PromiseResolver());
+          });
+          return resolverMap;
+        },
+
+        /**
+         * @param {string} methodName
+         * @return {!Promise} A promise that is resolved when the given method is

[Dan Beam, 2016/02/18 03:08:19]

80 col wrap

[dpapad, 2016/02/18 19:34:51]

Done.

+         *     called.
+         */
+        whenCalled: function(methodName) {
+          return this.resolverMap_.get(methodName).promise;
+        },
+
+        /**
+         * Resets the PromiseResolver associated with the given method.
+         * @param {string} methodName
+         */
+        resetResolver: function(methodName) {
+          this.resolverMap_.set(methodName, new PromiseResolver());
+        },
+
+        /** @override */
+        setDefaultSearchEngine: function(modelIndex) {
+          this.resolverMap_.get('setDefaultSearchEngine').resolve(modelIndex);
+        },
+
+        /** @override */
+        removeSearchEngine: function(modelIndex) {
+          this.resolverMap_.get('removeSearchEngine').resolve(modelIndex);
+        },
+
+        /** @override */
+        searchEngineEditStarted: function(modelIndex) {
+          this.resolverMap_.get('searchEngineEditStarted').resolve(modelIndex);
+        },
+
+        /** @override */
+        searchEngineEditCancelled: function() {
+          this.resolverMap_.get('searchEngineEditCancelled').resolve();
+        },
+
+        /** @override */
+        searchEngineEditCompleted: function(searchEngine, keyword, queryUrl) {
+          this.resolverMap_.get('searchEngineEditCompleted').resolve();
+        },
+
+        /**
+         * Sets the response to be returned by |getSearchEnginesList|.
+         * @param {!SearchEnginesInfo}
+         */
+        setGetSearchEnginesList: function(searchEnginesInfo) {

[Dan Beam, 2016/02/18 03:08:19]

setSearchEnginesInfo

[dpapad, 2016/02/18 19:34:51]

Done.

+          this.searchEnginesInfo_ = searchEnginesInfo;
+        },
+
+        /** @override */
+        getSearchEnginesList: function() {
+          this.resolverMap_.get('getSearchEnginesList').resolve();
+          return Promise.resolve(this.searchEnginesInfo_);
+        },
+
+        /** @override */
+        validateSearchEngineInput: function(fieldName, fieldValue) {
+          this.resolverMap_.get('validateSearchEngineInput').resolve();
+          return Promise.resolve(true);
+        },
+      };
+    });
+  }
+
+  /** @return {!SearchEngine} */
+  var getSampleSearchEngine = function() {

[Dan Beam, 2016/02/18 03:08:19]

get -> create

[dpapad, 2016/02/18 19:34:51]

Done.

+    return {
+      canBeDefault: false,
+      canBeEdited: true,
+      canBeRemoved: false,
+      default: false,
+      displayName: "Google",
+      iconURL: "http://www.google.com/favicon.ico",
+      isOmniboxExtension: false,
+      keyword: "google.com",
+      modelIndex: 0,
+      name: "Google",
+      url: "https://search.foo.com/search?p=%s",
+      urlLocked: false,
+    };
+  };
+

[Dan Beam, 2016/02/18 03:08:19]

\n\n -> \n

[dpapad, 2016/02/18 19:34:51]

Done.

+
+  function registerDialogTests() {
+    suite('AddSearchEngineDialogTests', function() {
+      /** @type {?SettingsAddSearchEngineDialog} */
+      var dialog = null;
+      var browserProxy = null;
+
+      suiteSetup(function() {
+        return Promise.all([
+          defineTestBrowserProxy(),
+          PolymerTest.importHtml('chrome://md-settings/i18n_setup.html'),
+          PolymerTest.importHtml(
+              'chrome://md-settings/search_engines_page/search_engine_dialog.html'),
+        ]);
+      });
+
+      setup(function() {
+        browserProxy = new TestSearchEnginesBrowserProxy();
+        settings.SearchEnginesBrowserProxy.instance_ = browserProxy;
+        PolymerTest.clearBody();
+        dialog = document.createElement('settings-search-engine-dialog');
+        document.body.appendChild(dialog);

[Dan Beam, 2016/02/18 03:08:19]

is appending to the body required? what if we moved to ready() instead of
attached()?

[dpapad, 2016/02/18 19:34:51]

There are other elements that the "settings-search-engine-dialog" is using
internally that do work on attached (for example
https://github.com/PolymerElements/paper-input/blob/master/paper-input-behavi...),
so doing this would make this test very fragile (even if it passes for now,
which I did not try). I don't see a good reason to test the element while
detached, when in the prod code this element is attached to the DOM.

The testing methodology I am using in this file is very simple.
 - Start with the default page of CrSettingsBrowserTest (prefs.html).
 - Unit test an element by individually loading it (for example
search-engine-dialog, search-engine-entry, search-engine-page) and then doing
assertions.

[Dan Beam, 2016/02/18 20:32:28]

that's fine if other elements rely on being attached
we would ideally be loading a specific component's .html file (i.e.
search_engine_entry.html) which would have the proper dependencies
right

[Dan Beam, 2016/02/18 20:37:55]

here's an example of this:
https://codereview.chromium.org/1700273003/diff/80001/chrome/test/data/webui/...
https://codereview.chromium.org/1700273003/diff/80001/chrome/test/data/webui/...

+      });
+
+      teardown(function() { dialog.remove(); });
+
+      // Tests that the dialog calls 'searchEngineEditStarted' and
+      // 'searchEngineEditCancelled' when closed from the 'x' button.
+      test('DialogOpenAndClose', function() {
+        return browserProxy.whenCalled('searchEngineEditStarted').then(
+            function() {
+              MockInteractions.tap(dialog.$.close);
+              return browserProxy.whenCalled('searchEngineEditCancelled');
+            });
+      });
+
+      // Tests that the dialog calls 'searchEngineEditStarted' and
+      // 'searchEngineEditCancelled' when closed from the 'cancel' button.
+      test('DialogOpenAndCancel', function() {
+        return browserProxy.whenCalled('searchEngineEditStarted').then(
+            function() {
+              MockInteractions.tap(dialog.$.cancel);
+              return browserProxy.whenCalled('searchEngineEditCancelled');
+            });
+      });
+
+      // Tests the dialog to add a new search engine. Specifically
+      //  - paper-input elements are empty initially.
+      //  - action button initially disabled.
+      //  - validation is triggered on 'focus'. 'change' is not teted because
+      //    MockInteractions does not currently provide a way to trigger such
+      //    events.
+      //  - action button is enabled when all fields are valid.
+      //  - action button triggers appropriate browser signal when tapped.
+      test('DialogAddSearchEngine', function() {
+        /**
+         * Asserts that the given paper-input element is empty.
+         * @param {string} inputId
+         */
+        var assertInputEmpty = function(inputId) {
+          var inputElement = dialog.$[inputId];
+          assertTrue(!!inputElement);
+          assertEquals(0, inputElement.value.length);
+        };
+
+        /**
+         * Asserts that the given paper-input element triggers validation when
+         * focused.
+         * @param {string} inputId
+         * @return {!Promise} A promise firing when assertion has completed.
+         */
+        var assertInputValidation = function(inputId) {

[Dan Beam, 2016/02/18 03:08:19]

can this be named focusAndValidate()?

[dpapad, 2016/02/18 19:34:51]

Done, but IMO "assert" prefix was making it clearer that this method is checking
for something, now it is not that clear that is part of the assertions this test
is making.

+          browserProxy.resetResolver('validateSearchEngineInput');
+          MockInteractions.focus(dialog.$[inputId]);
+          return browserProxy.whenCalled('validateSearchEngineInput');
+        };
+
+        assertInputEmpty('searchEngine');
+        assertInputEmpty('keyword');
+        assertInputEmpty('queryUrl');
+        var actionButton = dialog.$.actionButton;
+        assertTrue(actionButton.disabled);
+
+        return assertInputValidation('searchEngine').then(function() {
+          return assertInputValidation('keyword');
+        }).then(function() {
+          return assertInputValidation('queryUrl');
+        }).then(function() {
+          // Manually set the text to a non-empty string for all fields.
+          dialog.$.searchEngine.value = 'foo';
+          dialog.$.keyword.value = 'bar';
+          dialog.$.queryUrl.value = 'baz';
+
+          // MockInteractions does not provide a way to trigger a 'change' event
+          // yet. Triggering the 'focus' event instead, to update the state of
+          // the action button.
+          return assertInputValidation('searchEngine');
+        }).then(function() {
+          // Assert that the action button has been enabled now that all input
+          // is valid and non-empty.
+          assertFalse(actionButton.disabled);
+          MockInteractions.tap(actionButton);
+          return browserProxy.whenCalled('searchEngineEditCompleted');
+        });
+      });
+
+    });
+  }
+
+  function registerEntryTests() {
+    suite('SearchEngineEntryTests', function() {
+      /** @type {?SettingsSearchEngineEntryElement} */
+      var entry = null;
+
+      var browserProxy = null;
+
+      suiteSetup(function() {
+        return Promise.all([
+          defineTestBrowserProxy(),
+          PolymerTest.importHtml(
+              'chrome://md-settings/search_engines_page/search_engine_entry.html'),
+        ]);
+      });
+
+      setup(function() {
+        browserProxy = new TestSearchEnginesBrowserProxy();
+        settings.SearchEnginesBrowserProxy.instance_ = browserProxy;
+        PolymerTest.clearBody();
+        entry = document.createElement('settings-search-engine-entry');
+        entry.set('engine', getSampleSearchEngine());
+        document.body.appendChild(entry);
+      });
+
+      teardown(function() { entry.remove(); });
+
+      test('RemoveSearchEngine', function() {
+        var deleteButton = entry.$.delete;
+        assertTrue(!!deleteButton);
+        MockInteractions.tap(deleteButton);
+        return browserProxy.whenCalled('removeSearchEngine').then(
+            function(modelIndex) {
+              assertEquals(entry.engine.modelIndex, modelIndex);
+            });
+      });
+
+      test('MakeDefault', function() {
+        var makeDefaultButton = entry.$.makeDefault;
+        assertTrue(!!makeDefaultButton);
+        MockInteractions.tap(makeDefaultButton);
+        return browserProxy.whenCalled('setDefaultSearchEngine').then(
+            function(modelIndex) {
+              assertEquals(entry.engine.modelIndex, modelIndex);
+            });
+      });
+
+      // Test that the "make default" and "remove" buttons are hidden for
+      // the default search engine.
+      test('DefaultSearchEngineHiddenButtons', function() {
+        /**
+         * @param {string} buttonId
+         * @param {boolean} visible
+         */
+        var assertButtonVisibility = function(buttonId, visible) {
+          var buttonEl = entry.$[buttonId];
+          assertTrue(!!buttonEl);
+          assertEquals(!visible, buttonEl.hidden);
+        };
+        assertButtonVisibility('makeDefault', true);
+        assertButtonVisibility('edit', true);
+        assertButtonVisibility('delete', true);
+
+        // Mark the engine as the "default" one.
+        var engine = getSampleSearchEngine();
+        engine.default = true;
+        entry.set('engine', engine);
+
+        assertButtonVisibility('makeDefault', false);
+        assertButtonVisibility('edit', true);
+        assertButtonVisibility('delete', false);
+      });
+
+      // Test that clicking the "edit" button brings up a dialog.
+      test('Edit', function() {
+        var engine = entry.engine;
+        var editButton = entry.$.edit;
+        assertTrue(!!editButton);
+        MockInteractions.tap(editButton);
+        return browserProxy.whenCalled('searchEngineEditStarted').then(
+            function(modelIndex) {
+              assertEquals(engine.modelIndex, modelIndex);
+              var dialog = entry.$$('settings-search-engine-dialog');
+              assertTrue(!!dialog);
+
+              // Check that the paper-input fields are pre-populated.
+              assertEquals(engine.displayName, dialog.$.searchEngine.value);
+              assertEquals(engine.keyword, dialog.$.keyword.value);
+              assertEquals(engine.url, dialog.$.queryUrl.value);
+
+              assertFalse(dialog.$.actionButton.disabled);
+            });
+      });
+    });
+  }
+
+  function registerPageTests() {
+    suite('SearchEnginePageTests', function() {
+      /** @type {?SettingsSearchEnginesPageElement} */
+      var page = null;
+
+      var browserProxy = null;
+
+      /** @type {!SearchEnginesInfo} */
+      var searchEnginesInfo = {
+        defaults: [getSampleSearchEngine()],
+        others: [],
+        extensions: [],
+      };
+
+      suiteSetup(function() {
+        return Promise.all([
+          defineTestBrowserProxy(),
+          PolymerTest.importHtml('chrome://md-settings/i18n_setup.html'),
+          PolymerTest.importHtml(
+              'chrome://md-settings/search_engines_page/search_engines_page.html'),
+        ]);
+      });
+
+      setup(function() {
+        browserProxy = new TestSearchEnginesBrowserProxy();
+        browserProxy.setGetSearchEnginesList(searchEnginesInfo);
+        settings.SearchEnginesBrowserProxy.instance_ = browserProxy;
+        PolymerTest.clearBody();
+        page = document.createElement('settings-search-engines-page');
+        document.body.appendChild(page);
+      });
+
+      teardown(function() { page.remove(); });
+
+      // Tests that the page is querying and displaying search engine info on
+      // startup.
+      test('Initialization', function() {
+        return browserProxy.whenCalled('getSearchEnginesList').then(function() {
+          var searchEnginesLists = Polymer.dom(page.shadowRoot).
+              querySelectorAll('settings-search-engines-list');
+          assertEquals(2, searchEnginesLists.length);
+
+          Polymer.dom.flush();
+          var defaultsList = searchEnginesLists[0];
+          var defaultsEntries = Polymer.dom(defaultsList.shadowRoot).
+              querySelectorAll('settings-search-engine-entry');
+          assertEquals(
+              searchEnginesInfo.defaults.length, defaultsEntries.length);
+
+          var othersList = searchEnginesLists[1];
+          var othersEntries = Polymer.dom(othersList.shadowRoot).
+              querySelectorAll('settings-search-engine-entry');
+          assertEquals(
+              searchEnginesInfo.others.length, othersEntries.length);
+        });
+      });
+
+      // Tests that the add search engine dialog opens when the corresponding
+      // button is tapped.
+      test('AddSearchEngineDialog', function() {
+        assertFalse(!!page.$$('settings-search-engine-dialog'));
+        var addSearchEngineButton = page.$['addSearchEngine'];

[Dan Beam, 2016/02/18 03:08:19]

$.addSearchEngine

[dpapad, 2016/02/18 19:34:51]

Done, but never got a reply on why this is better. See my case at
https://codereview.chromium.org/1666623006/diff/280001/chrome/test/data/webui....

+        assertTrue(!!addSearchEngineButton);
+
+        MockInteractions.tap(addSearchEngineButton);
+        Polymer.dom.flush();
+        assertTrue(!!page.$$('settings-search-engine-dialog'));
+      });
+    });
+  }
+
+  return {
+    registerDialogTests: registerDialogTests,
+    registerEntryTests: registerEntryTests,
+    registerPageTests: registerPageTests,
+  };
+});