juju-gui

Merge lp:~frankban/juju-gui/fix-notification-tests into lp:juju-gui/experimental

  1. fix-notification-tests
  2. Merge into trunk
Proposed by Francesco Banconi
Status: Merged
Merged at revision: 279
Proposed branch: lp:~frankban/juju-gui/fix-notification-tests
Merge into: lp:juju-gui/experimental
Diff against target: 318 lines (+74/-159)
1 file modified
test/test_application_notifications.js (+74/-159)
To merge this branch: bzr merge lp:~frankban/juju-gui/fix-notification-tests
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+139677@code.launchpad.net

Description of the change

Restored notifications tests.

This branch restores the tests previously skipped
(as a consequence of the topology refactor) in
``test_application_notifications``.

Simplified the way the number of notifications is
tested: now, rather than checking a value in the DOM,
we check how many model instances are present in
the notifications model list.

Also added a test for the notifications view, that was
no longer exercised due to the change described above.

https://codereview.appspot.com/6942045/

To post a comment you must log in.
Revision history for this message
Francesco Banconi (frankban) wrote :

Reviewers: mp+139677_code.launchpad.net,

Message:
Please take a look.

Description:
Restored notifications tests.

This branch restores the tests previously skipped
(as a consequence of the topology refactor) in
``test_application_notifications``.

Simplified the way the number of notifications is
tested: now, rather than checking a value in the DOM,
we check how many model instances are present in
the notifications model list.

Also added a test for the notifications view, that was
no longer exercised due to the change described above.

https://code.launchpad.net/~frankban/juju-gui/fix-notification-tests/+merge/139677

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6942045/

Affected files:
   A [revision details]
   M test/test_application_notifications.js

Revision history for this message
Gary Poster (gary) wrote :

Land as is.

Really nice, Francesco. Thank you.

Gary

https://codereview.appspot.com/6942045/

Revision history for this message
Benjamin Saller (bcsaller) wrote :

As a fly by review, this branch made me happier than it should have,
Thanks.

https://codereview.appspot.com/6942045/

Revision history for this message
Francesco Banconi (frankban) wrote :

Hi Gary and Ben,

thanks for the reviews.

https://codereview.appspot.com/6942045/

Revision history for this message
Francesco Banconi (frankban) wrote :

*** Submitted:

Restored notifications tests.

This branch restores the tests previously skipped
(as a consequence of the topology refactor) in
``test_application_notifications``.

Simplified the way the number of notifications is
tested: now, rather than checking a value in the DOM,
we check how many model instances are present in
the notifications model list.

Also added a test for the notifications view, that was
no longer exercised due to the change described above.

R=gary.poster, benjamin.saller
CC=
https://codereview.appspot.com/6942045

