bluetooth: Basic browser tests for chrome://bluetooth-internals.

chrome/test/data/webui/bluetooth_internals_browsertest.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.
+
+/**
+ * @fileoverview Tests for chrome://bluetooth-internals
+ */
+
+/** @const {string} Path to source root. */
+var ROOT_PATH = '../../../../';
+
+/**
+ * Test fixture for BluetoothInternals WebUI testing.
+ * @constructor
+ * @extends testing.Test
+ */
+function BluetoothInternalsTest() {
+  this.adapterFactory = null;
+  this.setupResolver = new PromiseResolver();
+}
+
+BluetoothInternalsTest.prototype = {
+  __proto__: testing.Test.prototype,
+
+  /** @override */
+  browsePreload: 'chrome://bluetooth-internals',
+
+  /** @override */
+  isAsync: true,
+
+  /** @override */
+  runAccessibilityChecks: false,
+
+  /** @override */
+  extraLibraries: [
+    ROOT_PATH + 'third_party/mocha/mocha.js',
+    ROOT_PATH + 'chrome/test/data/webui/mocha_adapter.js',
+    ROOT_PATH + 'ui/webui/resources/js/promise_resolver.js',
+    ROOT_PATH + 'ui/webui/resources/js/cr.js',
+    ROOT_PATH + 'chrome/test/data/webui/settings/test_browser_proxy.js',
+  ],
+
+  preLoad: function() {
+    /**
+     * TODO(crbug.com/652361): Move to shared location.
+     * Helper to convert callback-based define() API to a promise-based API.
+     * @param {!Array<string>} moduleNames
+     * @return {!Promise}
+     */

[dpapad, 2016/11/02 00:03:00]

How many copies of this function do we currently have? This is adding yet
another one. Can we address this as part of this CL? Or in a separate CL that
lands before this CL?

[mbrunson, 2016/11/02 22:47:21]

Done. https://codereview.chromium.org/2428773005/

+    function importModules(moduleNames) {
+      return new Promise(function(resolve, reject) {
+        define(moduleNames, function(var_args) {
+          resolve(Array.prototype.slice.call(arguments, 0));
+        });
+      });
+    }
+
+    // A function that is called from chrome://bluetooth-internals to allow this
+    // test to replace the real Mojo browser proxy with a fake one, before any
+    // other code runs.
+    window.setupFn = function() {
+      return importModules([
+          'content/public/renderer/frame_interfaces',
+          'device/bluetooth/public/interfaces/adapter.mojom',
+          'device/bluetooth/public/interfaces/device.mojom',
+          'mojo/public/js/bindings',
+          'mojo/public/js/connection',
+        ])
+        .then(function(
+          [frameInterfaces, adapter, device, bindings, connection]) {
+          /**
+           * A test adapter factory proxy for the chrome://bluetooth-internals
+           * page.
+           *
+           * @constructor
+           * @extends {TestBrowserProxyBase}
+           */
+          var TestAdapterFactoryProxy = function() {
+            settings.TestBrowserProxy.call(this, [
+              'getAdapter',
+            ]);
+
+            this.adapter = new TestAdapter();
+          };
+          TestAdapterFactoryProxy.prototype = {
+            __proto__: settings.TestBrowserProxy.prototype,
+            getAdapter: function() {
+              this.methodCalled('getAdapter');
+
+              // Create message pipe bound to TestAdapter.
+              return Promise.resolve({
+                adapter: connection.bindStubDerivedImpl(this.adapter),

[dpapad, 2016/11/02 00:03:00]

Assuming this needs to be done every time getAdapter() is called, right?
Otherwise let's just call bindStubDerivedImpl it once in the constructor and
always return the same reference.

[mbrunson, 2016/11/02 22:47:23]

Done.

+              });
+            }
+          };
+
+          /**
+           * A test adapter for the chrome://bluetooth-internals page.
+           * Must be used to create message pipe handle from test code.
+           *
+           * @constructor
+           * @extends {adapter.Adapter.stubClass}
+           */
+          var TestAdapter = function() {

[dpapad, 2016/11/02 00:03:00]

Can you merge TestAdapterProxy class into TestAdapter? Why does TestAdapter need
to extend adapter.Adapter.stubClass?

[mbrunson, 2016/11/02 22:47:22]

connection.js documentation [1] says the stub passed into bindStubDerivedImpl
must be a subclass of an auto-generated stub class.

[1]
https://cs.chromium.org/chromium/src/mojo/public/js/connection.js?rcl=0&l=172

+            this.proxy = new TestAdapterProxy();
+          };

[dpapad, 2016/11/02 00:03:00]

\n

[mbrunson, 2016/11/02 22:47:24]

Done.

+          TestAdapter.prototype = {
+            __proto__: adapter.Adapter.stubClass.prototype,
+            getInfo: function() { return this.proxy.getInfo(); },
+            getDevices: function() { return this.proxy.getDevices(); },
+            setClient: function(client) { return this.proxy.setClient(client); }
+          };
+
+          /**
+           * A test adapter proxy for the chrome://bluetooth-internals page.
+           *
+           * @constructor
+           * @extends {TestBrowserProxyBase}
+           */
+          var TestAdapterProxy = function() {
+            settings.TestBrowserProxy.call(this, [
+              'getInfo',
+              'getDevices',
+              'setClient',
+            ]);
+
+            this.client = null;
+
+            this.adapterInfo_ = null;
+            this.devices_ = [];
+          };
+          TestAdapterProxy.prototype = {
+            __proto__: settings.TestBrowserProxy.prototype,
+            getInfo: function() {
+              this.methodCalled('getInfo');
+              return Promise.resolve({ info: this.adapterInfo_ });
+            },
+            getDevices: function() {
+              this.methodCalled('getDevices');
+              return Promise.resolve({ devices: this.devices_ });
+            },
+            setClient: function(client) {
+              this.methodCalled('setClient', client);
+              this.client = connection.bindHandleToProxy(client,
+                adapter.AdapterClient);
+            },
+            setTestAdapter: function(adapterInfo) {
+              this.adapterInfo_ = adapterInfo;
+            },
+            setTestDevices: function(devices) {
+              this.devices_ = devices;
+            }
+          };
+
+
+          frameInterfaces.addInterfaceOverrideForTesting(
+            adapter.AdapterFactory.name,
+            function(handle) {
+              var stub = connection.bindHandleToStub(
+                handle, adapter.AdapterFactory);
+              this.adapterFactory = new TestAdapterFactoryProxy();
+              bindings.StubBindings(stub).delegate = this.adapterFactory;
+
+              this.setupResolver.resolve();
+            }.bind(this));
+
+        }.bind(this));
+    }.bind(this);
+  },
+};
+
+TEST_F('BluetoothInternalsTest', 'Startup_BluetoothInternals', function() {
+

[dpapad, 2016/11/02 00:03:00]

Nit: No need for blank line at the start of function.

[mbrunson, 2016/11/02 22:47:24]

Done.

+  var fakeAdapterInfo = {
+    address: '02:1C:7E:6A:11:5A',
+    discoverable: false,
+    discovering: false,
+    initialized: true,
+    name: 'computer.example.com-0',
+    powered: true,
+    present: true,
+  };
+
+  var fakeDeviceInfo1 = {
+    address: "AA:AA:84:96:92:84",
+    name: "AAA",
+    name_for_display: "AAA",
+    rssi: {value: -40},
+    services: []
+  };
+
+  var fakeDeviceInfo2 = {
+    address: "BB:BB:84:96:92:84",
+    name: "BBB",
+    name_for_display: "BBB",
+    rssi: null,
+    services: []
+  };
+
+  var fakeDeviceInfo3 = {
+    address: "CC:CC:84:96:92:84",
+    name: "CCC",
+    name_for_display: "CCC",
+  };
+
+  var adapterFactory = null;
+
+  // Before tests are run, make sure setup completes.
+  var setupPromise = this.setupResolver.promise.then(function() {
+    adapterFactory = this.adapterFactory;
+  }.bind(this));
+
+
+  suite('BluetoothInternalsUITest', function() {
+    var EXPECTED_DEVICES = 2;
+
+    suiteSetup(function() {
+      return setupPromise.then(function() {
+        adapterFactory.adapter.proxy.setTestDevices([
+          fakeDeviceInfo1,
+          fakeDeviceInfo2
+        ]);
+        adapterFactory.adapter.proxy.setTestAdapter(fakeAdapterInfo);
+
+        return Promise.all([
+          adapterFactory.whenCalled('getAdapter'),
+          adapterFactory.adapter.proxy.whenCalled('getInfo'),
+          adapterFactory.adapter.proxy.whenCalled('getDevices'),
+        ]);
+      });
+    });
+
+    setup(function() {
+      var devices = new device_collection.DeviceCollection([]);
+      adapterClient.devices = devices;
+      deviceTable.setDevices(devices);
+      adapterClient.deviceAdded(fakeDeviceInfo1);
+      adapterClient.deviceAdded(fakeDeviceInfo2);
+    });
+
+    teardown(function() {
+      adapterFactory.reset();
+    });
+
+    /**
+     * Updates device info and verifies the contents of the device table.
+     * @param {!device_collection.DeviceInfo} deviceInfo
+     */
+    function changeDevice(deviceInfo) {
+      var deviceRow = document.querySelector('#' + escapeDeviceAddress(
+          deviceInfo.address));
+      var nameForDisplayColumn = deviceRow.children[0];
+      var addressColumn = deviceRow.children[1];
+      var rssiColumn = deviceRow.children[2];
+      var servicesColumn = deviceRow.children[3];
+
+      expectTrue(!!nameForDisplayColumn);
+      expectTrue(!!addressColumn);
+      expectTrue(!!rssiColumn);
+      expectTrue(!!servicesColumn);
+
+      adapterClient.deviceChanged(deviceInfo);
+
+      expectEquals(deviceInfo.name_for_display,
+                   nameForDisplayColumn.textContent);
+      expectEquals(deviceInfo.address, addressColumn.textContent);
+
+      expectEquals(String(deviceInfo.rssi.value), rssiColumn.textContent);
+
+      if (deviceInfo.services) {
+        expectEquals(String(deviceInfo.services.length),
+                     servicesColumn.textContent);
+      } else {
+        expectEquals('Unknown',
+                     servicesColumn.textContent);

[dpapad, 2016/11/02 00:03:00]

Fits in previous line?

[mbrunson, 2016/11/02 22:47:21]

Done.

+      }
+    }
+
+    /**
+     * Escapes colons in a device address for CSS formatting.
+     * @param {!string} address
+     */
+    function escapeDeviceAddress(address) {
+      return address.replace(/:/g, '\\:');
+    }
+
+    /**
+     * Expects whether device is removed or not.
+     * @param {!boolean} expectRemoved
+     */
+    function expectDeviceRemoved(expectRemoved) {
+      var removedRow = document.querySelector('#' + escapeDeviceAddress(
+          newDeviceInfo.address));
+
+      if (expectRemoved) expectTrue(removedRow.classList.contains('removed'));
+      else expectFalse(removedRow.classList.contains('removed'));

[dpapad, 2016/11/02 00:03:00]

expectEquals(expectRemoved, !removedRow.classList.contains('removed'));

Also why using expect() instead of assert(), throughout this file?

[mbrunson, 2016/11/02 22:47:24]

The WebUI browser_tests best practices [1] says to prefer expect* over assert*
because it allows the rest of the test to run. So, I've just been making them
all expect*. There are probably some places where assert* can be used instead
though.

[1]
https://www.chromium.org/Home/domui-testing/webui-browser_tests#TOC-Best-prac...

[dpapad, 2016/11/02 23:19:14]

Ok.
The reality is that expect*() functions make WebUI tests tightly coupled with
the fairly old test_api.js framework (see [1]), because expect functions have
side-effects, where as assert functions are stateless, and I would like to
remove them in the future. It would be nice if at some point we don't need to
run nested test runner frameworks (mocha inside test_api.js), but I guess that
is a battle to be fought in the future.


[1]
https://cs.chromium.org/chromium/src/chrome/test/data/webui/test_api.js?l=169...

+    }
+
+    /**
+     * Tests whether a device is added successfully and not duplicated.
+     */
+    test('DeviceAdded', function() {
+      var devices = document.querySelectorAll('#device-table tbody tr');
+      expectEquals(EXPECTED_DEVICES, devices.length);
+
+      adapterClient.deviceAdded(fakeDeviceInfo3);
+
+      // Same device shouldn't appear twice.
+      adapterClient.deviceAdded(fakeDeviceInfo3);
+
+      devices = document.querySelectorAll('#device-table tbody tr');
+      expectEquals(EXPECTED_DEVICES + 1, devices.length);
+    });
+
+    /**
+     * Tests whether a device is marked properly as removed.
+     */
+    test('DeviceSetToRemoved', function() {
+      var devices = document.querySelectorAll('#device-table tbody tr');
+      expectEquals(EXPECTED_DEVICES, devices.length);
+      adapterClient.deviceRemoved(fakeDeviceInfo2);
+
+      // The number of rows shouldn't change.
+      devices = document.querySelectorAll('#device-table tbody tr');
+      expectEquals(EXPECTED_DEVICES, devices.length);
+
+      expectDeviceRemoved(true);
+    });
+
+    /**
+     * Tests whether a changed device updates the device table properly.
+     */
+    test('DeviceChanged', function() {
+      var devices = document.querySelectorAll('#device-table tbody tr');
+      expectEquals(EXPECTED_DEVICES, devices.length);
+
+      var newDeviceInfo = Object.assign({}, fakeDeviceInfo1);
+      newDeviceInfo.name_for_display = 'DDDD';
+      newDeviceInfo.rssi = { value: -20 };
+      newDeviceInfo.services = ['service1', 'service2', 'service3'];
+
+      changeDevice(newDeviceInfo);
+    });
+
+    /**
+     * Tests the entire device cycle: added -> updated -> removed -> re-added.
+     */
+    test('DeviceUpdateCycle', function() {
+      var devices = document.querySelectorAll('#device-table tbody tr');
+      expectEquals(EXPECTED_DEVICES, devices.length);
+
+      adapterClient.deviceAdded(fakeDeviceInfo3);
+
+      var newDeviceInfo = Object.assign({}, fakeDeviceInfo3);
+      newDeviceInfo.name_for_display = 'DDDD';
+      newDeviceInfo.rssi = { value: -20 };
+      newDeviceInfo.services = ['service1', 'service2', 'service3'];
+
+      changeDevice(newDeviceInfo);
+      changeDevice(fakeDeviceInfo3);
+
+      adapterClient.deviceRemoved(fakeDeviceInfo3);
+      expectDeviceRemoved(true);
+
+      adapterClient.deviceAdded(fakeDeviceInfo3);
+      expectDeviceRemoved(false);
+    });
+  });
+
+  // Run all registered tests.
+  mocha.run();
+});