launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #07830
[Merge] lp:~jtv/maas/pre-963090 into lp:maas
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/pre-963090 into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/pre-963090/+merge/105622
This addresses some small things I ran into while trying to fix timing-dependent assertions in our JS tests. I did them in a separate branch because my main branch ran into some mysterious problems and I wanted as few unknowns in that mix as possible.
Things addressed:
* An animation in the view was started before its "end" handler was set up.
* Some dashboard views created in the tests were not destroyed; not sure if it was causing trouble.
As a free bonus, I punctuated sentences, and double-quoted free-form text (where an apostrophe should always feel welcome).
Jeroen
--
https://code.launchpad.net/~jtv/maas/pre-963090/+merge/105622
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/pre-963090 into lp:maas.
=== modified file 'src/maasserver/static/js/tests/test_node_views.js'
--- src/maasserver/static/js/tests/test_node_views.js 2012-05-11 11:05:45 +0000
+++ src/maasserver/static/js/tests/test_node_views.js 2012-05-14 08:55:23 +0000
@@ -125,20 +125,31 @@
];
},
+ /**
+ * Create a dashboard view, render it, and arrange for its cleanup.
+ *
+ * The "data" parameter defaults to this.data.
+ */
+ makeDashboard: function(data) {
+ if (data === undefined) {
+ data = this.data;
+ }
+ var view = create_dashboard_view(data, this);
+ this.addCleanup(function() { view.destroy(); });
+ view.render();
+ return view;
+ },
+
testInitializer: function() {
- var view = create_dashboard_view(this.data, this);
- this.addCleanup(function() { view.destroy(); });
- view.render();
+ var view = this.makeDashboard();
Y.Assert.areNotEqual(
'',
Y.one('#chart').get('text'),
- 'The chart node should have been populated');
+ "The chart node should have been populated.");
},
testHoverMouseover: function() {
- var view = create_dashboard_view(this.data, this);
- this.addCleanup(function() { view.destroy(); });
- view.render();
+ var view = this.makeDashboard();
// Chart hovers should be set up
Y.one(view.chart._offline_circle[0].node).simulate('mouseover');
@@ -146,156 +157,148 @@
Y.Assert.areEqual(
'4',
Y.one('#nodes-number').get('text'),
- 'The total number of offline nodes should be set');
+ "The total number of offline nodes should be set.");
Y.Assert.areEqual(
'nodes offline',
Y.one('#nodes-description').get('text'),
- 'The text should be set with nodes as a plural');
+ "The text should be set with nodes as a plural.");
}, 500);
},
testHoverMouseout: function() {
- var view = create_dashboard_view(this.data, this);
- this.addCleanup(function() { view.destroy(); });
- view.render();
+ var view = this.makeDashboard();
Y.one(view.chart._offline_circle[0].node).simulate('mouseout');
this.wait(function() {
Y.Assert.areEqual(
'12',
Y.one('#nodes-number').get('text'),
- 'The total number of nodes should be set');
+ "The total number of nodes should be set.");
Y.Assert.areEqual(
'nodes in this MAAS',
Y.one('#nodes-description').get('text'),
- 'The default text should be set');
+ "The default text should be set.");
}, 500);
},
testDisplay: function() {
- var view = create_dashboard_view(this.data, this);
- this.addCleanup(function() { view.destroy(); });
- view.render();
+ var view = this.makeDashboard();
// The number of nodes for each status should have been set
Y.Assert.areEqual(
1,
view.deployed_nodes,
- 'The number of deployed nodes should have been set');
+ "The number of deployed nodes should have been set.");
Y.Assert.areEqual(
2,
view.queued_nodes,
- 'The number of queued nodes should have been set');
+ "The number of queued nodes should have been set.");
Y.Assert.areEqual(
3,
view.reserved_nodes,
- 'The number of reserved nodes should have been set');
+ "The number of reserved nodes should have been set.");
Y.Assert.areEqual(
4,
view.offline_nodes,
- 'The number of offline nodes should have been set');
+ "The number of offline nodes should have been set.");
Y.Assert.areEqual(
2,
view.added_nodes,
- 'The number of added nodes should have been set');
+ "The number of added nodes should have been set.");
Y.Assert.areEqual(
1,
view.retired_nodes,
- 'The number of retired nodes should have been set');
+ "The number of retired nodes should have been set.");
Y.Assert.areEqual(
'12',
Y.one('#nodes-number').get('text'),
- 'The total number of nodes should be set');
+ "The total number of nodes should be set.");
Y.Assert.areEqual(
- 'nodes in this MAAS',
+ "nodes in this MAAS",
Y.one('#nodes-description').get('text'),
- 'The summary text should be set');
+ "The summary text should be set.");
Y.Assert.areEqual(
- '3 nodes reserved for named deployment.',
+ "3 nodes reserved for named deployment.",
Y.one('#reserved-nodes').get('text'),
- 'The reserved text should be set');
+ "The reserved text should be set.");
/* XXX: GavinPanella 2012-04-17 bug=984117:
* Hidden until we support reserved nodes. */
Y.Assert.areEqual("none", view.reservedNode.getStyle("display"));
Y.Assert.areEqual(
- '1 retired node not represented.',
+ "1 retired node not represented.",
Y.one('#retired-nodes').get('text'),
- 'The retired text should be set');
+ "The retired text should be set.");
/* XXX: GavinPanella 2012-04-17 bug=984116:
* Hidden until we support retired nodes. */
Y.Assert.areEqual("none", view.retiredNode.getStyle("display"));
},
testUpdateNodeCreation: function() {
- var view = create_dashboard_view(this.data, this);
+ var view = this.makeDashboard();
var node = {
system_id: 'sys14',
hostname: 'host14',
status: Y.maas.enums.NODE_STATUS.DECLARED
};
- this.addCleanup(function() { view.destroy(); });
- view.render();
Y.Assert.areEqual(
'12',
Y.one('#nodes-number').get('text'),
- 'The total number of nodes should be set');
+ "The total number of nodes should be set.");
// Check node creation
Y.Assert.areEqual(
2,
view.added_nodes,
- 'Check the initial number of nodes for the status');
+ "Check the initial number of nodes for the status.");
Y.fire('Node.created', {instance: node});
Y.Assert.areEqual(
'host14',
view.modelList.getById('sys14').get('hostname'),
- 'The node should exist in the modellist');
+ "The node should exist in the modellist.");
Y.Assert.areEqual(
3,
view.added_nodes,
- 'The status should have one extra node');
+ "The status should have one extra node.");
Y.Assert.areEqual(
3,
view.chart.get('added_nodes'),
- 'The chart status number should also be updated');
+ "The chart status number should also be updated.");
var self = this;
this.wait(function() {
Y.Assert.areEqual(
'13',
Y.one('#nodes-number').get('text'),
- 'The total number of nodes should have been updated');
+ "The total number of nodes should have been updated.");
}, 500);
},
testUpdateNodeUpdating: function() {
- var view = create_dashboard_view(this.data, this);
+ var view = this.makeDashboard();
var node = this.data[0];
- this.addCleanup(function() { view.destroy(); });
- view.render();
node.status = Y.maas.enums.NODE_STATUS.ALLOCATED;
Y.Assert.areEqual(
1,
view.deployed_nodes,
- 'Check the initial number of nodes for the new status');
+ "Check the initial number of nodes for the new status.");
Y.fire('Node.updated', {instance: node});
Y.Assert.areEqual(
6,
view.modelList.getById('sys1').get('status'),
- 'The node should have been updated');
+ "The node should have been updated.");
Y.Assert.areEqual(
2,
view.deployed_nodes,
- 'The new status should have one extra node');
+ "The new status should have one extra node.");
Y.Assert.areEqual(
2,
view.chart.get('deployed_nodes'),
- 'The new chart status number should also be updated');
+ "The new chart status number should also be updated.");
Y.Assert.areEqual(
1,
view.added_nodes,
- 'The old status count should have one less node');
+ "The old status count should have one less node.");
Y.Assert.areEqual(
1,
view.chart.get('added_nodes'),
- 'The old chart status number should also be updated');
+ "The old chart status number should also be updated.");
/* XXX: Bug: 963090 This is timing dependant and causes spurious
failures from time to time.
@@ -303,84 +306,80 @@
Y.Assert.areEqual(
Y.one('#nodes-number').get('text'),
'12',
- 'The total number of nodes should not have been updated');
+ "The total number of nodes should not have been updated.");
}, 500);
*/
},
testUpdateNodeDeleting: function() {
- var view = create_dashboard_view(this.data, this);
+ var view = this.makeDashboard();
var node = this.data[12];
- this.addCleanup(function() { view.destroy(); });
- view.render();
Y.fire('Node.deleted', {instance: node});
Y.Assert.isNull(
view.modelList.getById('sys14'),
- 'The node should have been deleted');
+ "The node should have been deleted.");
Y.Assert.areEqual(
1,
view.deployed_nodes,
- 'The status should have one less node');
+ "The status should have one less node.");
Y.Assert.areEqual(
1,
view.chart.get('deployed_nodes'),
- 'The chart status number should also be updated');
+ "The chart status number should also be updated.");
this.wait(function() {
Y.Assert.areEqual(
'12',
Y.one('#nodes-number').get('text'),
- 'The total number of nodes should have been updated');
+ "The total number of nodes should have been updated.");
}, 500);
},
testUpdateStatus: function() {
- var view = create_dashboard_view(this.data, this);
- this.addCleanup(function() { view.destroy(); });
- view.render();
+ var view = this.makeDashboard();
// Add a node to a status that also updates the chart
Y.Assert.areEqual(
2,
view.added_nodes,
- 'Check the initial number of nodes for the status');
+ "Check the initial number of nodes for the status.");
var result = view.updateStatus('add', 0);
Y.Assert.areEqual(
3,
view.added_nodes,
- 'The status should have one extra node');
+ "The status should have one extra node.");
Y.Assert.areEqual(
3,
view.chart.get('added_nodes'),
- 'The chart status number should also be updated');
+ "The chart status number should also be updated.");
Y.Assert.isTrue(
result,
- 'This status needs to update the chart, so it should return true');
+ "This status needs to update the chart, so should return true.");
// Remove a node from a status
result = view.updateStatus('remove', 0);
Y.Assert.areEqual(
2,
view.added_nodes,
- 'The status should have one less node');
+ "The status should have one less node.");
Y.Assert.areEqual(
2,
view.chart.get('added_nodes'),
- 'The chart status number should also be updated');
+ "The chart status number should also be updated.");
// Check a status that also updates text
Y.Assert.areEqual(
3,
view.reserved_nodes,
- 'Check the initial number of nodes for the reserved status');
+ "Check the initial number of nodes for the reserved status.");
result = view.updateStatus('add', 5);
Y.Assert.areEqual(
4,
view.reserved_nodes,
- 'The status should have one extra node');
+ "The status should have one extra node.");
Y.Assert.areEqual(
- '4 nodes reserved for named deployment.',
+ "4 nodes reserved for named deployment.",
Y.one('#reserved-nodes').get('text'),
- 'The dashboard reserved text should be updated');
+ "The dashboard reserved text should be updated.");
Y.Assert.isFalse(
result,
- 'This status should not to update the chart');
+ "This status should not to update the chart.");
},
testSetSummary: function() {
@@ -390,44 +389,40 @@
hostname: 'host9',
status: Y.maas.enums.NODE_STATUS.RESERVED
}];
- var view = create_dashboard_view(data, this);
- this.addCleanup(function() { view.destroy(); });
- view.render();
+ var view = this.makeDashboard(data);
view.setSummary(false);
Y.Assert.areEqual(
'1',
Y.one('#nodes-number').get('text'),
- 'The total number of nodes should be set');
+ "The total number of nodes should be set.");
Y.Assert.areEqual(
'node in this MAAS',
Y.one('#nodes-description').get('text'),
- 'The text should be set with nodes as singular');
+ "The text should be set with nodes as singular.");
// Test the default summary, with one node
- view = create_dashboard_view(this.data, this);
- view.render();
+ view = this.makeDashboard();
view.setSummary(false);
Y.Assert.areEqual(
'12',
Y.one('#nodes-number').get('text'),
- 'The total number of nodes should be set');
+ "The total number of nodes should be set.");
Y.Assert.areEqual(
'nodes in this MAAS',
Y.one('#nodes-description').get('text'),
- 'The text should be set with nodes as a plural');
+ "The text should be set with nodes as a plural.");
// Test we can set the summary for a particular status (multiple nodes)
- view = create_dashboard_view(this.data, this);
- view.render();
+ view = this.makeDashboard();
view.setSummary(false, 1, view.queued_template);
Y.Assert.areEqual(
'1',
Y.one('#nodes-number').get('text'),
- 'The total number of nodes should be set');
+ "The total number of nodes should be set.");
Y.Assert.areEqual(
'node queued',
Y.one('#nodes-description').get('text'),
- 'The text should be set with nodes as a plural');
+ "The text should be set with nodes as a plural.");
// Test the animation doesn't run if we have set it to run
var fade_out_anim = false;
@@ -436,17 +431,15 @@
this.wait(function() {
Y.Assert.isFalse(
fade_out_anim,
- 'The fade out animation should not have run');
+ "The fade out animation should not have run.");
Y.Assert.isFalse(
fade_in_anim,
- 'The fade in animation should not have run');
+ "The fade in animation should not have run.");
}, 500);
},
testSetSummaryAnimation: function() {
- var view = create_dashboard_view(this.data, this);
- this.addCleanup(function() { view.destroy(); });
- view.render();
+ var view = this.makeDashboard();
var fade_out_anim = false;
var fade_in_anim = false;
view.fade_out.on('end', function() {
@@ -459,47 +452,42 @@
this.wait(function() {
Y.Assert.isTrue(
fade_out_anim,
- 'The fade out animation should have run');
+ "The fade-out animation should have run.");
Y.Assert.isTrue(
fade_in_anim,
- 'The fade in animation should have run');
+ "The fade-in animation should have run.");
}, 500);
},
testSetNodeText: function() {
- var view = create_dashboard_view(this.data, this);
- this.addCleanup(function() { view.destroy(); });
- view.render();
+ var view = this.makeDashboard();
view.setNodeText(
view.reservedNode, view.reserved_template, view.reserved_nodes);
Y.Assert.areEqual(
- '3 nodes reserved for named deployment.',
+ "3 nodes reserved for named deployment.",
Y.one('#reserved-nodes').get('text'),
- 'The text should be set with nodes as a plural');
+ "The text should be set with nodes as a plural.");
var data = [{
system_id: 'sys9',
hostname: 'host9',
status: Y.maas.enums.NODE_STATUS.RESERVED
}];
- view = create_dashboard_view(data, this);
- view.render();
+ view = this.makeDashboard(data);
view.setNodeText(
view.reservedNode, view.reserved_template, view.reserved_nodes);
Y.Assert.areEqual(
- '1 node reserved for named deployment.',
+ "1 node reserved for named deployment.",
Y.one('#reserved-nodes').get('text'),
- 'The text should be set with nodes as singular');
+ "The text should be set with nodes as singular.");
},
testGetNodeCount: function() {
- var view = create_dashboard_view(this.data, this);
- this.addCleanup(function() { view.destroy(); });
- view.render();
+ var view = this.makeDashboard();
Y.Assert.areEqual(
12,
view.getNodeCount(),
- 'The total nodes should not return retired nodes');
+ "The total nodes should not return retired nodes.");
},
tearDown : function () {
Follow ups