https://codereview.appspot.com/6942045/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'test/test_application_notifications.js'
--- test/test_application_notifications.js 2012-12-06 17:46:42 +0000
+++ test/test_application_notifications.js 2012-12-13 11:59:44 +0000
@@ -1,14 +1,8 @@
1'use strict';1'use strict';
22
3describe('juju application notifications', function() {3describe('juju application notifications', function() {
4 var Y, juju, models, views, applicationContainer, notificationsContainer,4 var _setTimeout, _viewsHighlightRow, db, ERR_EV, juju, models, NO_OP,
5 viewContainer, db, _setTimeout, _viewsHighlightRow, ERR_EV, NO_OP;5 viewContainer, views, Y;
6
7 function assertNotificationNumber(value) {
8 assert.equal(
9 applicationContainer.one('#notify-indicator').getHTML().trim(),
10 value, 'The system didn\'t show the alert');
11 }
126
13 before(function() {7 before(function() {
14 Y = YUI(GlobalConfig).use(['node',8 Y = YUI(GlobalConfig).use(['node',
@@ -39,34 +33,12 @@
39 'juju-tests-utils',33 'juju-tests-utils',
40 'node-event-simulate'],34 'node-event-simulate'],
41 function(Y) {35 function(Y) {
42 applicationContainer = Y.Node.create('<div id="test-container" />');
43 applicationContainer.appendTo(Y.one('body'));
44
45 notificationsContainer = Y.Node.create('<div id="notifications" />');
46 notificationsContainer.appendTo(applicationContainer);
47
48 viewContainer = Y.Node.create('<div />');36 viewContainer = Y.Node.create('<div />');
49 viewContainer.appendTo(applicationContainer);37 viewContainer.appendTo(Y.one('body'));
38 viewContainer.hide();
5039
51 db = new models.Database();40 db = new models.Database();
5241
53 var notificationsView = new views.NotificationsView(
54 { container: notificationsContainer,
55 db: db,
56 env: {
57 on: NO_OP,
58 get: function(key) {
59 if (key === 'connected') {
60 return true;
61 }
62 return null;
63 }
64 },
65 notifications: db.notifications
66 });
67
68 notificationsView.render();
69
70 // The notifications.js delays the notification update.42 // The notifications.js delays the notification update.
71 // We are going to avoid this timeout to make it possible to test43 // We are going to avoid this timeout to make it possible to test
72 // the notification callback synchronously.44 // the notification callback synchronously.
@@ -83,11 +55,32 @@
83 });55 });
8456
85 afterEach(function() {57 afterEach(function() {
86 applicationContainer.remove(true);58 viewContainer.remove(true);
87 window.setTimeout = _setTimeout;59 window.setTimeout = _setTimeout;
88 views.highlightRow = _viewsHighlightRow;60 views.highlightRow = _viewsHighlightRow;
89 });61 });
9062
63 it('should notify errors in the notifications view', function() {
64 viewContainer.set('id', 'notifications');
65 var notificationsView = new views.NotificationsView({
66 container: viewContainer,
67 env: {
68 on: NO_OP,
69 get: function(key) {
70 if (key === 'connected') {
71 return true;
72 }
73 return null;
74 }
75 },
76 notifications: db.notifications
77 });
78 notificationsView.render();
79 var notification = new models.Notification({level: 'error'});
80 db.notifications.add(notification);
81 assert.equal('1', viewContainer.one('#notify-indicator').getHTML().trim());
82 });
83
91 it('should show notification for "add_unit" and "remove_units" exceptions' +84 it('should show notification for "add_unit" and "remove_units" exceptions' +
92 ' (service view)', function() {85 ' (service view)', function() {
93 var view = new views.service(86 var view = new views.service(
@@ -124,11 +117,11 @@
124117
125 // It triggers the "add unit" logic118 // It triggers the "add unit" logic
126 view._modifyUnits(3);119 view._modifyUnits(3);
127 assertNotificationNumber('1');120 assert.equal(1, db.notifications.size());
128121
129 // It triggers the "remove unit" logic122 // It triggers the "remove unit" logic
130 view._modifyUnits(1);123 view._modifyUnits(1);
131 assertNotificationNumber('2');124 assert.equal(2, db.notifications.size());
132 });125 });
133126
134 it('should show notification for "remove_units" and "resolved" exceptions' +127 it('should show notification for "remove_units" and "resolved" exceptions' +
@@ -173,7 +166,7 @@
173 view.remove_panel.footerNode.one('.btn-danger').simulate('click');166 view.remove_panel.footerNode.one('.btn-danger').simulate('click');
174 view.remove_panel.destroy();167 view.remove_panel.destroy();
175168
176 assertNotificationNumber('1');169 assert.equal(1, db.notifications.size());
177170
178 // Fake relation171 // Fake relation
179 db.relations.getById = function() {172 db.relations.getById = function() {
@@ -192,128 +185,50 @@
192 }185 }
193 });186 });
194187
195 assertNotificationNumber('2');188 assert.equal(2, db.notifications.size());
196 });189 });
197190
198 it.skip('should show notification for "add_relation" and "remove_relation"' +191 it('should add a notification for "addRelation" exceptions (env view)',
199 ' exceptions (environment view)', function() {192 function() {
200 var view = new views.environment({193 var view = new views.environment({db: db, container: viewContainer});
201 db: db,
202 container: viewContainer});
203 view.render();194 view.render();
195 var module = view.topo.modules.MegaModule;
196 // The callback wants to remove the pending relation from the db.
204 db.relations.remove = NO_OP;197 db.relations.remove = NO_OP;
205198 // The _addRelationCallback args are: view, relation id, event.
206 view.service_click_actions._addRelationCallback.apply(view,199 var args = [module, 'relation_id', ERR_EV];
207 [view, 'relation_id', ERR_EV]);200 module.service_click_actions._addRelationCallback.apply(module, args);
208201 assert.equal(1, db.notifications.size());
209 assertNotificationNumber('1');202 });
210203
211 //view, relationElement, relationId, confirmButton, ev204 it('should add a notification for "removeRelation" exceptions (env view)',
212 view._removeRelationCallback.apply(view, [{205 function() {
213 get: function() {return {hide: NO_OP, destroy: NO_OP};},206 var view = new views.environment({db: db, container: viewContainer});
214 removeSVGClass: NO_OP207 view.render();
215 }, {}, '', {208 var module = view.topo.modules.MegaModule;
216 set: NO_OP209 // The _removeRelationCallback args are: view, relation element,
217 }, ERR_EV]);210 // relation id, confirm button, event.
218211 var args = [
219 assertNotificationNumber('2');212 {get: function() {return {hide: NO_OP, destroy: NO_OP};},
220 });213 removeSVGClass: NO_OP
221214 }, {}, 'relation_id', {set: NO_OP}, ERR_EV
222 it.skip('should show notification for "add_relation" and "destroy_service"' +215 ];
223 ' exceptions (environment view)', function() {216 module._removeRelationCallback.apply(module, args);
224 var fakeLink = (function() {217 assert.equal(1, db.notifications.size());
225 var link = [{}, {}];218 });
226 link.enter = function() {219
227 return {220 it('should add a notification for "destroyService" exceptions (env view)',
228 insert: function() {221 function() {
229 return {222 var view = new views.environment({db: db, container: viewContainer});
230 attr: NO_OP223 view.render();
231 };224 var module = view.topo.modules.MegaModule;
232 }225 // The callback uses the 'getModelURL' attribute to retrieve the
233 };226 // service URL.
234 };227 module.set('getModelURL', NO_OP);
235 return link;228 // The _destroyCallback args are: service, view, confirm button, event.
236 })(),229 var args = [{}, module, {set: NO_OP}, ERR_EV];
237 env = {230 module.service_click_actions._destroyCallback.apply(module, args);
238 destroy_service: function(service, callback) {231 assert.equal(1, db.notifications.size());
239 callback(ERR_EV);
240 },
241 add_relation: function(endpoint_a, endpoint_b, callback) {
242 callback(ERR_EV);
243 }
244 },
245 view = {
246 set: NO_OP,
247 drawRelation: NO_OP,
248 cancelRelationBuild: NO_OP,
249
250 vis: {
251 selectAll: function() {
252 return {
253 data: function() {return fakeLink;}
254 };
255 }
256 },
257 removeSVGClass: NO_OP,
258 db: db,
259 destroy_service: {
260 get: NO_OP
261 },
262 env: env,
263 get: function(key) {
264 if ('getModelURL' === key) {
265 return NO_OP;
266 }
267 if ('updateEndpoints' === key) {
268 return NO_OP;
269 }
270 if ('env' === key) {
271 return env;
272 }
273 if ('addRelationStart_service' === key) {
274 return {};
275 }
276 if ('db' === key) {
277 return db;
278 }
279 if ('destroy_service' === key) {
280 return {
281 get: NO_OP
282 };
283 }
284 return null;
285 },
286 container: viewContainer,
287 _addRelationCallback: function() {
288 // Executing the "views.environment.prototype
289 // .service_click_actions._addRelationCallback" function
290 //instead.
291 views.environment.prototype.service_click_actions
292 ._addRelationCallback.apply(this, arguments);
293 },
294 _destroyCallback: function() {
295 // Executing the "views.environment.prototype
296 // .service_click_actions._destroyCallback" function
297 //instead.
298 views.environment.prototype.service_click_actions
299 ._destroyCallback.apply(this, arguments);
300 }
301 };
302
303 views.environment.prototype.service_click_actions.addRelationEnd
304 .apply(view, [
305 [
306 ['s1', {name: 'n', role: 'client'}],
307 ['s2', {name: 'n', role: 'server'}]],
308 view]);
309
310 assertNotificationNumber('1');
311
312 views.environment.prototype.service_click_actions.destroyService.apply(
313 //destroyService function signature > (m, view, btn)
314 view, [{}, view, {set: NO_OP}]);
315
316 assertNotificationNumber('2');
317 });232 });
318233
319 it('should show notification for "get_service" exceptions' +234 it('should show notification for "get_service" exceptions' +
@@ -338,7 +253,7 @@
338253
339 view.updateConstraints();254 view.updateConstraints();
340255
341 assertNotificationNumber('1');256 assert.equal(1, db.notifications.size());
342 });257 });
343258
344 it('should show notification for "get_service", "expose" and "unexpose"' +259 it('should show notification for "get_service", "expose" and "unexpose"' +
@@ -388,13 +303,13 @@
388 view.render();303 view.render();
389304
390 view.saveConfig();305 view.saveConfig();
391 assertNotificationNumber('1');306 assert.equal(1, db.notifications.size());
392307
393 view.exposeService();308 view.exposeService();
394 assertNotificationNumber('2');309 assert.equal(2, db.notifications.size());
395310
396 view.unexposeService();311 view.unexposeService();
397 assertNotificationNumber('3');312 assert.equal(3, db.notifications.size());
398 });313 });
399314
400 it('should show notification for "remove_relation"' +315 it('should show notification for "remove_relation"' +
@@ -437,7 +352,7 @@
437 view.remove_panel.footerNode.one('.btn-danger').simulate('click');352 view.remove_panel.footerNode.one('.btn-danger').simulate('click');
438 view.remove_panel.destroy();353 view.remove_panel.destroy();
439354
440 assertNotificationNumber('1');355 assert.equal(1, db.notifications.size());
441 });356 });
442357
443 it('should show notification for "deploy" exceptions (charm view)',358 it('should show notification for "deploy" exceptions (charm view)',

Subscribers

People subscribed via source and target branches