launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04053
[Merge] lp:~deryck/launchpad/speedup-yuitest into lp:launchpad
Deryck Hodge has proposed merging lp:~deryck/launchpad/speedup-yuitest into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~deryck/launchpad/speedup-yuitest/+merge/65825
This make the YUI test suite a bit faster (on the order of 20 seconds faster on my machine) by shortening wait times where possible. I was inspired by Curtis getting a YUI test layer working, his comment on odd wait times, and being at Velocity Conf.
The wait times we have in the test suite are all related to waiting on DOM animations to complete, usually sliding in and out an element. The first step was to fix lazr-js so that the effects slide anims could be monkey-patched to change the default slide duration. This is done and merged. So this branch makes use of that lazr-js version and sets all the durations to 0 or near 0 to get faster animations. The wait times can then be seriously reduced. I've tested in a real browser and with the test runner and all times work fine for me.
There were some other places in the test where wait times were just plain too high and could be set lower. Finally, I fixed FormActionsWidget to allow changing the anim duration as well, so that test could be sped up.
There are some cases that cannot be made faster as they are today, notably the subscriptions tests. Pulling a name out of the subscribers list (or adding one) seems to need about a second of animation time. This is a bit slow and is a sign we're likely doing too much DOM manipulation there. But the widget will need to be fixed to fix the test time.
With this branch landed, the test suite will be faster and we know that any test requiring a long wait time is either written wrong (i.e. not setting a shorter duration for the sake of the test) or testing a slow widget. Long wait times should be regarded as suspect when doing js code reviews going forward.
--
https://code.launchpad.net/~deryck/launchpad/speedup-yuitest/+merge/65825
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/speedup-yuitest into lp:launchpad.
=== modified file 'lib/lp/app/javascript/tests/test_lp_collapsibles.js'
--- lib/lp/app/javascript/tests/test_lp_collapsibles.js 2011-06-07 16:42:11 +0000
+++ lib/lp/app/javascript/tests/test_lp_collapsibles.js 2011-06-24 19:08:32 +0000
@@ -5,7 +5,7 @@
filter: 'raw',
combine: false,
fetchCSS: false
- }).use('test', 'console', 'lp', function(Y) {
+ }).use('test', 'console', 'lp', 'lazr.effects', function(Y) {
var Assert = Y.Assert; // For easy access to isTrue(), etc.
@@ -47,6 +47,15 @@
this.container.appendChild(
this.default_fieldset_node.cloneNode(
this.default_fieldset_node));
+
+ // Monkey patch effects duration to make effects instant.
+ // This keeps wait times to a minimum.
+ this.original_defaults = Y.lazr.effects.slide_effect_defaults;
+ Y.lazr.effects.slide_effect_defaults.duration = 0;
+ },
+
+ tearDown: function() {
+ Y.lazr.effects.slide_effect_defaults = this.original_defaults;
},
test_activate_collapsibles_creates_anchor: function() {
@@ -131,7 +140,7 @@
Assert.isTrue(wrapper_div.hasClass('lazr-closed'));
Assert.areNotEqual(
-1, icon.get('src').indexOf('/@@/treeCollapsed'));
- }, 500);
+ }, 20);
},
test_activate_collapsibles_doesnt_collapse_uncollapsed: function() {
@@ -146,7 +155,7 @@
Assert.isFalse(wrapper_div.hasClass('lazr-closed'));
Assert.areNotEqual(
-1, icon.get('src').indexOf('/@@/treeExpanded'));
- }, 500);
+ }, 20);
},
test_toggle_collapsible_opens_collapsed_collapsible: function() {
@@ -164,7 +173,7 @@
Assert.isFalse(wrapper_div.hasClass('lazr-closed'));
Assert.areNotEqual(
-1, icon.get('src').indexOf('/@@/treeExpanded'));
- }, 500);
+ }, 20);
},
test_toggle_collapsible_closes_open_collapsible: function() {
@@ -182,7 +191,7 @@
Assert.isTrue(wrapper_div.hasClass('lazr-closed'));
Assert.areNotEqual(
-1, icon.get('src').indexOf('/@@/treeCollapsed'));
- }, 500);
+ }, 20);
},
=== modified file 'lib/lp/bugs/javascript/tests/test_bug_notification_level.js'
--- lib/lp/bugs/javascript/tests/test_bug_notification_level.js 2011-06-08 16:04:02 +0000
+++ lib/lp/bugs/javascript/tests/test_bug_notification_level.js 2011-06-24 19:08:32 +0000
@@ -1,8 +1,8 @@
YUI({
base: '../../../../canonical/launchpad/icing/yui/',
filter: 'raw', combine: false, fetchCSS: false
- }).use('test', 'console', 'lp.bugs.bug_notification_level',
- 'node-event-simulate',
+ }).use('test', 'console', 'lazr.effects',
+ 'lp.bugs.bug_notification_level', 'node-event-simulate',
function(Y) {
var suite = new Y.Test.Suite("lp.bugs.bug_notification_level Tests");
@@ -117,10 +117,17 @@
suite.add(new Y.Test.Case({
name: 'Toggle visibility of the notification levels with animations',
+ setUp: function() {
+ // Monkey patch effects duration to make effects instant.
+ // This keeps wait times to a minimum.
+ this.original_defaults = Y.lazr.effects.slide_effect_defaults;
+ Y.lazr.effects.slide_effect_defaults.duration = 0;
+ },
+
tearDown: function() {
// Restore the default value.
module._bug_notification_level_visible = true;
-
+ Y.lazr.effects.slide_effect_defaults = this.original_defaults;
},
test_quick_close: function() {
@@ -142,7 +149,7 @@
// Wait for the animation to complete.
Y.Assert.isTrue(node.hasClass('lazr-closed'));
Y.Assert.isFalse(module._bug_notification_level_visible);
- }, 500);
+ }, 20);
},
test_show_node: function() {
@@ -154,7 +161,7 @@
// Wait for the animation to complete.
Y.Assert.isTrue(node.hasClass('lazr-opened'));
Y.Assert.isTrue(module._bug_notification_level_visible);
- }, 500);
+ }, 20);
},
test_show_and_hide: function() {
@@ -170,7 +177,7 @@
module._toggle_field_visibility(node);
// The slide-out animation should be stopped now.
Y.Assert.isFalse(module._slideout_animation.get('running'));
- }, 100);
+ }, 20);
}
}));
@@ -312,11 +319,16 @@
window.LP = { links: { me: "/~" + this.MY_NAME } };
this.root = Y.one('body').appendChild(
Y.Node.create('<div></div>'));
+ // Monkey patch effects duration to make effects instant.
+ // This keeps wait times to a minimum.
+ this.original_defaults = Y.lazr.effects.slide_effect_defaults;
+ Y.lazr.effects.slide_effect_defaults.duration = 0;
},
tearDown: function() {
delete window.LP;
this.root.empty();
+ Y.lazr.effects.slide_effect_defaults = this.original_defaults;
},
test_multiple_nodes_with_level_options: function() {
@@ -347,7 +359,7 @@
// Event is fired regardless.
this.wait(function() {
Y.Assert.isTrue(event_fired);
- }, 50);
+ }, 5);
},
test_single_option_no_animation: function() {
@@ -370,7 +382,7 @@
// Event is fired regardless.
this.wait(function() {
Y.Assert.isTrue(event_fired);
- }, 50);
+ }, 5);
},
test_animation_set_up: function() {
@@ -400,7 +412,7 @@
// And event is fired when the form set-up has been completed.
this.wait(function() {
Y.Assert.isTrue(event_fired);
- }, 50);
+ }, 5);
// Clicking the second option hides the initially shown
// notification level options.
@@ -419,8 +431,8 @@
Y.Assert.isTrue(level_node.hasClass('lazr-opened'));
Y.Assert.isFalse(level_node.hasClass('lazr-closed'));
Y.Assert.isTrue(module._bug_notification_level_visible);
- }, 500);
- }, 500);
+ }, 20);
+ }, 20);
}
}));
=== modified file 'lib/lp/bugs/javascript/tests/test_subscription.js'
--- lib/lp/bugs/javascript/tests/test_subscription.js 2011-06-16 13:50:58 +0000
+++ lib/lp/bugs/javascript/tests/test_subscription.js 2011-06-24 19:08:32 +0000
@@ -2,6 +2,7 @@
base: '../../../../canonical/launchpad/icing/yui/',
filter: 'raw', combine: false, fetchCSS: false
}).use('test', 'console', 'lp.bugs.subscription', 'node-event-simulate',
+ 'lazr.effects',
function(Y) {
var suite = new Y.Test.Suite("lp.bugs.subscription Tests");
@@ -1874,6 +1875,18 @@
suite.add(new Y.Test.Case({
name: 'Test creation of node describing all non-direct subscriptions.',
+ setUp: function() {
+ // Monkey patch effects duration to make effects instant.
+ // This keeps wait times to a minimum.
+ this.original_defaults = Y.lazr.effects.slide_effect_defaults;
+ Y.lazr.effects.slide_effect_defaults.duration = 0;
+ },
+
+ tearDown: function() {
+ Y.lazr.effects.slide_effect_defaults = this.original_defaults;
+ },
+
+
test_no_subscriptions: function() {
// With just a personal subscription, undefined is returned.
var info = {
@@ -2014,8 +2027,8 @@
Y.Assert.isFalse(link.hasClass('treeExpanded'));
Y.Assert.isTrue(list.hasClass('lazr-closed'));
delete window.LP;
- }, 500);
- }, 500);
+ }, 50);
+ }, 50);
}
}));
@@ -2060,7 +2073,7 @@
1, this.content_node.all('#direct-subscription').size());
Y.Assert.areEqual(
0, this.content_node.all('#other-subscriptions').size());
- }, 50);
+ }, 10);
delete window.LP;
},
@@ -2084,7 +2097,7 @@
Y.Assert.areEqual(
1, this.content_node.all('#other-subscriptions').size());
delete window.LP;
- }, 50);
+ }, 10);
},
test_reference_substitutions: function() {
@@ -2353,7 +2366,7 @@
function () {
Y.Assert.isNull(Y.one('.subscription-description'));
},
- 500
+ 50
);
},
@@ -2601,7 +2614,7 @@
node.simulate('click');
this.wait(function() {
Y.Assert.isTrue(node.hasClass('spinner'));
- }, 500);
+ }, 10);
module._lp_client.named_post.resume();
Y.Assert.isFalse(node.hasClass('spinner'));
},
@@ -2639,7 +2652,7 @@
// And afterwards it has been incremented to 1.
Y.Assert.areEqual(
1, window.LP.cache.bug_subscription_info.direct.count);
- }, 50);
+ }, 10);
},
test_on_mute_updates_info: function() {
@@ -2675,7 +2688,7 @@
// And afterwards it is still 1.
Y.Assert.areEqual(
1, window.LP.cache.bug_subscription_info.direct.count);
- }, 50);
+ }, 10);
},
test_on_unsubscribe_updates_info: function() {
@@ -2708,7 +2721,7 @@
// And afterwards it has been decremented to 2.
Y.Assert.areEqual(
2, window.LP.cache.bug_subscription_info.direct.count);
- }, 50);
+ }, 10);
},
test_on_unmute_updates_info: function() {
@@ -2741,7 +2754,7 @@
// And afterwards it has been decremented to 2.
Y.Assert.areEqual(
2, window.LP.cache.bug_subscription_info.direct.count);
- }, 50);
+ }, 10);
},
test_fail: function() {
=== modified file 'lib/lp/registry/javascript/distroseries.js'
--- lib/lp/registry/javascript/distroseries.js 2011-06-10 15:54:48 +0000
+++ lib/lp/registry/javascript/distroseries.js 2011-06-24 19:08:32 +0000
@@ -1149,6 +1149,21 @@
.superclass.constructor.apply(this, arguments);
};
+FormActionsWidget.ATTRS = {
+ duration: {
+ value: 1.0
+ },
+
+ height: {
+ value: 0
+ },
+
+ opacity: {
+ value: 0
+ }
+};
+
+
Y.mix(FormActionsWidget, {
NAME: 'formActionsWidget',
@@ -1255,9 +1270,9 @@
.addClass("message")
.set("text", message);
var form = this.get("contentBox").ancestor("form");
- form.transition(
- {duration: 1, height: 0, opacity: 0},
- function() { form.remove(true); });
+ form.transition({
+ duration: this.get('duration'), height: this.get('height'),
+ opacity: this.get('opacity')}, function() { form.remove(true); });
form.insert(messageNode, "after");
},
=== modified file 'lib/lp/registry/javascript/tests/test_distroseries.js'
--- lib/lp/registry/javascript/tests/test_distroseries.js 2011-06-20 18:44:35 +0000
+++ lib/lp/registry/javascript/tests/test_distroseries.js 2011-06-24 19:08:32 +0000
@@ -1038,6 +1038,7 @@
setUp: function() {
this.actions = this.makeActionsDiv();
this.widget = new initseries.DeriveDistroSeriesActionsWidget({
+ duration: 0,
srcNode: this.actions,
context: {
name: "hagfish",
@@ -1103,7 +1104,7 @@
this.wait(function() {
Assert.isFalse(
this.container.contains(this.form));
- }, 1100);
+ }, 30);
},
testSubmit: function() {
=== modified file 'lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.js'
--- lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.js 2011-06-08 16:19:52 +0000
+++ lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.js 2011-06-24 19:08:32 +0000
@@ -85,7 +85,7 @@
config.on.success(completed_voc);
}
};
- dsd_details.poll_interval = 2000;
+ dsd_details.poll_interval = 100;
},
test_request_wrong_click: function() {
@@ -150,8 +150,8 @@
Assert.isTrue(package_diff.hasClass('PENDING'));
this.wait(function() {
Assert.isTrue(package_diff.hasClass('COMPLETED'));
- }, 2500);
- }, 2500);
+ }, dsd_details.poll_interval);
+ }, dsd_details.poll_interval);
},
test_polling_for_pending_items: function() {
@@ -168,8 +168,8 @@
Assert.isTrue(package_diff.hasClass('PENDING'));
this.wait(function() {
Assert.isTrue(package_diff.hasClass('COMPLETED'));
- }, 2500);
- }, 2500);
+ }, dsd_details.poll_interval);
+ }, dsd_details.poll_interval);
}
};
=== modified file 'lib/lp/registry/javascript/tests/test_structural_subscription.js'
--- lib/lp/registry/javascript/tests/test_structural_subscription.js 2011-06-15 10:35:28 +0000
+++ lib/lp/registry/javascript/tests/test_structural_subscription.js 2011-06-24 19:08:32 +0000
@@ -5,8 +5,8 @@
filter: 'raw',
combine: false,
fetchCSS: false
- }).use('test', 'console', 'node', 'node-event-simulate', 'lp.client',
- 'lp.registry.structural_subscription', function(Y) {
+ }).use('test', 'console', 'node', 'node-event-simulate', 'lazr.effects',
+ 'lp.client', 'lp.registry.structural_subscription', function(Y) {
var suite = new Y.Test.Suite("Structural subscription overlay tests");
@@ -482,9 +482,15 @@
this.content_node = create_test_node();
Y.one('body').appendChild(this.content_node);
+
+ // Monkey patch effects duration to make effects instant.
+ // This keeps wait times to a minimum.
+ this.original_defaults = Y.lazr.effects.slide_effect_defaults;
+ Y.lazr.effects.slide_effect_defaults.duration = 0;
},
tearDown: function() {
+ Y.lazr.effects.slide_effect_defaults = this.original_defaults;
remove_test_node();
delete this.content_node;
},
@@ -541,12 +547,12 @@
added_changed.simulate('click');
this.wait(function() {
Assert.isTrue(filter_wrapper.hasClass('lazr-opened'));
- }, 500);
+ }, 10);
// Closes when deselected.
Y.one('#added-or-closed').simulate('click');
this.wait(function() {
Assert.isTrue(filter_wrapper.hasClass('lazr-closed'));
- }, 500);
+ }, 10);
},
test_advanced_filter_toggles: function() {
@@ -565,17 +571,17 @@
var accordion_wrapper = Y.one('#accordion-wrapper');
this.wait(function() {
Assert.isTrue(accordion_wrapper.hasClass('lazr-closed'));
- }, 500);
+ }, 10);
// Opens when selected.
advanced_filter.set('checked', true);
this.wait(function() {
Assert.isTrue(accordion_wrapper.hasClass('lazr-opened'));
- }, 500);
+ }, 10);
// Closes when deselected.
advanced_filter.set('checked', false);
this.wait(function() {
Assert.isTrue(accordion_wrapper.hasClass('lazr-closed'));
- }, 500);
+ }, 10);
},
test_importances_select_all_none: function() {
@@ -1518,7 +1524,7 @@
this.wait(function() {
Assert.isTrue(called_method);
- }, 20);
+ }, 10);
},
// Success handler for adding a subscription creates
=== modified file 'lib/lp/soyuz/javascript/tests/lp_dynamic_dom_updater.js'
--- lib/lp/soyuz/javascript/tests/lp_dynamic_dom_updater.js 2011-06-07 16:42:11 +0000
+++ lib/lp/soyuz/javascript/tests/lp_dynamic_dom_updater.js 2011-06-24 19:08:32 +0000
@@ -148,7 +148,7 @@
// Verify that both requests were issued.
this.wait(function() {
Y.Mock.verify(this.lp_client_mock);
- }, 100);
+ }, 5);
}, 5);
},
=== modified file 'versions.cfg'
--- versions.cfg 2011-06-23 10:32:38 +0000
+++ versions.cfg 2011-06-24 19:08:32 +0000
@@ -38,7 +38,7 @@
lazr.smtptest = 1.1
lazr.testing = 0.1.1
lazr.uri = 1.0.2
-lazr-js = 1.6DEV-r213
+lazr-js = 1.7DEV
manuel = 1.1.1
martian = 0.11
mechanize = 0.1.11
