launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04293
lp:~allenap/launchpad/localpackagediffs-filter-by-package-set-bug-809786-refactor into lp:launchpad
Gavin Panella has proposed merging lp:~allenap/launchpad/localpackagediffs-filter-by-package-set-bug-809786-refactor into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~allenap/launchpad/localpackagediffs-filter-by-package-set-bug-809786-refactor/+merge/68233
This moves a few DistroSeries related bits of JavaScript around and
fixes some lint. The only thing that really needs reviewing is the new
code in lib/lp/app/javascript/testing/testrunner.js. Its doc comment:
/**
* Merely loading this script into a page will cause it to look for a
* list of suites in the document using the selector ul#suites>li. If
* found, the text within each node is considered to be a test module
* name. This is then loaded, and its "suite" property passed to
* Runner.run().
*
* Here's how to declare the suites to run:
*
* <ul id="suites">
* <li>lp.registry.distroseries.initseries.test</li>
* </ul>
*
*/
The test is then declared as a regular YUI module instead of
containing test run code.
--
https://code.launchpad.net/~allenap/launchpad/localpackagediffs-filter-by-package-set-bug-809786-refactor/+merge/68233
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/localpackagediffs-filter-by-package-set-bug-809786-refactor into lp:launchpad.
=== modified file 'Makefile'
--- Makefile 2011-07-15 15:55:34 +0000
+++ Makefile 2011-07-18 11:31:28 +0000
@@ -25,11 +25,15 @@
JS_BUILD := min
endif
-JS_SOURCE_PATHS = -path './lib/lp/*/javascript/*' ! -path '*/tests/*' \
- ! -path '*/testing/*' ! -path './lib/lp/services/*'
+define JS_LP_PATHS
+lib -path 'lib/lp/*/javascript/*' \
+! -path '*/tests/*' ! -path '*/testing/*' \
+! -path 'lib/lp/services/*'
+endef
+
JS_YUI := $(shell utilities/yui-deps.py $(JS_BUILD:raw=))
JS_OTHER := $(wildcard lib/canonical/launchpad/javascript/*/*.js)
-JS_LP := $(shell find $(JS_SOURCE_PATHS) -name '*.js' ! -name '.*.js' )
+JS_LP := $(shell find $(JS_LP_PATHS) -name '*.js')
JS_ALL := $(JS_YUI) $(JS_OTHER) $(JS_LP)
JS_OUT := $(LP_BUILT_JS_ROOT)/launchpad.js
@@ -164,7 +168,7 @@
jsbuild_widget_css: bin/jsbuild
${SHHH} bin/jsbuild \
- --srcdir lib/lp/app/javascript \
+ --srcdir lib/lp/app/javascript \
--builddir $(LP_BUILT_JS_ROOT)
$(JS_LP): jsbuild_widget_css
@@ -425,7 +429,9 @@
copy-apache-config:
# We insert the absolute path to the branch-rewrite script
# into the Apache config as we copy the file into position.
- sed -e 's,%BRANCH_REWRITE%,$(shell pwd)/scripts/branch-rewrite.py,' configs/development/local-launchpad-apache > /etc/apache2/sites-available/local-launchpad
+ sed -e 's,%BRANCH_REWRITE%,$(shell pwd)/scripts/branch-rewrite.py,' \
+ configs/development/local-launchpad-apache > \
+ /etc/apache2/sites-available/local-launchpad
cp configs/development/local-vostok-apache \
/etc/apache2/sites-available/local-vostok
touch /var/tmp/bazaar.launchpad.dev/rewrite.log
=== modified file 'lib/lp/app/javascript/testing/testrunner.js'
--- lib/lp/app/javascript/testing/testrunner.js 2011-07-08 06:06:15 +0000
+++ lib/lp/app/javascript/testing/testrunner.js 2011-07-18 11:31:28 +0000
@@ -39,3 +39,32 @@
};
}, "0.1", {"requires": ["oop", "test", "console"]});
+
+
+/**
+ * Merely loading this script into a page will cause it to look for a
+ * list of suites in the document using the selector ul#suites>li. If
+ * found, the text within each node is considered to be a test module
+ * name. This is then loaded, and its "suite" property passed to
+ * Runner.run().
+ *
+ * Here's how to declare the suites to run:
+ *
+ * <ul id="suites">
+ * <li>lp.registry.distroseries.initseries.test</li>
+ * </ul>
+ *
+ */
+YUI().use("event", function(Y) {
+ Y.on("domready", function() {
+ var suites = Y.all("ul#suites > li");
+ Y.each(suites, function(suite_node) {
+ var suite_name = suite_node.get("text");
+ Y.use("lp.testing.runner", suite_name, function(y) {
+ // eval is not evil here; jslint can stfu.
+ var suite = eval("y." + suite_name + ".suite");
+ y.lp.testing.Runner.run(suite);
+ });
+ });
+ });
+});
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2011-07-08 09:06:24 +0000
+++ lib/lp/registry/browser/distroseries.py 2011-07-18 11:31:28 +0000
@@ -827,6 +827,8 @@
super(DistroSeriesDifferenceBaseView, self).initialize()
def initialize_sync_label(self, label):
+ # XXX: GavinPanella 2011-07-13 bug=809985: Good thing the app servers
+ # are running single threaded...
self.__class__.actions.byname['actions.sync'].label = label
@property
@@ -861,6 +863,8 @@
SimpleTerm(diff, diff.id)
for diff in self.cached_differences.batch]
diffs_vocabulary = SimpleVocabulary(terms)
+ # XXX: GavinPanella 2011-07-13 bug=809985: Good thing the app servers
+ # are running single threaded...
choice = self.form_fields['selected_differences'].field.value_type
choice.vocabulary = diffs_vocabulary
=== renamed file 'lib/lp/registry/javascript/distroseries.js' => 'lib/lp/registry/javascript/distroseries.initseries.js'
--- lib/lp/registry/javascript/distroseries.js 2011-06-27 17:04:27 +0000
+++ lib/lp/registry/javascript/distroseries.initseries.js 2011-07-18 11:31:28 +0000
@@ -22,7 +22,9 @@
*
* @class FormRowWidget
*/
-var FormRowWidget = function() {
+var FormRowWidget;
+
+FormRowWidget = function() {
FormRowWidget.superclass.constructor.apply(this, arguments);
};
@@ -166,7 +168,9 @@
* can also be made an overlay, and a component and a pocket selected.
*
*/
-var ParentSeriesListWidget = function() {
+var ParentSeriesListWidget;
+
+ParentSeriesListWidget = function() {
ParentSeriesListWidget
.superclass.constructor.apply(this, arguments);
};
@@ -466,7 +470,9 @@
*
* @class ChoiceListWidget
*/
-var ChoiceListWidget = function() {
+var ChoiceListWidget;
+
+ChoiceListWidget = function() {
ChoiceListWidget.superclass.constructor.apply(this, arguments);
};
@@ -660,7 +666,9 @@
*
* @class ArchitecturesChoiceListWidget
*/
-var ArchitecturesChoiceListWidget = function() {
+var ArchitecturesChoiceListWidget;
+
+ArchitecturesChoiceListWidget = function() {
ArchitecturesChoiceListWidget
.superclass.constructor.apply(this, arguments);
};
@@ -726,14 +734,15 @@
*/
remove_distroseries: function(distroseries_id) {
// Compute which das is only in the distroseries to be removed.
- arch_to_remove = [];
+ var arch_to_remove = [];
var das = this._distroseries[distroseries_id];
var i, ds, j;
for (i=0; i<das.entries.length; i++) {
- remove_das = true;
- arch = das.entries[i].get('architecture_tag');
+ var remove_das = true;
+ var arch = das.entries[i].get('architecture_tag');
for (ds in this._distroseries) {
- if (ds !== distroseries_id) {
+ if (this._distroseries.hasOwnProperty(ds) &&
+ ds !== distroseries_id) {
var other_das = this._distroseries[ds];
for (j=0; j<other_das.entries.length; j++) {
var other_arch = other_das.entries[j].get(
@@ -782,7 +791,9 @@
*
* @class SelectWidget
*/
-var SelectWidget = function() {
+var SelectWidget;
+
+SelectWidget = function() {
SelectWidget.superclass.constructor.apply(this, arguments);
};
@@ -949,7 +960,9 @@
*
* @class PackagesetPickerWidget
*/
-var PackagesetPickerWidget = function() {
+var PackagesetPickerWidget;
+
+PackagesetPickerWidget = function() {
PackagesetPickerWidget
.superclass.constructor.apply(this, arguments);
};
@@ -1144,7 +1157,9 @@
*
* @class FormActionsWidget
*/
-var FormActionsWidget = function() {
+var FormActionsWidget;
+
+FormActionsWidget = function() {
FormActionsWidget
.superclass.constructor.apply(this, arguments);
};
@@ -1234,7 +1249,9 @@
*
* @class DeriveDistroSeriesActionsWidget
*/
-var DeriveDistroSeriesActionsWidget = function() {
+var DeriveDistroSeriesActionsWidget;
+
+DeriveDistroSeriesActionsWidget = function() {
DeriveDistroSeriesActionsWidget
.superclass.constructor.apply(this, arguments);
};
@@ -1333,7 +1350,8 @@
add_parent_series(result);
};
- parent_picker = Y.lp.app.picker.create('DistroSeriesDerivation', config);
+ var parent_picker =
+ Y.lp.app.picker.create('DistroSeriesDerivation', config);
parent_picker.show();
};
@@ -1354,17 +1372,21 @@
*
* @function setup
*/
-namespace.setup = function() {
- var form_actions = namespace.setupWidgets();
- namespace.setupInteraction(form_actions);
+namespace.setup = function(cache) {
+ var form_actions = namespace.setupWidgets(cache);
+ namespace.setupInteraction(form_actions, cache);
};
/**
* Setup the widgets objects and return the form object.
*
* @function setupWidgets
+ * @param {Object} cache Specify the value cache to use. If not
+ * specified, LP.cache is used. Intended chiefly for testing.
*/
-namespace.setupWidgets = function() {
+namespace.setupWidgets = function(cache) {
+ if (cache === undefined) { cache = LP.cache; }
+
var form_container = Y.one("#initseries-form-container");
var form_table = form_container.one("table.form");
var form_table_body = form_table.append(Y.Node.create('<tbody />'));
@@ -1394,7 +1416,7 @@
"architectures)."))
.render(form_table_body);
var packageset_choice = null;
- if (LP.cache['is_first_derivation']) {
+ if (cache.is_first_derivation) {
packageset_choice =
new PackagesetPickerWidget()
.set("name", "field.packagesets")
@@ -1424,7 +1446,7 @@
.render(form_table_body);
var form_actions =
new DeriveDistroSeriesActionsWidget({
- context: LP.cache.context,
+ context: cache.context,
srcNode: form_container.one("div.actions"),
deriveFromChoices: parents_selection,
architectureChoice: architecture_choice,
@@ -1440,10 +1462,14 @@
* Setup the interaction between the widgets.
*
* @function setupInteraction
- * @param {DeriveDistroSeriesActionsWidget} The form widget containing all
- * the other widgets.
+ * @param {DeriveDistroSeriesActionsWidget} The form widget containing
+ * all the other widgets.
+ * @param {Object} cache Specify the value cache to use. If not
+ * specified, LP.cache is used. Intended chiefly for testing.
*/
-namespace.setupInteraction = function(form_actions) {
+namespace.setupInteraction = function(form_actions, cache) {
+ if (cache === undefined) { cache = LP.cache; }
+
// Wire up the add parent series link.
var link = Y.one('#add-parent-series');
if (Y.Lang.isValue(link)) {
@@ -1458,7 +1484,7 @@
}
});
- if (LP.cache['is_first_derivation']) {
+ if (cache.is_first_derivation) {
// Wire the architecture and packageset pickers to the parent picker.
Y.on('parent_added', function(parent) {
form_actions.architectureChoice.add_distroseries(parent);
@@ -1471,7 +1497,7 @@
});
}
else {
- // Disable rebuilding if LP.cache['is_first_derivation'] is false.
+ // Disable rebuilding if cache.is_first_derivation is false.
form_actions.packageCopyOptions.fieldNode
.one('input[value="Copy Source and Rebuild"]')
.set('disabled', 'disabled');
@@ -1485,13 +1511,12 @@
"if you want to use all the available " +
"architectures)."));
form_actions.architectureChoice.add_distroseries(
- LP.cache['previous_series']);
+ cache.previous_series);
// Setup the pre-selected parents (parents from the previous series).
- var index;
- for (index in LP.cache['previous_parents']) {
- form_actions.deriveFromChoices.add_parent(
- LP.cache['previous_parents'][index]);
- }
+ Y.each(
+ cache.previous_parents,
+ form_actions.deriveFromChoices.add_parent,
+ form_actions.deriveFromChoices);
}
// Wire up the form submission.
=== renamed file 'lib/lp/registry/javascript/tests/test_distroseries.html' => 'lib/lp/registry/javascript/tests/test_distroseries.initseries.html'
--- lib/lp/registry/javascript/tests/test_distroseries.html 2011-07-08 06:06:15 +0000
+++ lib/lp/registry/javascript/tests/test_distroseries.initseries.html 2011-07-18 11:31:28 +0000
@@ -6,27 +6,40 @@
<title>Launchpad DistroSeries</title>
<!-- YUI and test setup -->
- <script type="text/javascript"
- src="../../../../canonical/launchpad/icing/yui/yui/yui.js">
- </script>
<link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
<script type="text/javascript"
+ src="../../../../canonical/launchpad/icing/yui/yui/yui.js"></script>
+ <script type="text/javascript"
src="../../../app/javascript/testing/testrunner.js"></script>
- <!-- Required modules -->
- <script type="text/javascript" src="../../../app/javascript/client.js"></script>
- <script type="text/javascript" src="../../../app/javascript/activator/activator.js"></script>
- <script type="text/javascript" src="../../../app/javascript/anim/anim.js"></script>
- <script type="text/javascript" src="../../../app/javascript/lazr/lazr.js"></script>
- <script type="text/javascript" src="../../../app/javascript/overlay/overlay.js"></script>
- <script type="text/javascript" src="../../../app/javascript/picker/picker.js"></script>
- <script type="text/javascript" src="../../../app/javascript/picker/person_picker.js"></script>
- <script type="text/javascript" src="../../../app/javascript/picker/picker_patcher.js"></script>
- <!-- The module under test -->
- <script type="text/javascript" src="../distroseries.js"></script>
- <!-- The test suite -->
- <script type="text/javascript" src="test_distroseries.js"></script>
+ <!-- Required modules -->
+ <script type="text/javascript"
+ src="../../../app/javascript/client.js"></script>
+ <script type="text/javascript"
+ src="../../../app/javascript/activator/activator.js"></script>
+ <script type="text/javascript"
+ src="../../../app/javascript/anim/anim.js"></script>
+ <script type="text/javascript"
+ src="../../../app/javascript/lazr/lazr.js"></script>
+ <script type="text/javascript"
+ src="../../../app/javascript/overlay/overlay.js"></script>
+ <script type="text/javascript"
+ src="../../../app/javascript/picker/picker.js"></script>
+ <script type="text/javascript"
+ src="../../../app/javascript/picker/person_picker.js"></script>
+ <script type="text/javascript"
+ src="../../../app/javascript/picker/picker_patcher.js"></script>
+
+ <!-- The module under test -->
+ <script type="text/javascript" src="../distroseries.initseries.js"></script>
+
+ <!-- The test suite -->
+ <script type="text/javascript" src="test_distroseries.initseries.js"></script>
+
</head>
<body class="yui3-skin-sam">
+ <ul id="suites">
+ <li>lp.registry.distroseries.initseries.test</li>
+ </ul>
</body>
</html>
=== renamed file 'lib/lp/registry/javascript/tests/test_distroseries.js' => 'lib/lp/registry/javascript/tests/test_distroseries.initseries.js'
--- lib/lp/registry/javascript/tests/test_distroseries.js 2011-07-08 05:39:39 +0000
+++ lib/lp/registry/javascript/tests/test_distroseries.initseries.js 2011-07-18 11:31:28 +0000
@@ -1,9 +1,16 @@
-/* Copyright (c) 2011, Canonical Ltd. All rights reserved. */
-
-YUI().use(
- 'lp.testing.runner', 'test', 'console', 'node-event-simulate',
- 'lp.registry.distroseries.initseries',
- function(Y) {
+/**
+ * Copyright 2011 Canonical Ltd. This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * Tests for DistroSeries related stuff.
+ *
+ * @module lp.registry.distroseries
+ * @submodule test
+ */
+
+YUI.add('lp.registry.distroseries.initseries.test', function(Y) {
+
+ var namespace = Y.namespace('lp.registry.distroseries.initseries.test');
var Assert = Y.Assert;
var ArrayAssert = Y.ArrayAssert;
@@ -440,6 +447,7 @@
},
testParentGetter: function() {
+ var parent;
parent = {value: "4", title: "Hoary", api_uri: "ubuntu/hoary"};
this.widget.add_parent(parent);
parent = {value: "3", title: "Warty", api_uri: "ubuntu/warty"};
@@ -495,7 +503,7 @@
Assert.isFunction(config.on.failure);
}
};
- distroseries = {api_uri: "ubuntu/hoary", value: 3};
+ var distroseries = {api_uri: "ubuntu/hoary", value: 3};
this.widget.add_distroseries(distroseries);
Assert.isTrue(io, "No IO initiated.");
},
@@ -1056,6 +1064,10 @@
else if (name === "overlay_components") {
return ['restricted', null];
}
+ else {
+ Assert.fail("Unrecognized property: " + name);
+ return null; // Keep lint quiet.
+ }
}
},
architectureChoice: {
@@ -1169,30 +1181,10 @@
Y.one('body').appendChild(node);
},
- setUpFirstDerivation: function() {
- window.LP = {cache: {is_first_derivation: true}};
- },
-
- setUpNotFirstDerivation: function() {
- window.LP = {cache:
- {is_first_derivation: false,
- previous_series: {
- api_uri: '/ubuntu/natty',
- value: '3',
- title: 'Ubunty: Natty'},
- previous_parents: [
- {api_uri: '/debian/sid',
- value: '4',
- title: 'Debian: Sid'},
- {api_uri: '/zz/aa',
- value: '5',
- title: 'ZZ: aa'}]}};
- },
-
testIsFirstDerivation: function() {
- this.setUpFirstDerivation();
- var form_actions = initseries.setupWidgets();
- initseries.setupInteraction(form_actions);
+ var cache = {is_first_derivation: true};
+ var form_actions = initseries.setupWidgets(cache);
+ initseries.setupInteraction(form_actions, cache);
// No pre-populated parent.
ArrayAssert.itemsAreEqual(
@@ -1201,8 +1193,21 @@
},
testIsNotFirstDerivation: function() {
- this.setUpNotFirstDerivation();
- var form_actions = initseries.setupWidgets();
+ var cache = {
+ is_first_derivation: false,
+ previous_series: {
+ api_uri: '/ubuntu/natty',
+ value: '3',
+ title: 'Ubunty: Natty'
+ },
+ previous_parents: [
+ {api_uri: '/debian/sid',
+ value: '4', title: 'Debian: Sid'},
+ {api_uri: '/zz/aa',
+ value: '5', title: 'ZZ: aa'}
+ ]
+ };
+ var form_actions = initseries.setupWidgets(cache);
// Monkeypatch LP client.
var client = {
get: function(path, config) {
@@ -1220,7 +1225,7 @@
}
};
form_actions.architectureChoice.client = client;
- initseries.setupInteraction(form_actions);
+ initseries.setupInteraction(form_actions, cache);
// Parents are populated.
ArrayAssert.itemsAreEqual(
@@ -1237,7 +1242,8 @@
};
suite.add(new Y.Test.Case(testDeriveDistroSeriesSetup));
-
- Y.lp.testing.Runner.run(suite);
-
-});
+ namespace.suite = suite;
+
+}, "0.1", {"requires": [
+ 'test', 'console', 'node-event-simulate',
+ 'lp.registry.distroseries.initseries']});
