MD WebUI: Add tests for cr-toolbar-search-field

chrome/test/data/webui/cr_elements_browsertest.js

Index: chrome/test/data/webui/cr_elements_browsertest.js
diff --git a/chrome/test/data/webui/cr_elements_browsertest.js b/chrome/test/data/webui/cr_elements_browsertest.js
new file mode 100644
index 0000000000000000000000000000000000000000..bee6e1023775228270ffb9e227a136b02d912ed8
--- /dev/null
+++ b/chrome/test/data/webui/cr_elements_browsertest.js
@@ -0,0 +1,112 @@
+// 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.
+
+/** @fileoverview Suite of tests for cr elements. */
+
+/** @const {string} Path to source root. */
+var ROOT_PATH = '../../../../';
+
+// Polymer BrowserTest fixture.
+GEN_INCLUDE(
+    [ROOT_PATH + 'chrome/test/data/webui/polymer_browser_test_base.js']);
+
+/**
+ * Test fixture for cr elements
+ * @constructor
+ * @extends {PolymerTest}
+ */
+function CrElementsBrowserTest() {}
+
+CrElementsBrowserTest.prototype = {
+  __proto__: PolymerTest.prototype,
+  extraLibraries: PolymerTest.getLibraries(ROOT_PATH),
+};
+
+TEST_F('CrElementsBrowserTest', 'CrToolbarSearchFieldTest', function() {
+  suite('cr-toolbar-search-field', function() {
+    /** @type {CrToolbarSearchFieldElement} */
+    var field;
+    /** @type {Array<string>} */
+    var searches;

[dpapad, 2016/05/26 21:43:36]

How about making this field a member variable of the delegate instead (see
related following snippets)?

var delegate = null;

[tsergeant, 2016/05/27 01:20:13]

Done.

+
+    /** @param {string} term */
+    function mockSearch(term) {

[dpapad, 2016/05/26 21:43:37]

"mock" is an overloaded term IMO. Personally I think either of the following
names would be clearer
simulateSearch()
triggerSearch()

[tsergeant, 2016/05/27 01:20:13]

Done.

+      field.$.searchInput.bindValue = term;
+      field.onSearchTermSearch_();
+    }
+
+    /**
+     * @constructor
+     * @implements {SearchFieldDelegate}
+     */
+    function MockSearchFieldDelegate() {

[dpapad, 2016/05/26 21:43:36]

/**
  * @constructor
  * @implements {SearchFieldDelegate}
  */
function MockSearchFieldDelegate() {
  /** @type {!Array<string>} */
  this.searches = [];
}

MockSearchFieldDelegate.prototype = {
  /** @override */
  onSearchTermSearch: function(term) {
    searches.push(term);
  }
};

[dpapad, 2016/05/26 21:44:47]

Small correction on my previous suggestion:
this.searches.push(term);

[tsergeant, 2016/05/27 01:20:13]

Done.

+    }
+
+    MockSearchFieldDelegate.prototype = {
+      onSearchTermSearch: function(term) {
+        searches.push(term);
+      }
+    };
+
+    suiteSetup(function() {
+      return PolymerTest.importHtml(
+          'chrome://resources/cr_elements/cr_toolbar/' +
+          'cr_toolbar_search_field.html');
+    });
+
+    setup(function() {
+      PolymerTest.clearBody();
+      searches = [];

[dpapad, 2016/05/26 21:43:36]

// no need to instantiate both "searches" and a new delegate every time.
// Constructor does that now.
delegate = new MockSearchFieldDelegate();

[tsergeant, 2016/05/27 01:20:13]

Done.

+      field = document.createElement('cr-toolbar-search-field');
+      document.body.appendChild(field);
+      field.setDelegate(new MockSearchFieldDelegate());

[dpapad, 2016/05/26 21:43:36]

field.setDelegate(delegate);

[tsergeant, 2016/05/27 01:20:13]

Done.

+    });

[dpapad, 2016/05/26 21:43:36]

It is generally good practice to have tearDown undo what setUp has done before,
such that no unwanted state is leaked between tests.


tearDown(function() {
  field.remove();
  field = null;
});

[tsergeant, 2016/05/27 01:20:13]

Done.

+
+    test('opens and closes correctly', function() {
+      return Promise.resolve().then(function() {

[dpapad, 2016/05/26 21:43:36]

Why are you using a Promise here? This test does not do anything asynchronous.

[tsergeant, 2016/05/27 01:20:13]

CrSearchFieldBehavior.focus_() was previously async (due to the dom-if), which
is what I was hitting up against here.

I've removed the async bits from that method, and now this test can be
synchronous as well.

+        assertFalse(field.showingSearch);
+        MockInteractions.tap(field);
+        assertTrue(field.showingSearch);
+      }).then(function() {

[dpapad, 2016/05/26 21:43:36]

Same here (and below). Why is starting a new function here needed?

[tsergeant, 2016/05/27 01:20:14]

See above.

+        assertEquals(field.$.searchInput, field.root.activeElement);
+
+        MockInteractions.blur(field.$.searchInput);
+        assertFalse(field.showingSearch);
+
+        MockInteractions.tap(field);
+      }).then(function() {
+        assertEquals(field.$.searchInput, field.root.activeElement);
+        MockInteractions.pressAndReleaseKeyOn(
+            field.$.searchInput, 27, '', 'Escape');
+        assertFalse(field.showingSearch, 'Pressing escape closes field.');
+      }).then(function() {
+        assertNotEquals(field.$.searchInput, field.root.activeElement);
+      });
+    });
+
+    test('passes searches correctly', function() {
+      MockInteractions.tap(field);
+      mockSearch('test');
+      assertEquals('test', field.getValue());
+
+      MockInteractions.tap(field.$.clearSearch);
+      assertFalse(field.showingSearch);
+      assertEquals('', field.getValue());
+
+      assertEquals(2, searches.length);
+      assertEquals('test', searches[0]);
+      assertEquals('', searches[1]);
+    });
+
+    test('blur does not close field when a search is active', function() {
+      MockInteractions.tap(field);
+      mockSearch('test');
+      MockInteractions.blur(field.$.searchInput);
+
+      assertTrue(field.showingSearch);
+    });
+  });
+
+  mocha.run();
+});