← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~rvb/maas/maas-js-send-mockup into lp:maas

 

Raphaël Badin has proposed merging lp:~rvb/maas/maas-js-send-mockup into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/maas-js-send-mockup/+merge/99363

This branch fixes a problem in the way we test XHR calls in our JS tests.
The following pattern:

    mockXhr.send = function(url, cfg) {
        fired = true;
        Y.Assert.areEqual('testurl', url);
        cfg.on.failure();
        };
    this.mockIO(mockXhr, module);
    link.simulate('click');

…won't work if the Xhr call is triggered in an event listener and thus if the assertion happens in an event listener.  If the assertion (Y.Assert.areEqual('testurl', url)) fails, it will throw an exception that will be swallowed by the YUI event system.  Note that everything will be ok if the Xhr is not triggered in an event listener and this explain why most of our tests which were using this pattern worked.

I've improved mockSuccess and mockFailure to have them record the calls in order to change the pattern above into:

    var log = this.mockFailure('error message', module);
    link.simulate('click');
    Y.Assert.areEqual('testurl', log.pop()[0]);

This way, the assertion is *not* in an event listener no matter what.
-- 
https://code.launchpad.net/~rvb/maas/maas-js-send-mockup/+merge/99363
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/maas-js-send-mockup into lp:maas.
=== modified file 'src/maasserver/static/js/testing/testing.js'
--- src/maasserver/static/js/testing/testing.js	2012-03-26 04:48:59 +0000
+++ src/maasserver/static/js/testing/testing.js	2012-03-26 16:28:47 +0000
@@ -15,7 +15,7 @@
 function make_fake_response(response_text, status_code) {
     var out = {};
     // status_code defaults to undefined, since it's not always set.
-    if (status_code !== undefined) {
+    if (Y.Lang.isValue(status_code)) {
         out.status = status_code;
     }
     out.responseText = response_text;
@@ -116,7 +116,8 @@
     },
 
     /**
-     * Set up mockIO to feign successful I/O completion.
+     * Set up mockIO to feign successful I/O completion.  Returns an array
+     * where calls will be recorded.
      *
      * @method mockSuccess
      * @param response_text The response text to fake.  It will be available
@@ -127,17 +128,21 @@
      *     undefined, since the attribute may not always be available.
      */
     mockSuccess: function(response_text, module, status_code) {
+        var log = [];
         var mockXhr = new Y.Base();
         mockXhr.send = function(url, cfg) {
+            log.push([url, cfg]);
             var response = make_fake_response(response_text, status_code);
             var arbitrary_txn_id = '4';
             cfg.on.success(arbitrary_txn_id, response);
         };
         this.mockIO(mockXhr, module);
+        return log;
     },
 
     /**
-     * Set up mockIO to feign I/O failure.
+     * Set up mockIO to feign I/O failure.  Returns an array
+     * where calls will be recorded.
      *
      * @method mockFailure
      * @param response_text The response text to fake.  It will be available
@@ -148,13 +153,16 @@
      *     undefined, since the attribute may not always be available.
      */
     mockFailure: function(response_text, module, status_code) {
+        var log = [];
         var mockXhr = new Y.Base();
         mockXhr.send = function(url, cfg) {
+            log.push([url, cfg]);
             var response = make_fake_response(response_text, status_code);
             var arbitrary_txn_id = '4';
             cfg.on.failure(arbitrary_txn_id, response);
         };
         this.mockIO(mockXhr, module);
+        return log;
     }
 
 });

=== modified file 'src/maasserver/static/js/tests/test_longpoll.js'
--- src/maasserver/static/js/tests/test_longpoll.js	2012-03-13 17:13:01 +0000
+++ src/maasserver/static/js/tests/test_longpoll.js	2012-03-26 16:28:47 +0000
@@ -34,7 +34,7 @@
     setUp: function() {
         var old_repoll = longpoll._repoll;
         longpoll._repoll = false;
-        this.addCleanup(function() {longpoll._repoll = old_repoll});
+        this.addCleanup(function() {longpoll._repoll = old_repoll; });
     },
 
     tearDown: function() {
@@ -63,14 +63,10 @@
         Y.on(longpoll.longpoll_fail_event, function() {
             fired = true;
         });
-        // Monkeypatch io to simulate failure.
         var manager = longpoll.getLongPollManager();
-        var mockXhr = new Y.Base();
-        mockXhr.send = function(uri, cfg) {
-            cfg.on.failure();
-        };
-        this.mockIO(mockXhr, longpoll);
-        var manager = longpoll.setupLongPollManager('key', '/longpoll/');
+        // Simulate failure.
+        this.mockFailure('unused', longpoll);
+        longpoll.setupLongPollManager('key', '/longpoll/');
         Y.Assert.isTrue(fired, "Failure event not fired.");
     },
 
