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

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

Index: chrome/test/data/webui/settings/search_engines_page_test.js
diff --git a/chrome/test/data/webui/settings/search_engines_page_test.js b/chrome/test/data/webui/settings/search_engines_page_test.js
new file mode 100644
index 0000000000000000000000000000000000000000..a3df7dc92d7f01a28a65c3dfa8d42c737d3f2a16
--- /dev/null
+++ b/chrome/test/data/webui/settings/search_engines_page_test.js
@@ -0,0 +1,432 @@
+// 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() {
+  /** @constructor */
+  var PromiseResolver = function() {
+    this.promise = new Promise(function(resolve, reject) {
+      this.resolve = resolve;
+      this.reject = reject;
+    }.bind(this));
+  };

[Dan Beam, 2016/02/17 02:21:36]

this is a copy.  can you share this code?

[dpapad, 2016/02/17 03:22:36]

Added a TODO. Prefer not to block this CL on this TODO, but will follow up.

+
+  // 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() {

[Dan Beam, 2016/02/17 02:21:36]

why is this a function just to call it twice and no-op the second+ times rather
than just statically initialized?

var testBrowserProxyLoaded = PolymerTest.importHtml(testBrowserProxyPath).then(

[dpapad, 2016/02/17 03:22:36]

Tried this, but PolymerTest.importHTML fails unless you call it from within
setUp(), see error.
 "Uncaught TypeError: Cannot read property 'push' of undefined", source:
polymer_browser_test_base.js (136)
because the underlying array is only initialized in setUp().

+    if (TestSearchEnginesBrowserProxy != null)
+      return Promise.resolve();
+
+    var browserProxyImportPath =
+        'chrome://md-settings/search_engines_page/search_engines_browser_proxy.html';
+    return PolymerTest.importHtml(browserProxyImportPath).then(function() {
+      /**
+       * A test version of SearchEnginesBrowserProxy. Provides helper methods for

[Dan Beam, 2016/02/17 02:21:36]

wrap at 80 cols

[dpapad, 2016/02/17 03:22:36]

Done.

+       * 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() {

[Dan Beam, 2016/02/17 02:21:36]

put this in ctor

[dpapad, 2016/02/17 03:22:36]

Done.

+          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
+         *     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) {
+          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() {
+    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,
+    };
+  };
+
+
+  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'),

[Dan Beam, 2016/02/17 02:21:36]

indent off

[dpapad, 2016/02/17 03:22:36]

Done.

+        ]);
+      });
+
+      setup(function() {
+        browserProxy = new TestSearchEnginesBrowserProxy();
+        settings.SearchEnginesBrowserProxy.instance_ = browserProxy;
+        PolymerTest.clearBody();
+        dialog = document.createElement('settings-search-engine-dialog');
+        document.body.appendChild(dialog);
+      });
+
+      teardown(function() {dialog.remove();});

[Dan Beam, 2016/02/17 02:21:36]

{\sdialog.remove();\s}

[dpapad, 2016/02/17 03:22:36]

Done.

+
+      // 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);

[Dan Beam, 2016/02/17 02:21:36]

how is this better than:

assertEquals('', dialog.$.thing.value);

?

[dpapad, 2016/02/17 03:22:36]

Not sure of what you are contrasting here. Length check vs empty string check?
They seem equivalent to me, not worth the ink of those lines. How is the
suggestion any better?

+        };
+
+        /**
+         * 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) {
+          browserProxy.resetResolver('validateSearchEngineInput');
+          MockInteractions.focus(dialog.$[inputId]);
+          return browserProxy.whenCalled('validateSearchEngineInput');
+        };
+        var actionButton = dialog.$['actionButton'];

[Dan Beam, 2016/02/17 02:21:36]

why is this better than .$.actionButton?

[Dan Beam, 2016/02/17 02:21:36]

lower definition right before use

[dpapad, 2016/02/17 03:22:36]

Done. 

Why better?
1) If we ever compile this, compiler will not complain about former, but will
about latter (unless PolymerPass is that good, which will be nice).
2) In my editor the foo.$['bar'] is more readable, because 'bar' stands out in
red font VS foo.$.bar which is all white and nothing stands out.

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

polymer pass does have a renaming map, using ['id'] instead of .id would
probably be more hurtful than helpful (surprisingly)
['bar'] is actually creating a string whereas .bar is a property accessor

+
+        assertInputEmpty('searchEngine');
+        assertInputEmpty('keyword');
+        assertInputEmpty('queryUrl');
+        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);

[Dan Beam, 2016/02/17 02:21:36]

how is this better than

  assertFalse(entry.$.someId.hidden);
  assertTrue(entry.$.someId.hidden);

[dpapad, 2016/02/17 03:22:36]

Because I don't have to if branch as follows
if (visible)
  assertTrue(entry.$.someId.hidden);
else
  assertFalse(entry.$.someId.hidden);

I can just use assertEquals() in one line.

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

no, I mean

assertFalse(entry.$.makeDefault.hidden);
assertFalse(entry.$.edit.hidden);
assertFalse(entry.$.delete.hidden);

// Mark the engine as the "default" one.
var engine = getSampleSearchEngine();
engine.default = true;
entry.set('engine', engine);

assertTrue(entry.$.makeDefault.hidden);
assertTrue(entry.$.edit.hidden);
assertTrue(entry.$.delete.hidden);

[dpapad, 2016/02/18 20:58:42]

Done.

+        };
+        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);

[Dan Beam, 2016/02/17 02:21:35]

do you need to call Polymer.dom.flush() here?

[dpapad, 2016/02/17 03:22:36]

Well, it does not seem that this is needed (guessing because I am using the
Polymer helper method "set()"). I prefer not adding calls just to be sure,
confusing future readers.

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

your template binds hidden="[[entry.default]]" which might be updated
asynchronously.  Polymer.dom.flush() forces templates to re-render right away.

[dpapad, 2016/02/18 20:58:42]

Added a call to Polymer.dom.flush().

+
+        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);

[Dan Beam, 2016/02/17 02:21:35]

what's the point of this?  it'll just blow up on the next line

[dpapad, 2016/02/17 03:22:36]

Because the error message will be clearer if it blows up here. It also more
explicit IMO.

+
+              // 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'];
+        assertTrue(!!addSearchEngineButton);
+
+        MockInteractions.tap(addSearchEngineButton);
+        Polymer.dom.flush();
+        assertTrue(!!page.$$('settings-search-engine-dialog'));
+      });
+    });
+  }
+
+  return {
+    registerDialogTests: registerDialogTests,
+    registerEntryTests: registerEntryTests,
+    registerPageTests: registerPageTests,
+  };
+});