[MD settings] appearance browser tests

chrome/test/data/webui/settings/appearance_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 Polymer Appearance Settings elements. */
+/** @fileoverview Runs polymer appearance font settings elements. */
 
-GEN_INCLUDE(['settings_page_browsertest.js']);
-
-/**
- * Test Polymer Appearance Settings elements.
- * @constructor
- * @extends {SettingsPageBrowserTest}
-*/
-function AppearanceSettingsBrowserTest() {}
-
-AppearanceSettingsBrowserTest.prototype = {
-  __proto__: SettingsPageBrowserTest.prototype,
-
-  /** @return {string} */
-  appearancePage: function(selector) {
-    var section = this.getSection(this.getPage('basic'), 'appearance');
-    assertTrue(!!section);
-    var appearance = section.querySelector('settings-appearance-page');
-    assertTrue(!!appearance);
-    var result = appearance.$$(selector);
-    assertTrue(!!result);
-    return result;
-  },
-};
-
-TEST_F('AppearanceSettingsBrowserTest', 'uiTests', function() {
+cr.define('settings_appearance', function() {
   /**
-   * The prefs API that will get a fake implementation.
-   * @type {!SettingsPrivate}
+   * A test version of FontsBrowserProxy.
+   *
+   * @constructor
+   * @implements {settings.FontsBrowserProxy}
+   * @extends {settings.TestBrowserProxy}
    */
-  var settingsPrefs;
-  var self = this;
-
-  var fontSize = function() {
-    return self.appearancePage(
-        '#defaultFontSize').$$('[class=iron-selected]').textContent.trim();
+  var TestFontsBrowserProxy = function() {
+    settings.TestBrowserProxy.call(this, [
+      'fetchFontsData',
+    ]);
   };
 
-  suite('AppearanceHandler', function() {
-    suiteSetup(function() {
-      settingsPrefs = document.querySelector(
-          'cr-settings').$$('settings-prefs');
-      assertTrue(!!settingsPrefs);
-      return CrSettingsPrefs.initialized;
-    });
+  TestFontsBrowserProxy.prototype = {
+    __proto__: settings.TestBrowserProxy.prototype,
 
-    test('very small font', function() {
-      settingsPrefs.set('prefs.webkit.webprefs.default_font_size.value', 9);
-      assertEquals('Very small', fontSize());
-    });
-
-    test('large font', function() {
-      settingsPrefs.set('prefs.webkit.webprefs.default_font_size.value', 20);
-      assertEquals('Large', fontSize());
-    });
+    fontsData: {

[dpapad, 2016/04/06 17:14:55]

Since fontsData is not a function, it makes more sense to declare it in the
constructor instead of placing it on the prototype, for example see
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/data/w....

[dschuyler, 2016/04/07 20:47:42]

Done.

+      'fontList': [['font name', 'alternate', 'ltr']],
+      'encodingList': [['encoding name', 'alternate', 'ltr']],
+    },
 
     /**

[dpapad, 2016/04/06 17:14:55]

/** @override */

[dschuyler, 2016/04/07 20:47:42]

Done.

-     * If the font size is not one of the preset options (e.g. 16, 20, etc.)
-     * then the menu label will be 'Custom' (rather than 'Medium', 'Large',
-     * etc.).
      */
-    test('custom font size', function() {
-      settingsPrefs.set(
-          'prefs.webkit.webprefs.default_font_size.value', 19);
-      assertEquals('Custom', fontSize());
+    fetchFontsData: function() {
+      this.methodCalled('fetchFontsData');
+      return Promise.resolve(this.fontsData);
+    },
+  };
+
+  function registerAppearanceFontSettingsBrowserTest() {
+    var fontsEntry = null;

[dpapad, 2016/04/06 17:14:55]

Nit: s/fontsEntry/fontsPage or simply page?

[dschuyler, 2016/04/07 20:47:42]

Done.

+
+    /** @type {?TestFontsBrowserProxy} */
+    var fontsBrowserProxy = null;
+
+    suite('AppearanceFontHandler', function() {
+      setup(function() {
+        fontsBrowserProxy = new TestFontsBrowserProxy();
+        settings.FontsBrowserProxyImpl.instance_ = fontsBrowserProxy;
+
+        PolymerTest.clearBody();
+
+        fontsEntry = document.createElement('settings-appearance-fonts-page');
+        document.body.appendChild(fontsEntry);
+      });
+
+      teardown(function() { fontsEntry.remove(); });
+
+      test('fetchFontsData', function() {
+        var fetchPromise = fontsBrowserProxy.whenCalled('fetchFontsData');
+        var responsePromise = fontsBrowserProxy.fetchFontsData().then(

[dpapad, 2016/04/06 17:14:55]

|fetchPromise| tests that the actual production code is making a call to the
browser proxy, so it is useful, but I am confused by the existence of
|responsePromise|. It seems that it is testing the TestFontsBrwoserProxy itself.
Could you explain what are you trying to test here?

If you are simply trying to test that the appearance page makes a call to
"fetchFontsData" on startup then the first promise should be enough.

test('fetchFontsData', function() {
  return fontsBrowserProxy.whenCalled('fetchFontsData');
});

In addition to that I think it would make sense to test that |fontsEntry| is
correctly populated according to the mock data you are providing, by making some
assertions on the DOM structure. I don't see a reason the test should be calling
|fetchFontsData| directly.

[dschuyler, 2016/04/07 20:47:42]

Done.

+            function(response) {
+              assertGT(response.fontList.length, 0);
+              assertGT(response.encodingList.length, 0);
+              return true;
+            });
+        return Promise.all([fetchPromise, responsePromise]);
+      });
     });
-  });
-  mocha.run();
+  }
+
+  return {
+    registerTests: function() {
+      registerAppearanceFontSettingsBrowserTest();
+    },
+  };
 });