@@ -113,14 +109,10 @@
             shortdelay_event_fired = true;
         });
         var manager = longpoll.getLongPollManager();
-        // Monkeypatch io to simulate failure.
-        var mockXhr = new Y.Base();
-        mockXhr.send = function(uri, cfg) {
-            cfg.on.failure();
-        };
-        this.mockIO(mockXhr, longpoll);
+        // Simulate failure.
+        this.mockFailure('unused', longpoll);
         Y.Assert.areEqual(0, manager._failed_attempts);
-        var manager = longpoll.setupLongPollManager('key', '/longpoll/');
+        longpoll.setupLongPollManager('key', '/longpoll/');
         Y.Assert.areEqual(1, manager._failed_attempts);
         var i, delay;
         for (i=0; i<longpoll.MAX_SHORT_DELAY_FAILED_ATTEMPTS-2; i++) {
@@ -136,14 +128,10 @@
         Y.Assert.isTrue(longdelay_event_fired);
         Y.Assert.areEqual(delay, longpoll.LONG_DELAY);
 
-        // Monkeypatch io to simulate success.
-        var mockXhr = new Y.Base();
-        mockXhr.send = function(uri, cfg) {
-            var out = {};
-            out.responseText = Y.JSON.stringify({'event_key': 'response'});
-            cfg.on.success(4, out);
-        };
-        this.mockIO(mockXhr, longpoll);
+        // Simulate success.
+        this.mockSuccess(
+            Y.JSON.stringify({'event_key': 'response'}), longpoll);
+
         // After a success, longpoll.longpoll_shortdelay is fired.
         Y.Assert.isFalse(shortdelay_event_fired);
         delay = manager.poll();
@@ -156,26 +144,16 @@
         // /longpoll/?uuid=key&sequence=2
         // /longpoll/?uuid=key&sequence=3
         // ...
-        var count = 0;
-        // Monkeypatch io to simulate failure.
         var manager = longpoll.getLongPollManager();
-        var mockXhr = new Y.Base();
-        mockXhr.send = function(uri, cfg) {
-            Y.Assert.areEqual(
-                '/longpoll/?uuid=key&sequence=' + (count+1),
-                uri);
-            count = count + 1;
-            var response = {
-               responseText: '{"i":2}'
-            };
-            cfg.on.success(2, response);
-        };
-        this.mockIO(mockXhr, longpoll);
+        // Simulate success.
+        var log = this.mockSuccess('{"i":2}', longpoll);
         longpoll.setupLongPollManager('key', '/longpoll/');
         var request;
         for (request=1; request<10; request++) {
-            Y.Assert.isTrue(count === request, "Uri not requested.");
             manager.poll();
+            Y.Assert.areEqual(
+                '/longpoll/?uuid=key&sequence=' + (request + 1),
+                log.pop()[0]);
         }
     },
 
@@ -184,13 +162,8 @@
         // with code error_code, it is not treated as a failed
         // connection attempt.
         var manager = longpoll.getLongPollManager();
-        // Monkeypatch io to simulate a request timeout.
-        var mockXhr = new Y.Base();
-        mockXhr.send = function(uri, cfg) {
-            response = {status: error_code};
-            cfg.on.failure(4, response);
-        };
-        this.mockIO(mockXhr, longpoll);
+        // Simulate a request timeout.
+        this.mockFailure('{"i":2}', longpoll, error_code);
 
         Y.Assert.areEqual(0, manager._failed_attempts);
         longpoll.setupLongPollManager('key', '/longpoll/');
@@ -225,22 +198,20 @@
             'event_key': 'my-event',
             'something': {something_else: 1234}
         };
-        var fired = false;
+        var event_payload = null;
         Y.on(custom_response.event_key, function(data) {
-            fired = true;
-            Y.Assert.areEqual(data, custom_response);
+            event_payload = data;
+            Y.Assert.areEqual(data, custom_response + '44');
         });
         var manager = longpoll.getLongPollManager();
-        // Monkeypatch io.
-        var mockXhr = new Y.Base();
-        mockXhr.send = function(uri, cfg) {
-            var out = {};
-            out.responseText = Y.JSON.stringify(custom_response);
-            cfg.on.success(4, out);
-        };
-        this.mockIO(mockXhr, longpoll);
+        // Simulate success.
+        this.mockSuccess(Y.JSON.stringify(custom_response), longpoll);
         longpoll.setupLongPollManager('key', '/longpoll/');
