Next phase for chrome.networkingPrivate interface.

chrome/test/data/extensions/api_test/networking/test.js

Index: chrome/test/data/extensions/api_test/networking/test.js
diff --git a/chrome/test/data/extensions/api_test/networking/test.js b/chrome/test/data/extensions/api_test/networking/test.js
index 5503ced293932998012d9546437436cd5d410326..67046ec2366ad86059258324fb248f1b8b42f597 100644
--- a/chrome/test/data/extensions/api_test/networking/test.js
+++ b/chrome/test/data/extensions/api_test/networking/test.js
@@ -7,92 +7,186 @@ var callbackFail = chrome.test.callbackFail;
 var assertTrue = chrome.test.assertTrue;
 var assertEq = chrome.test.assertEq;
 
-chrome.test.runTests([
-  function getVisibleNetworks() {
+function connectListener_(network, done) {

[not at google - send to devlin, 2013/02/12 22:07:25]

why the trailing _?

[Greg Spencer (Chromium), 2013/02/12 23:11:02]

Because I was trying to indicate that these were "private" helpers.  I suppose I
could put them in a namespace instead.

+    var self = this;
+    var collectProperties_ = function(testingConnect, properties) {
+        var finishTest = function() {

[not at google - send to devlin, 2013/02/12 22:07:25]

just 2 space indent

[Greg Spencer (Chromium), 2013/02/12 23:11:02]

Done.

+            chrome.networkingPrivate.onNetworksChanged.removeListener(

[not at google - send to devlin, 2013/02/12 22:07:25]

just 2 space indent

[Greg Spencer (Chromium), 2013/02/12 23:11:02]

Done.

+                self.watchForConnect);
+            done();
+        }

[not at google - send to devlin, 2013/02/12 22:07:25]

semicolon after all of these

[Greg Spencer (Chromium), 2013/02/12 23:11:02]

Done.

+        var startDisconnect = function() {
+            chrome.networkingPrivate.startDisconnect(
+                network,
+                callbackPass(function() {}));
+        }
+        var startConnect = function() {
+            chrome.networkingPrivate.startConnect(
+                network,
+                callbackPass(function() {}));
+        }
+        if (properties.ConnectionState == "NotConnected") {
+            if (testingConnect)

[not at google - send to devlin, 2013/02/12 22:07:25]

I don't understand what this testingConnect is supposed to be doing. What does
the false value mean?

[Greg Spencer (Chromium), 2013/02/12 23:11:02]

I'll see if I can change the name to something more descriptive.  It's meant to
indicate that we're testing the "connect" transition as opposed to the
"disconnect" transition.

+                finishTest();
+            else
+                startConnect();
+        }
+        if (properties.ConnectionState == "Connected") {
+            if (testingConnect)
+                startDisconnect();
+            else
+                finishTest();
+        }
+    }

[not at google - send to devlin, 2013/02/12 22:07:25]

semicolon, 2 space indent after all of these

[Greg Spencer (Chromium), 2013/02/12 23:11:02]

Done.

+    this.watchForConnect = function(changes) {
+        assertEq([network], changes);
+        chrome.networkingPrivate.getProperties(
+            network, collectProperties_.bind(undefined, true));
+    }
+    this.watchForDisconnect = function(changes) {
+        assertEq([network], changes);
+        chrome.networkingPrivate.getProperties(
+            network, collectProperties_.bind(undefined, false));
+    }
+}
+
+function listListener_(network, expected, done) {

[not at google - send to devlin, 2013/02/12 22:07:25]

ditto all the stuff above.

Actually they apply to the whole file.

+    var self = this;
+    this.listenForChanges = function(list) {
+        assertEq(expected, list);
+        chrome.networkingPrivate.onNetworkListChanged.removeListener(
+            self.listenForChanges);
+        done();
+    }
+}
+
+/////////////////////////////////////////////////////
+// Tests
+
+function startConnect() {
+    chrome.networkingPrivate.startConnect("stub_wifi2",
+                                          callbackPass(function() {}));
+}
+
+function startDisconnect() {
+    chrome.networkingPrivate.startDisconnect("stub_wifi2",
+                                             callbackPass(function() {}));
+}
+
+function startConnectNonexistent() {
+    // Make sure we get an error when we try to connect to a nonexistent
+    // network.
+    chrome.networkingPrivate.startConnect(
+        "nonexistent_path",
+        callbackFail("Error.InvalidService", function() {}));
+}
+
+function getVisibleNetworks() {
     chrome.networkingPrivate.getVisibleNetworks(
         "All",
         callbackPass(function(result) {
-          assertTrue(!!result);
-          assertEq(4, result.length);
-          assertEq([{ "Name": "eth0",
-                      "GUID": "stub_ethernet",
-                      "ConnectionState": "Connected",
-                      "Type": "Ethernet"
-                    },
-                    { "Name": "wifi1",
-                      "GUID": "stub_wifi1",
-                      "ConnectionState": "Connected",
-                      "Type": "WiFi",
-                      "WiFi": {
+            assertTrue(!!result);
+            assertEq(4, result.length);
+            assertEq([
+                {
+                    "ConnectionState": "Connected",
+                    "GUID": "stub_ethernet",
+                    "Name": "eth0",
+                    "Type": "Ethernet"
+                },
+                {
+                    "ConnectionState": "Connected",
+                    "GUID": "stub_wifi1",
+                    "Name": "wifi1",
+                    "Type": "WiFi",
+                    "WiFi": {
                         "SSID": "stub_wifi1",
                         "Type": "WiFi"
-                      }
-                    },
-                    { "Name": "wifi2_PSK",
-                      "GUID": "stub_wifi2",
-                      "ConnectionState": "NotConnected",
-                      "Type": "WiFi",
-                      "WiFi": {
+                    }
+                },
+                {
+                    "ConnectionState": "NotConnected",
+                    "GUID": "stub_wifi2",
+                    "Name": "wifi2_PSK",
+                    "Type": "WiFi",
+                    "WiFi": {
                         "SSID": "stub_wifi2",
                         "Type": "WiFi"
-                      }
-                    },
-                    { "Name": "cellular1",
-                      "GUID": "stub_cellular1",
-                      "ConnectionState": "NotConnected",
-                      "Type": "Cellular"
-                    }], result);
+                    }
+                },
+                {
+                    "ConnectionState": "NotConnected",
+                    "GUID": "stub_cellular1",
+                    "Name": "cellular1",
+                    "Type": "Cellular"
+                }
+            ], result);
         }));
-  },
-  function getVisibleNetworksWifi() {
+}
+
+function getVisibleNetworksWifi() {
     chrome.networkingPrivate.getVisibleNetworks(
-      "WiFi",
-      callbackPass(function(result) {
-        assertTrue(!!result);
-        assertEq(2, result.length);
-        assertEq([{ "Name": "wifi1",
-                    "GUID": "stub_wifi1",
+        "WiFi",
+        callbackPass(function(result) {
+            assertTrue(!!result);
+            assertEq(2, result.length);
+            assertEq([
+                {
                     "ConnectionState": "Connected",
+                    "GUID": "stub_wifi1",
+                    "Name": "wifi1",
                     "Type": "WiFi",
                     "WiFi": {
-                      "SSID": "stub_wifi1",
-                      "Type":"WiFi"
+                        "SSID": "stub_wifi1",
+                        "Type": "WiFi"
                     }
-                  },
-                  { "Name": "wifi2_PSK",
-                    "GUID": "stub_wifi2",
+                },
+                {
                     "ConnectionState": "NotConnected",
+                    "GUID": "stub_wifi2",
+                    "Name": "wifi2_PSK",
                     "Type": "WiFi",
                     "WiFi": {
-                      "SSID": "stub_wifi2",
-                      "Type": "WiFi"
+                        "SSID": "stub_wifi2",
+                        "Type": "WiFi"
                     }
-                  }], result);
-      }));
-  },
-  function getProperties() {
+                }
+            ], result);
+        }));
+}
+
+function getProperties() {
     chrome.networkingPrivate.getProperties(
         "stub_wifi2",
         callbackPass(function(result) {
-          assertTrue(!!result);
-          assertEq("wifi2_PSK", result.Name);
-          assertEq("NotConnected", result.ConnectionState);
-          assertEq("WiFi", result.Type);
+            assertTrue(!!result);
+            assertEq("wifi2_PSK", result.Name);
+            assertEq("NotConnected", result.ConnectionState);
+            assertEq("WiFi", result.Type);
         }));
-  },
-  function startConnect() {
-    chrome.networkingPrivate.startConnect("stub_wifi2",
+}
+
+function onNetworksChangedEvent() {
+    var network = "stub_wifi2";
+    var done = chrome.test.callbackAdded();
+    var listener = new connectListener_(network, done);
+    chrome.networkingPrivate.onNetworksChanged.addListener(
+        listener.watchForConnect);
+    chrome.networkingPrivate.startConnect(network,
                                           callbackPass(function() {}));
-  },
-  function startDisconnect() {
-    chrome.networkingPrivate.startDisconnect("stub_wifi2",
-                                             callbackPass(function() {}));
-  },
-  function startConnectNonexistent() {
-    // Make sure we get an error when we try to connect to a nonexistent
-    // network.
-    chrome.networkingPrivate.startConnect(
-      "nonexistent_path",
-      callbackFail("Error.InvalidService", function() {}));
-  }
-]);
+}
+
+function onNetworkListChangedEvent() {
+    var network = "stub_wifi2";
+    var expected = ["stub_wifi2","stub_ethernet", "stub_wifi1",
+                    "stub_cellular1"];
+    var done = chrome.test.callbackAdded();
+    var listener = new listListener_(network, expected, done);
+    chrome.networkingPrivate.onNetworkListChanged.addListener(
+        listener.listenForChanges);
+    chrome.networkingPrivate.startConnect(network,
+                                          callbackPass(function() {}));
+}
+
+var testToRun = window.location.hash.substring(1);

[not at google - send to devlin, 2013/02/12 22:07:25]

query string seems more idiomatic but it's also more verbose I suppose. Eh,
whatever.

[Greg Spencer (Chromium), 2013/02/12 23:11:02]

Not really.  I can switch to query string if you want.  Hash just seemed simple.

[not at google - send to devlin, 2013/02/12 23:36:09]

nah never mind.

+chrome.test.runTests([window[testToRun]]);

[not at google - send to devlin, 2013/02/12 22:07:25]

prefer to put the tests on a separate object rather than window[testToRun].

var tests = {
  getVisibleNetworksWifi: function() {
  },

  ...
};

would be nice.

[Greg Spencer (Chromium), 2013/02/12 23:11:02]

Yeah, I'd prefer that too, and started off with that, but the problem is that
then when a test fails, it prints "undefined" as the function name for the
failing test (or even when it succeeds).

I also tried:

var tests = {
  getVisibleNetworksWifi: function getVisibleNetworksWifi(){
  },
  ...
};

And that solved the name problem, but now you have to type it twice on the same
line, which seemed dumb.  Using window[testToRun] seemed the lesser evil.

[not at google - send to devlin, 2013/02/12 23:36:09]

I think that window[testToRun] is the greater evil :)