-        Y.Assert.isTrue(fired, "Custom event not fired.");
+        // Note that a utility to compare objects does not yet exist in YUI.
+        // http://yuilibrary.com/projects/yui3/ticket/2529868.
+        Y.Assert.areEqual('my-event', event_payload.event_key);
+        Y.Assert.areEqual(
+            1234, event_payload.something.something_else);
     }
 
 }));

=== modified file 'src/maasserver/static/js/tests/test_node_add.js'
--- src/maasserver/static/js/tests/test_node_add.js	2012-03-26 05:47:25 +0000
+++ src/maasserver/static/js/tests/test_node_add.js	2012-03-26 16:28:47 +0000
@@ -113,17 +113,10 @@
         module.showAddNodeWidget();
         var mockXhr = new Y.Base();
         var form = find_form();
-        var fired = false;
-        var passed_form;
-        mockXhr.send = function(uri, cfg) {
-            fired = true;
-            passed_form = cfg.form;
-        };
-        this.mockIO(mockXhr, module);
+        var log = this.logIO(module);
         find_hostname_input().set('value', 'host');
         find_add_button().simulate('click');
-        Y.Assert.isTrue(fired);
-        Y.Assert.areEqual(form, passed_form.id);
+        Y.Assert.areEqual(form, log.pop()[1].form.id);
     },
 
     testAddNodeAPICall: function() {
@@ -154,11 +147,7 @@
     },
 
     testNodeidPopulation: function() {
-        var mockXhr = new Y.Base();
-        mockXhr.send = function(url, cfg) {
-            cfg.on.success(3, {response: Y.JSON.stringify({system_id: 3})});
-        };
-        this.mockIO(mockXhr, module);
+        this.mockSuccess(Y.JSON.stringify({system_id: 3}), module);
         module.showAddNodeWidget();
         this.addCleanup(
             Y.bind(

=== modified file 'src/maasserver/static/js/tests/test_prefs.js'
--- src/maasserver/static/js/tests/test_prefs.js	2012-03-15 13:58:32 +0000
+++ src/maasserver/static/js/tests/test_prefs.js	2012-03-26 16:28:47 +0000
@@ -57,21 +57,16 @@
 
     testDeleteTokenCall: function() {
         // A click on the delete link calls the API to delete a token.
-        var mockXhr = new Y.Base();
-        var fired = false;
-        mockXhr.send = function(url, cfg) {
-            fired = true;
-            Y.Assert.areEqual(MAAS_config.uris.account_handler, url);
-            Y.Assert.areEqual(
-                "op=delete_authorisation_token&token_key=tokenkey1",
-                cfg.data);
-        };
-        this.mockIO(mockXhr, module);
+        var log = this.logIO(module);
         var widget = this.createWidget();
         widget.render();
         var link = widget.get('srcNode').one('.delete-link');
         link.simulate('click');
-        Y.Assert.isTrue(fired);
+        var request_info = log.pop();
+        Y.Assert.areEqual(MAAS_config.uris.account_handler, request_info[0]);
+        Y.Assert.areEqual(
+            "op=delete_authorisation_token&token_key=tokenkey1",
+             request_info[1].data);
     },
 
     testDeleteTokenCallsAPI: function() {
@@ -86,11 +81,7 @@
     testDeleteTokenFail404DisplaysErrorMessage: function() {
         // If the API call to delete a token fails with a 404 error,
         // an error saying that the key has already been deleted is displayed.
-        var mockXhr = new Y.Base();
-        mockXhr.send = function(url, cfg) {
-            cfg.on.failure(3, {status: 404});
-        };
-        this.mockIO(mockXhr, module);
+        this.mockFailure('unused', module, 404);
         var widget = this.createWidget();
         widget.render();
         var link = widget.get('srcNode').one('.delete-link');
@@ -102,11 +93,7 @@
 
     testDeleteTokenFailDisplaysErrorMessage: function() {
         // If the API call to delete a token fails, an error is displayed.
-        var mockXhr = new Y.Base();
-        mockXhr.send = function(url, cfg) {
-            cfg.on.failure(3, {status: 500});
-        };
-        this.mockIO(mockXhr, module);
+        this.mockFailure('unused', module, 500);
         var widget = this.createWidget();
         widget.render();
         var link = widget.get('srcNode').one('.delete-link');
@@ -119,19 +106,13 @@
     testDeleteTokenDisplay: function() {
         // When the token is successfully deleted by the API, the
         // corresponding row is deleted.
-        var mockXhr = new Y.Base();
-        var fired = false;
-        mockXhr.send = function(url, cfg) {
-            fired = true;
-            cfg.on.success(3);
-        };
-        this.mockIO(mockXhr, module);
+        var log = this.mockSuccess('unused', module);
         var widget = this.createWidget();
         widget.render();
         var link = widget.get('srcNode').one('.delete-link');
         Y.Assert.isNotNull(Y.one('#tokenkey1'));
         link.simulate('click');
-        Y.Assert.isTrue(fired);
+        Y.Assert.areEqual(1, log.length);
         Y.Assert.isNull(Y.one('#tokenkey1'));
         Y.Assert.isNotNull(Y.one('#tokenkey2'));
         Y.Assert.areEqual(1, widget.get('nb_tokens'));
@@ -159,37 +140,26 @@
     testCreateTokenCall: function() {
         // A click on the "create a new token" link calls the API to
         // create a token.
-        var mockXhr = new Y.Base();
-        var fired = false;
-        mockXhr.send = function(url, cfg) {
-            fired = true;
-            Y.Assert.areEqual(MAAS_config.uris.account_handler, url);
-            Y.Assert.areEqual(
-                "op=create_authorisation_token",
-                cfg.data);
-        };
-        this.mockIO(mockXhr, module);
+        var log = this.logIO(module);
         var widget = this.createWidget();
         widget.render();
         var create_link = widget.get('srcNode').one('#create_token');
         create_link.simulate('click');
-        Y.Assert.isTrue(fired);
+        var request_infos = log.pop();
+        Y.Assert.areEqual(MAAS_config.uris.account_handler, request_infos[0]);
+        Y.Assert.areEqual(
+            "op=create_authorisation_token",
+            request_infos[1].data);
     },
 
     testCreateTokenFail: function() {
         // If the API call to create a token fails, an error is displayed.
-        var mockXhr = new Y.Base();
-        var fired = false;
-        mockXhr.send = function(url, cfg) {
-            fired = true;
-            cfg.on.failure(3);
-        };
-        this.mockIO(mockXhr, module);
+        var log = this.mockFailure('unused', module);
         var widget = this.createWidget();
         widget.render();
         var create_link = widget.get('srcNode').one('#create_token');
         create_link.simulate('click');
-        Y.Assert.isTrue(fired);
+        Y.Assert.areEqual(1, log.length);
         Y.Assert.areEqual(
             'Unable to create a new token.',
             widget.get('srcNode').one('#create_error').get('text'));
@@ -198,23 +168,17 @@
     testCreateTokenDisplay: function() {
         // When a new token is successfully created by the API, a new
         // corresponding row is added.
-        var mockXhr = new Y.Base();
-        var fired = false;
-        mockXhr.send = function(url, cfg) {
-            fired = true;
-            var response = {
-                consumer_key: 'consumer_key',
-                token_key: 'token_key',
-                token_secret: 'token_secret'
-            };
-            cfg.on.success(3, {response: Y.JSON.stringify(response)});
+        var response = {
+            consumer_key: 'consumer_key',
+            token_key: 'token_key',
+            token_secret: 'token_secret'
         };
-        this.mockIO(mockXhr, module);
+        var log = this.mockSuccess(Y.JSON.stringify(response), module);
         var widget = this.createWidget();
         widget.render();
         var create_link = widget.get('srcNode').one('#create_token');
         create_link.simulate('click');
-        Y.Assert.isTrue(fired);
+        Y.Assert.areEqual(1, log.length);
         Y.Assert.areEqual(3, widget.get('nb_tokens'));
         Y.Assert.isNotNull(Y.one('#token_key'));
     }

=== modified file 'src/maasserver/static/js/tests/test_utils.js'
--- src/maasserver/static/js/tests/test_utils.js	2012-03-15 13:58:32 +0000
+++ src/maasserver/static/js/tests/test_utils.js	2012-03-26 16:28:47 +0000
@@ -207,18 +207,13 @@
         var widget = this.createWidget();
         widget._editing = true;
         widget.set('title', "SampleTitle");
-        var mockXhr = new Y.Base();
-        var fired = false;
-        mockXhr.send = function(url, cfg) {
-            fired = true;
-            Y.Assert.areEqual(MAAS_config.uris.maas_handler, url);
-            Y.Assert.areEqual(
-                "op=set_config&name=maas_name&value=SampleTitle",
-                cfg.data);
-        };
-        this.mockIO(mockXhr, module);
+        var log = this.logIO(module);
         widget.titleEditEnd();
-        Y.Assert.isTrue(fired);
+        var req_args = log.pop();
+        Y.Assert.areEqual(MAAS_config.uris.maas_handler, req_args[0]);
+        Y.Assert.areEqual(
+            'op=set_config&name=maas_name&value=SampleTitle',
+            req_args[1].data);
         Y.Assert.isFalse(widget._editing);
         Y.Assert.areEqual(
             "SampleTitle MAAS", widget.get('title'));