← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cjwatson/launchpad/eslint-abolish-test-should into lp:launchpad

 

Colin Watson has proposed merging lp:~cjwatson/launchpad/eslint-abolish-test-should into lp:launchpad with lp:~cjwatson/launchpad/eslint-declare-vars as a prerequisite.

Commit message:
Abolish use of _should in JS tests, replacing it with (mostly) Y.Assert.throwsError.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/eslint-abolish-test-should/+merge/327932

The _should property of YUI test cases is an abomination: it places an essential property of the individual test (whether it's expected to throw an exception or not) out of sight at the top of the Y.Test.Case object rather than in the test itself.  Fortunately, nowadays YUI has a Y.Assert.throwsError method which is much more sensible, and we can use that in place of _should.error.  In the process, this lets us squash some eslint complaints about using the new operator solely for its side-effects.

_should.ignore can be replaced by prefixing the test's name with 'ignore:', which is a bit odd but similarly achieves the goal of localising information about the test.

I left one use of _should intact, namely the one in lib/lp/bugs/javascript/tests/test_buglisting_utils.js which conditionally ignores a couple of tests.  Conditionally renaming the tests was possible but seemed more verbose without much gain, and the only other approach I could think of was a conditional early return which is more like a pass than a skip, so I decided it was best to just leave it alone.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/eslint-abolish-test-should into lp:launchpad.
=== modified file 'lib/lp/app/javascript/choice.js'
--- lib/lp/app/javascript/choice.js	2015-02-19 03:23:11 +0000
+++ lib/lp/app/javascript/choice.js	2017-07-22 23:50:27 +0000
@@ -161,7 +161,7 @@
  */
 namespace.addPopupChoiceForRadioButtons = function(field_name, choices, cfg) {
     if (!choices || choices.length === 0) {
-        throw 'No choices for the popup.';
+        Y.error('No choices for the popup.');
     }
     cfg = Y.merge(default_popup_choice_config, cfg);
     var field_node = cfg.container.one('[name="field.' + field_name + '"]');

=== modified file 'lib/lp/app/javascript/information_type.js'
--- lib/lp/app/javascript/information_type.js	2012-11-13 20:08:22 +0000
+++ lib/lp/app/javascript/information_type.js	2017-07-22 23:50:27 +0000
@@ -78,7 +78,7 @@
  */
 Y.on(ns.EV_CHANGE, function (ev) {
     if (!ev.value) {
-        throw('Information type change event without new value');
+        Y.error('Information type change event without new value');
     }
 
     if (is_private_event(ev.value)) {

=== modified file 'lib/lp/app/javascript/ordering/tests/test_orderby_widget.js'
--- lib/lp/app/javascript/ordering/tests/test_orderby_widget.js	2017-07-22 23:50:27 +0000
+++ lib/lp/app/javascript/ordering/tests/test_orderby_widget.js	2017-07-22 23:50:27 +0000
@@ -12,15 +12,6 @@
         name: 'orderbybar_widget_tests',
         orderby: null,
 
-        _should: {
-            error: {
-                test_sort_order_validator:
-                    new Error('sort_order must be either "asc" or "desc"'),
-                test_active_sort_validator:
-                    new Error('active attribute was not found in sort_keys')
-            }
-        },
-
         tearDown: function() {
             if (Y.Lang.isValue(this.orderby)) {
                 this.orderby.destroy();
@@ -191,7 +182,9 @@
                 sort_keys: test_sort_keys,
                 active: 'foobarbazdonotexists'
             });
-            this.orderby.render();
+            Y.Assert.throwsError(
+                new Error('active attribute was not found in sort_keys'),
+                this.orderby.render.bind(this.orderby));
         },
 
         test_sort_order_validator: function() {
@@ -200,7 +193,9 @@
             this.orderby = new Y.lp.ordering.OrderByBar({
                 sort_order: 'foobar'
             });
-            this.orderby.render();
+            Y.Assert.throwsError(
+                new Error('sort_order must be either "asc" or "desc"'),
+                this.orderby.render.bind(this.orderby));
         },
 
         test_click_current_sort_arrow_changes: function() {

=== modified file 'lib/lp/app/javascript/subscribers/tests/test_subscribers_list.js'
--- lib/lp/app/javascript/subscribers/tests/test_subscribers_list.js	2017-07-22 23:50:27 +0000
+++ lib/lp/app/javascript/subscribers/tests/test_subscribers_list.js	2017-07-22 23:50:27 +0000
@@ -80,19 +80,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'SubscribersList constructor test',
 
-        _should: {
-            error: {
-                test_no_container_error:
-                new Error(
-                    'Container node must be specified in ' +
-                    'config.container_box.'),
-                test_multiple_containers_error:
-                new Error(
-                    "Multiple container nodes for selector '.container' "+
-                        "present in the page. You need to be more explicit.")
-            }
-        },
-
         setUp: function() {
             this.root = Y.Node.create('<div />');
             Y.one('body').appendChild(this.root);
@@ -110,9 +97,16 @@
         test_no_container_error: function() {
             // When there is no matching container node in the DOM tree,
             // an exception is thrown.
-            new module.SubscribersList({
-                container_box: '#not-found',
-                subscriber_levels: []});
+            Y.Assert.throwsError(
+                new Error(
+                    'Container node must be specified in ' +
+                    'config.container_box.'),
+                function() {
+                    return new module.SubscribersList({
+                        container_box: '#not-found',
+                        subscriber_levels: [],
+                    });
+                });
         },
 
         test_single_container: function() {
@@ -133,9 +127,16 @@
                 Y.Node.create('<div />').addClass('container'));
             this.root.appendChild(
                 Y.Node.create('<div />').addClass('container'));
-            new module.SubscribersList({
-                container_box: '.container',
-                subscriber_levels: []});
+            Y.Assert.throwsError(
+                new Error(
+                    "Multiple container nodes for selector '.container' " +
+                    "present in the page. You need to be more explicit."),
+                function() {
+                    return new module.SubscribersList({
+                        container_box: '.container',
+                        subscriber_levels: [],
+                    });
+                });
         },
 
         test_subscriber_levels: function() {
@@ -275,13 +276,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'SubscribersList.startActivity() and stopActivity() test',
 
-        _should: {
-            error: {
-                test_setActivityErrorIcon_error: true,
-                test_setActivityText_error: true
-            }
-        },
-
         setUp: function() {
             this.root = Y.Node.create('<div />');
             Y.one('body').appendChild(this.root);
@@ -354,7 +348,9 @@
             // With non-activity node passed in, it fails.
             var subscribers_list = setUpSubscribersList(this.root);
             var node = Y.Node.create('<div />');
-            subscribers_list._setActivityErrorIcon(node, true);
+            Y.Assert.throwsError(TypeError, function() {
+                subscribers_list._setActivityErrorIcon(node, true);
+            });
         },
 
         test_setActivityText: function() {
@@ -372,7 +368,9 @@
             // With non-activity node passed in, it fails.
             var subscribers_list = setUpSubscribersList(this.root);
             var node = Y.Node.create('<div />');
-            subscribers_list._setActivityText(node, "Blah");
+            Y.Assert.throwsError(TypeError, function() {
+                subscribers_list._setActivityText(node, "Blah");
+            });
         },
 
         test_startActivity: function() {
@@ -664,17 +662,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'SubscribersList._removeSectionNodeIfEmpty() test',
 
-        _should: {
-            error: {
-                test_sectionNodeHasSubscribers_error:
-                new Error(
-                    'No div.subscribers-list found inside the passed `node`.'),
-                test_removeSectionNodeIfEmpty_non_section_error:
-                new Error(
-                    'Node is not a section node.')
-            }
-        },
-
         setUp: function() {
             this.root = Y.Node.create('<div />');
             Y.one('body').appendChild(this.root);
@@ -689,7 +676,11 @@
             // it throws an error.
             var subscribers_list = setUpSubscribersList(this.root);
             var node = Y.Node.create('<div />');
-            subscribers_list._sectionNodeHasSubscribers(node);
+            Y.Assert.throwsError(
+                new Error(
+                    'No div.subscribers-list found inside the passed `node`.'),
+                subscribers_list._sectionNodeHasSubscribers.bind(
+                    subscribers_list, node));
         },
 
         test_sectionNodeHasSubscribers_no_subscribers: function() {
@@ -723,7 +714,10 @@
             // an exception.
             var subscribers_list = setUpSubscribersList(this.root);
             var section_node = Y.Node.create('<div />');
-            subscribers_list._removeSectionNodeIfEmpty(section_node);
+            Y.Assert.throwsError(
+                new Error('Node is not a section node.'),
+                subscribers_list._removeSectionNodeIfEmpty.bind(
+                    subscribers_list, section_node));
         },
 
         test_removeSectionNodeIfEmpty_remove: function() {
@@ -791,20 +785,6 @@
             this.root.remove();
         },
 
-        _should: {
-            error: {
-                test_validateSubscriber_no_name_error:
-                new Error('No `name` passed in `subscriber`.'),
-                test_addSubscriber_incorrect_level:
-                new Error(
-                    'Level "Test" is not an acceptable subscription level.'),
-                test_addSubscriber_not_in_section_error:
-                new Error(
-                    "Matching subscriber node doesn't seem to be in any " +
-                        "subscribers list sections.")
-            }
-        },
-
         test_getNodeIdForSubscriberName: function() {
             // Returns a CSS class name to use as the ID for subscribers
             // prefixed with 'subscriber-'.  Uses launchpad_to_css for
@@ -831,7 +811,10 @@
             // When no name attribute is present, an exception is thrown.
             var subscribers_list = setUpSubscribersList(this.root);
             var subscriber = { };
-            subscribers_list._validateSubscriber(subscriber);
+            Y.Assert.throwsError(
+                new Error('No `name` passed in `subscriber`.'),
+                subscribers_list._validateSubscriber.bind(
+                    subscribers_list, subscriber));
         },
 
         test_validateSubscriber_no_overriding: function() {
@@ -979,8 +962,11 @@
         test_addSubscriber_incorrect_level: function() {
             // When an incorrect level is passed in, an exception is thrown.
             var subscribers_list = setUpSubscribersList(this.root);
-            subscribers_list.addSubscriber(
-                { name: 'user' }, 'Test');
+            Y.Assert.throwsError(
+                new Error(
+                    'Level "Test" is not an acceptable subscription level.'),
+                subscribers_list.addSubscriber.bind(
+                    subscribers_list, { name: 'user' }, 'Test'));
         },
 
         test_addSubscriber_change_level: function() {
@@ -1010,8 +996,12 @@
             subscribers_list.container_node.appendChild(node);
 
             // And addSubscriber now throws an exception.
-            subscribers_list.addSubscriber(
-                { name: 'user' }, 'Level3');
+            Y.Assert.throwsError(
+                new Error(
+                    "Matching subscriber node doesn't seem to be in any " +
+                    "subscribers list sections."),
+                subscribers_list.addSubscriber.bind(
+                    subscribers_list, { name: 'user' }, 'Level3'));
         },
 
         test_addSubscriber_ordering: function() {
@@ -1078,25 +1068,6 @@
             this.root.remove();
         },
 
-        _should: {
-            error: {
-                test_getSubscriberNode_error:
-                new Error('Subscriber is not present in the subscribers ' +
-                    'list. Please call addSubscriber(subscriber) first.'),
-                test_addUnsubscribeAction_error:
-                new Error('Passed in callback for unsubscribe action ' +
-                          'is not a function.'),
-                test_removeSubscriber_error:
-                new Error(
-                    'Subscriber is not present in the subscribers list. ' +
-                        'Please call addSubscriber(subscriber) first.'),
-                test_removeSubscriber_not_in_section_error:
-                new Error(
-                    "Matching subscriber node doesn't seem to be in any " +
-                        "subscribers list sections.")
-            }
-        },
-
         test_getSubscriberNode: function() {
             // Gets a subscriber node from the subscribers list when present.
             var subscribers_list = setUpSubscribersList(this.root);
@@ -1110,7 +1081,12 @@
             // When subscriber node is not present, throws an error.
             var subscribers_list = setUpSubscribersList(this.root);
             var subscriber = { name: 'user' };
-            subscribers_list._getSubscriberNode(subscriber);
+            Y.Assert.throwsError(
+                new Error(
+                    'Subscriber is not present in the subscribers ' +
+                    'list. Please call addSubscriber(subscriber) first.'),
+                subscribers_list._getSubscriberNode.bind(
+                    subscribers_list, subscriber));
         },
 
         test_getOrCreateActionsNode: function() {
@@ -1188,7 +1164,11 @@
             var subscribers_list = setUpSubscribersList(this.root);
             var subscriber = { name: 'user' };
             subscribers_list.addSubscriber(subscriber, 'Level1');
-            subscribers_list.addUnsubscribeAction(subscriber, "not-function");
+            Y.Assert.throwsError(
+                new Error('Passed in callback for unsubscribe action ' +
+                          'is not a function.'),
+                subscribers_list.addUnsubscribeAction.bind(
+                    subscribers_list, subscriber, "not-function"));
         },
 
         test_addUnsubscribeAction_callback_on_click: function() {
@@ -1216,7 +1196,12 @@
             // Removing a non-existent subscriber fails with an error.
             var subscribers_list = setUpSubscribersList(this.root);
             var subscriber = { name: 'user' };
-            subscribers_list.removeSubscriber(subscriber);
+            Y.Assert.throwsError(
+                new Error(
+                    'Subscriber is not present in the subscribers list. ' +
+                        'Please call addSubscriber(subscriber) first.'),
+                subscribers_list.removeSubscriber.bind(
+                    subscribers_list, subscriber));
         },
 
         test_removeSubscriber_section_removed: function() {
@@ -1254,7 +1239,12 @@
                 .set('id', 'subscriber-user');
             // We hack the node directly into the entire subscribers list node.
             subscribers_list.container_node.appendChild(node);
-            subscribers_list.removeSubscriber({ name: 'user' });
+            Y.Assert.throwsError(
+                new Error(
+                    "Matching subscriber node doesn't seem to be in any " +
+                    "subscribers list sections."),
+                subscribers_list.removeSubscriber.bind(
+                    subscribers_list, { name: 'user' }));
         }
     }));
 
@@ -1278,22 +1268,16 @@
             Y.lp.anim.flash_in.defaults.duration = this.anim_duration;
         },
 
-        _should: {
-            error: {
-                test_indicateSubscriberActivity_error:
-                new Error('Subscriber is not present in the subscribers ' +
-                    'list. Please call addSubscriber(subscriber) first.'),
-                test_stopSubscriberActivity_error:
-                new Error('Subscriber is not present in the subscribers ' +
-                    'list. Please call addSubscriber(subscriber) first.')
-            }
-        },
-
         test_indicateSubscriberActivity_error: function() {
             // When subscriber is not in the list, fails with an exception.
             var subscribers_list = setUpSubscribersList(this.root);
             var subscriber = { name: 'user' };
-            subscribers_list.indicateSubscriberActivity(subscriber);
+            Y.Assert.throwsError(
+                new Error(
+                    'Subscriber is not present in the subscribers ' +
+                    'list. Please call addSubscriber(subscriber) first.'),
+                subscribers_list.indicateSubscriberActivity.bind(
+                    subscribers_list, subscriber));
         },
 
         test_indicateSubscriberActivity_node: function() {
@@ -1329,7 +1313,12 @@
             // When subscriber is not in the list, fails with an exception.
             var subscribers_list = setUpSubscribersList(this.root);
             var subscriber = { name: 'user' };
-            subscribers_list.stopSubscriberActivity(subscriber);
+            Y.Assert.throwsError(
+                new Error(
+                    'Subscriber is not present in the subscribers ' +
+                    'list. Please call addSubscriber(subscriber) first.'),
+                subscribers_list.stopSubscriberActivity.bind(
+                    subscribers_list, subscriber));
         },
 
         test_stopSubscriberActivity_noop: function() {
@@ -1417,26 +1406,6 @@
     tests.suite.add(new Y.Test.Case({
        name: 'SubscribersLoader() construction test',
 
-       _should: {
-           error: {
-               test_SubscribersLoader_container_error:
-               new Error(
-                   'Container node must be specified in config.container_box.'),
-               test_SubscribersLoader_context_error:
-               new Error(
-                   "No context specified in `config' or context.web_link " +
-                       "is invalid."),
-               test_SubscribersLoader_context_web_link_error:
-               new Error(
-                   "No context specified in `config' or context.web_link " +
-                       "is invalid."),
-               test_SubscribersLoader_portlet_link_error:
-               new Error(
-                   "No config.subscribers_details_view specified to load " +
-                       "other subscribers from.")
-           }
-       },
-
        setUp: function() {
            this.root = Y.Node.create('<div />');
            Y.one('body').appendChild(this.root);
@@ -1449,29 +1418,51 @@
        test_SubscribersLoader_container_error: function() {
            // If no container node to hold the subscribers list is specified,
            // it fails with an error.
-           new module.SubscribersLoader({
-               container_box: '#not-found',
-               subscriber_levels: []});
+           Y.Assert.throwsError(
+               new Error(
+                   'Container node must be specified in ' +
+                   'config.container_box.'),
+               function() {
+                   return new module.SubscribersLoader({
+                       container_box: '#not-found',
+                       subscriber_levels: [],
+                   });
+               });
        },
 
        test_SubscribersLoader_context_error: function() {
            // Context needs to be passed in as well.
            // setUpLoader constructs the container node for us.
+           var root = this.root;
            var config = {};
-           setUpLoader(this.root, config, true);
+           Y.Assert.throwsError(
+               new Error(
+                   "No context specified in `config' or context.web_link " +
+                   "is invalid."),
+               function() { setUpLoader(root, config, true); });
        },
 
        test_SubscribersLoader_context_web_link_error: function() {
            // Fails if the passed in context has no web_link attribute defined.
+           var root = this.root;
            var config = { context: {} };
-           setUpLoader(this.root, config, true);
+           Y.Assert.throwsError(
+               new Error(
+                   "No context specified in `config' or context.web_link " +
+                   "is invalid."),
+               function() { setUpLoader(root, config, true); });
        },
 
        test_SubscribersLoader_portlet_link_error: function() {
            // Fails if the passed in config has no passed in
            // portlet URI for loading context subscribers details.
+           var root = this.root;
            var config = { context: { web_link: '' } };
-           setUpLoader(this.root, config, true);
+           Y.Assert.throwsError(
+               new Error(
+                   "No config.subscribers_details_view specified to load " +
+                   "other subscribers from."),
+               function() { setUpLoader(root, config, true); });
        },
 
        test_SubscribersLoader_default_config_parameters: function() {
@@ -1592,17 +1583,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'SubscribersLoader() subscribers loading test',
 
-        _should: {
-            error: {
-                test_loadSubscribersFromList_not_list_error:
-                new Error('Got non-array "Not-a-list" in ' +
-                          '_loadSubscribersFromList().'),
-                test_loadSubscribersFromList_no_objects_error:
-                new Error('Subscriber details at index 0 (Subscriber)' +
-                          ' are not an object.')
-            }
-        },
-
         setUp: function() {
             this.root = Y.Node.create('<div />');
             Y.one('body').appendChild(this.root);
@@ -1732,7 +1712,10 @@
             var data = "Not-a-list";
 
             var loader = setUpLoader(this.root);
-            loader._loadSubscribersFromList(data);
+            Y.Assert.throwsError(
+                new Error('Got non-array "Not-a-list" in ' +
+                          '_loadSubscribersFromList().'),
+                function() { loader._loadSubscribersFromList(data); });
         },
 
         test_loadSubscribersFromList_no_objects_error: function() {
@@ -1740,7 +1723,10 @@
             var data = ["Subscriber"];
 
             var loader = setUpLoader(this.root);
-            loader._loadSubscribersFromList(data);
+            Y.Assert.throwsError(
+                new Error('Subscriber details at index 0 (Subscriber)' +
+                          ' are not an object.'),
+                function() { loader._loadSubscribersFromList(data); });
         },
 
         test_loadSubscribers_success: function() {
@@ -1954,15 +1940,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'SubscribersLoader() subscribe-someone-else test',
 
-        _should: {
-            error: {
-                test_setupSubscribeSomeoneElse_error:
-                new Error("No link matching CSS selector " +
-                          "'#sub-someone-else-link' " +
-                          "for subscribing someone else found.")
-            }
-        },
-
         setUp: function() {
             this.root = Y.Node.create('<div />');
             Y.one('body').appendChild(this.root);
@@ -2008,7 +1985,11 @@
             // Initialize the loader with no subscribe-someone-else link.
             var loader = setUpLoader(this.root);
             loader.subscribe_someone_else_link = '#sub-someone-else-link';
-            loader._setupSubscribeSomeoneElse();
+            Y.Assert.throwsError(
+                new Error("No link matching CSS selector " +
+                          "'#sub-someone-else-link' " +
+                          "for subscribing someone else found."),
+                loader._setupSubscribeSomeoneElse.bind(loader));
         },
 
         test_setupSubscribeSomeoneElse_not_logged_in: function() {
@@ -2186,15 +2167,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'SubscribersLoader() subscribe-me test',
 
-        _should: {
-            error: {
-                test_setupSubscribeMe_error:
-                new Error("No link matching CSS selector " +
-                          "'#sub-not-found-me-link' " +
-                          "for subscribing me found.")
-            }
-        },
-
         setUp: function() {
             this.root = Y.Node.create('<div />');
             Y.one('body').appendChild(this.root);
@@ -2259,7 +2231,11 @@
             // Initialize the loader with no subscribe-me link.
             var loader = setUpLoader(this.root);
             loader.subscribe_me_link = '#sub-not-found-me-link';
-            loader._setupSubscribeMe();
+            Y.Assert.throwsError(
+                new Error("No link matching CSS selector " +
+                          "'#sub-not-found-me-link' " +
+                          "for subscribing me found."),
+                loader._setupSubscribeMe.bind(loader));
         },
 
         test_setupSubscribeMe_not_logged_in: function() {

=== modified file 'lib/lp/app/javascript/tests/test_choice.js'
--- lib/lp/app/javascript/tests/test_choice.js	2013-03-20 03:41:40 +0000
+++ lib/lp/app/javascript/tests/test_choice.js	2017-07-22 23:50:27 +0000
@@ -22,13 +22,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'radio_popup_choice',
 
-        _should: {
-            error: {
-                test_error_undefined_data: true,
-                test_error_empty_array: true
-            }
-        },
-
         setUp: function () {
             window.LP = {
                 cache: {
@@ -68,15 +61,19 @@
         test_error_undefined_data: function () {
             // An exception should be raised if you create a button with
             // undefined options.
-            choice.addPopupChoiceForRadioButtons('information_type',
-                                                 undefined);
+            Y.Assert.throwsError(
+                'No choices for the popup.',
+                choice.addPopupChoiceForRadioButtons.bind(
+                    choice, 'information_type', undefined));
         },
 
         test_error_empty_array: function () {
             // An exception should be raised if you create a button with no
             // options.
-            choice.addPopupChoiceForRadioButtons('information_type',
-                                                 []);
+            Y.Assert.throwsError(
+                'No choices for the popup.',
+                choice.addPopupChoiceForRadioButtons.bind(
+                    choice, 'information_type', []));
         }
     }));
 

=== modified file 'lib/lp/app/javascript/tests/test_expander.js'
--- lib/lp/app/javascript/tests/test_expander.js	2017-07-22 23:50:27 +0000
+++ lib/lp/app/javascript/tests/test_expander.js	2017-07-22 23:50:27 +0000
@@ -92,12 +92,6 @@
 
         name: 'expandable',
 
-        _should: {
-            ignore: {
-                test_receive_restarts_at_current_height: true
-            }
-        },
-
         test_separate_animate_node: function() {
             var icon = Y.Node.create('<td></td>'),
                 content = Y.Node.create('<td></td>'),
@@ -295,7 +289,7 @@
                 ['stop', 'run'], anim.call_stack);
         },
 
-        test_receive_restarts_at_current_height: function() {
+        'ignore:test_receive_restarts_at_current_height': function() {
             var expander = this.makeExpander();
 
             var anim = new FakeAnim();

=== modified file 'lib/lp/app/javascript/tests/test_information_type.js'
--- lib/lp/app/javascript/tests/test_information_type.js	2017-07-22 23:50:27 +0000
+++ lib/lp/app/javascript/tests/test_information_type.js	2017-07-22 23:50:27 +0000
@@ -10,12 +10,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'lp.app.information_type_tests',
 
-        _should: {
-            error: {
-                test_change_event_empty: true
-            }
-        },
-
         setUp: function() {
             window.LP = {
                 cache: {
@@ -285,7 +279,9 @@
 
         test_change_event_empty: function () {
             // When firing a change event you must supply a value.
-            Y.fire('information_type:change');
+            Y.Assert.throwsError(
+                'Information type change event without new value',
+                Y.fire.bind(Y, 'information_type:change'));
         },
 
         test_change_event_fires: function () {

=== modified file 'lib/lp/app/javascript/tests/test_lp_names.js'
--- lib/lp/app/javascript/tests/test_lp_names.js	2013-03-20 03:41:40 +0000
+++ lib/lp/app/javascript/tests/test_lp_names.js	2017-07-22 23:50:27 +0000
@@ -12,41 +12,36 @@
     tests.suite.add(new Y.Test.Case({
         name: 'lp.names.launchpad_to_css() test',
 
-        _should: {
-            error: {
-                test_not_lp_name_error:
-                    'Passed value "~" is not a valid Launchpad name.',
-                test_start_with_plus_error:
-                    'Passed value "+name" is not a valid Launchpad name.',
-                test_start_with_dot_error:
-                    'Passed value ".name" is not a valid Launchpad name.',
-                test_start_with_dash_error:
-                    'Passed value "-name" is not a valid Launchpad name.'
-            }
-        },
-
         test_not_lp_name_error: function() {
             // Anything but a-z0-9+-. is not allowed in a LP name.
             // This triggers an exception.
-            names.launchpad_to_css('~');
+            Y.Assert.throwsError(
+                'Passed value "~" is not a valid Launchpad name.',
+                names.launchpad_to_css.bind(names, '~'));
         },
 
         test_start_with_plus_error: function() {
             // Strings starting with plus character are
             // invalid LP names and throw an exception.
-            names.launchpad_to_css('+name');
+            Y.Assert.throwsError(
+                'Passed value "+name" is not a valid Launchpad name.',
+                names.launchpad_to_css.bind(names, '+name'));
         },
 
         test_start_with_dot_error: function() {
             // Strings starting with dot character are
             // invalid LP names and throw an exception.
-            names.launchpad_to_css('.name');
+            Y.Assert.throwsError(
+                'Passed value ".name" is not a valid Launchpad name.',
+                names.launchpad_to_css.bind(names, '.name'));
         },
 
         test_start_with_dash_error: function() {
             // Strings starting with dash character are
             // invalid LP names and throw an exception.
-            names.launchpad_to_css('-name');
+            Y.Assert.throwsError(
+                'Passed value "-name" is not a valid Launchpad name.',
+                names.launchpad_to_css.bind(names, '-name'));
         },
 
         test_valid_in_both: function() {
@@ -115,42 +110,37 @@
     tests.suite.add(new Y.Test.Case({
         name: 'lp.names.css_to_launchpad() test',
 
-        _should: {
-            error: {
-                test_not_css_class_error:
-                    'Passed value "+" is not a valid CSS class name.',
-                test_start_with_digit_error:
-                    'Passed value "1name" is not a valid CSS class name.',
-                test_non_lp_converted_name_error:
-                    'Passed value "_name" is not produced by launchpad_to_css.',
-                test_non_lp_converted_name_error2:
-                    'Passed value "na_me" is not produced by launchpad_to_css.'
-            }
-        },
-
         test_not_css_class_error: function() {
             // Anything but a-z0-9_-. is not allowed in a LP name.
             // This triggers an exception.
-            names.css_to_launchpad('+');
+            Y.Assert.throwsError(
+                'Passed value "+" is not a valid CSS class name.',
+                names.css_to_launchpad.bind(names, '+'));
         },
 
         test_start_with_digit_error: function() {
             // Strings starting with underscore are not valid CSS class names.
-            names.css_to_launchpad('1name');
+            Y.Assert.throwsError(
+                'Passed value "1name" is not a valid CSS class name.',
+                names.css_to_launchpad.bind(names, '1name'));
         },
 
         test_non_lp_converted_name_error: function() {
             // Strings which are otherwise valid CSS class names, but
             // could not be the result of the launchpad_to_css conversion
             // are rejected with an exception.
-            names.css_to_launchpad('_name');
+            Y.Assert.throwsError(
+                'Passed value "_name" is not produced by launchpad_to_css.',
+                names.css_to_launchpad.bind(names, '_name'));
         },
 
         test_non_lp_converted_name_error2: function() {
             // Strings which are otherwise valid CSS class names, but
             // could not be the result of the launchpad_to_css conversion
             // are rejected with an exception.
-            names.css_to_launchpad('na_me');
+            Y.Assert.throwsError(
+                'Passed value "na_me" is not produced by launchpad_to_css.',
+                names.css_to_launchpad.bind(names, 'na_me'));
         },
 
         test_valid_in_both: function() {

=== modified file 'lib/lp/bugs/javascript/tests/test_bug_notification_level.js'
--- lib/lp/bugs/javascript/tests/test_bug_notification_level.js	2017-07-22 23:50:27 +0000
+++ lib/lp/bugs/javascript/tests/test_bug_notification_level.js	2017-07-22 23:50:27 +0000
@@ -302,13 +302,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'Test initial set-up of the level options display.',
 
-        _should: {
-            error: {
-                test_multiple_nodes_with_level_options: new Error(
-                    'There are multiple bug-notification-level-field nodes.')
-            }
-        },
-
         setUp: function () {
             this.MY_NAME = "ME";
             window.LP = { links: { me: "/~" + this.MY_NAME } };
@@ -335,7 +328,10 @@
             this.root.appendChild(
                 Y.Node.create('<div></div>')
                     .addClass('bug-notification-level-field'));
-            module.setup();
+            Y.Assert.throwsError(
+                new Error(
+                    'There are multiple bug-notification-level-field nodes.'),
+                module.setup.bind(module));
         },
 
         test_no_level_options: function() {

=== modified file 'lib/lp/bugs/javascript/tests/test_subscription.js'
--- lib/lp/bugs/javascript/tests/test_subscription.js	2017-07-22 23:50:27 +0000
+++ lib/lp/bugs/javascript/tests/test_subscription.js	2017-07-22 23:50:27 +0000
@@ -932,17 +932,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'Get reason and actions for a direct subscription',
 
-        _should: {
-            error: {
-                test_multiple_direct_subscriptions:
-                new Error('Programmer error: a person should not have more than '+
-                          'one direct personal subscription.'),
-                test_direct_subscription_at_unknown_level:
-                new Error('Programmer error: unknown bug notification level: '+
-                          'The Larch')
-            }
-        },
-
         setUp: function() {
             window.LP = {cache: {subscription_info: []}};
         },
@@ -954,12 +943,15 @@
         test_multiple_direct_subscriptions: function() {
             // It should not be possible to have multiple direct,
             // personal subscriptions.
-            // This errors out (see _should.error above).
             var info = {
                 direct: _constructCategory(['1', '2']),
                 count: 2
             };
-            module._get_direct_subscription_information(info);
+            Y.Assert.throwsError(
+                new Error('Programmer error: a person should not have more ' +
+                          'than one direct personal subscription.'),
+                module._get_direct_subscription_information.bind(
+                    module, info));
         },
 
         test_no_subscriptions_at_all: function() {
@@ -1154,8 +1146,11 @@
                 direct: _constructCategory([sub]),
                 count: 1
             };
-            // This should raise an error.
-            module._get_direct_subscription_information(info);
+            Y.Assert.throwsError(
+                new Error('Programmer error: unknown bug notification ' +
+                          'level: The Larch'),
+                module._get_direct_subscription_information.bind(
+                    module, info));
         },
 
         test_direct_subscription_as_reporter: function() {
@@ -1253,12 +1248,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'Test conversion of ObjectLink to HTML.',
 
-        _should: {
-            error: {
-                test_non_link: new Error('Not a proper ObjectLink.')
-            }
-        },
-
         test_string: function() {
             // When a string is passed in, it is returned unmodified.
             var link = 'test';
@@ -1269,9 +1258,11 @@
 
         test_non_link: function() {
             // When an object that doesn't have both 'title' and 'url'
-            // passed in, it fails. (see _should.error above)
+            // passed in, it fails.
             var link = {};
-            module._get_objectlink_html(link);
+            Y.Assert.throwsError(
+                new Error('Not a proper ObjectLink.'),
+                module._get_objectlink_html.bind(module, link));
         },
 
         test_simple: function() {
@@ -1324,12 +1315,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'Test variable substitution in subscription descriptions.',
 
-        _should: {
-            error: {
-                test_non_link: new Error('Not a proper ObjectLink.')
-            }
-        },
-
         test_no_variables: function() {
             // For a string with no variables, no substitution is performed.
             var sub = {
@@ -2118,14 +2103,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'Test duplicate actions text and subscriptions list.',
 
-        _should: {
-            error: {
-                test_multiple_teams_fails:
-                new Error('We can only unsubscribe a single team from ' +
-                          'multiple duplicate bugs.')
-            }
-        },
-
         setUp: function() {
             window.LP = { cache: { context: { web_link: 'http://test/' } },
                           links: { me: '~' } };
@@ -2204,7 +2181,11 @@
             var args = { bugs: [ { self: { self_link: 'http://bug/' } } ],
                          teams: [ { self: { self_link: 'http://team1/' } },
                                   { self: { self_link: 'http://team2/' } }] };
-            module._get_unsubscribe_duplicates_text_and_subscriptions(args);
+            Y.Assert.throwsError(
+                new Error('We can only unsubscribe a single team from ' +
+                          'multiple duplicate bugs.'),
+                module._get_unsubscribe_duplicates_text_and_subscriptions.bind(
+                    module, args));
         }
 
     }));
@@ -2411,14 +2392,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'Test duplicate actions text and subscriptions list.',
 
-        _should: {
-            error: {
-                test_multiple_teams_fails:
-                new Error('We can only unsubscribe a single team from ' +
-                          'multiple duplicate bugs.')
-            }
-        },
-
         setUp: function() {
             window.LP = { cache: { context: { bug_link: 'http://bug/' } },
                           links: { me: '~' } };
@@ -2460,7 +2433,11 @@
             var args = { bugs: [ { self: { self_link: 'http://bug/' } } ],
                          teams: [ { self: { self_link: 'http://team1/' } },
                                   { self: { self_link: 'http://team2/' } }] };
-            module._get_unsubscribe_duplicates_text_and_subscriptions(args);
+            Y.Assert.throwsError(
+                new Error('We can only unsubscribe a single team from ' +
+                          'multiple duplicate bugs.'),
+                module._get_unsubscribe_duplicates_text_and_subscriptions.bind(
+                    module, args));
         }
 
     }));

=== modified file 'lib/lp/code/javascript/tests/test_productseries_setbranch.js'
--- lib/lp/code/javascript/tests/test_productseries_setbranch.js	2012-10-26 10:00:20 +0000
+++ lib/lp/code/javascript/tests/test_productseries_setbranch.js	2017-07-22 23:50:27 +0000
@@ -13,13 +13,6 @@
         // Test the onclick results.
         name: 'select_branchtype',
 
-        _should: {
-            error: {
-                //test_config_undefined: true,
-                //test_missing_tbody_is_an_error: true
-                }
-        },
-
         setUp: function() {
            this.tbody = Y.one('#productseries-setbranch');
 

=== modified file 'lib/lp/registry/javascript/tests/test_milestone_creation_failures.js'
--- lib/lp/registry/javascript/tests/test_milestone_creation_failures.js	2013-03-20 03:41:40 +0000
+++ lib/lp/registry/javascript/tests/test_milestone_creation_failures.js	2017-07-22 23:50:27 +0000
@@ -11,11 +11,6 @@
         // Test the setup method.
         name: 'Milestone creation error handling',
 
-        _should: {
-            error: {
-                }
-        },
-
         setUp: function() {
             this.mockio = new Y.lp.testing.mockio.MockIo();
             this.client = new Y.lp.client.Launchpad({

=== modified file 'lib/lp/registry/javascript/tests/test_milestone_table.js'
--- lib/lp/registry/javascript/tests/test_milestone_table.js	2013-03-20 03:41:40 +0000
+++ lib/lp/registry/javascript/tests/test_milestone_table.js	2017-07-22 23:50:27 +0000
@@ -10,14 +10,6 @@
         // Test the setup method.
         name: 'setup',
 
-        _should: {
-            error: {
-                test_config_undefined: true,
-                test_config_property_undefined: true,
-                test_missing_tbody_is_an_error: true
-                }
-            },
-
         setUp: function() {
             this.tbody = Y.one('#milestone-rows');
             },
@@ -43,7 +35,9 @@
 
         test_config_undefined: function() {
             // Verify an error is thrown if there is no config.
-            milestonetable.setup();
+            Y.Assert.throwsError(
+                new Error('Missing setup config for milestonetable.'),
+                milestonetable.setup.bind(milestonetable));
             },
 
         test_config_property_undefined: function() {
@@ -51,7 +45,11 @@
             var config = {
                 milestone_row_uri_template: '/uri'
                 };
-            milestonetable.setup(config);
+            Y.Assert.throwsError(
+                new Error(
+                    'Undefined properties in setup config for ' +
+                    'milestonetable.'),
+                milestonetable.setup.bind(milestonetable, config));
             },
 
         test_missing_tbody_is_an_error: function() {
@@ -60,7 +58,9 @@
                 milestone_row_uri_template: '/uri',
                 milestone_rows_id:  'does-not-exist'
                 };
-            milestonetable.setup(config);
+            Y.Assert.throwsError(
+                new Error("'does-not-exist' not in page."),
+                milestonetable.setup.bind(milestonetable, config));
             }
         }));
 

=== modified file 'lib/lp/registry/javascript/tests/test_structural_subscription.js'
--- lib/lp/registry/javascript/tests/test_structural_subscription.js	2017-07-22 23:50:27 +0000
+++ lib/lp/registry/javascript/tests/test_structural_subscription.js	2017-07-22 23:50:27 +0000
@@ -85,16 +85,6 @@
     test_case = new Y.Test.Case({
         name: 'structural_subscription_overlay',
 
-        _should: {
-            error: {
-                test_setup_config_none: new Error(
-                    'Missing config for structural_subscription.'),
-                test_setup_config_no_content_box: new Error(
-                    'Structural_subscription configuration has undefined '+
-                    'properties.')
-                }
-        },
-
         setUp: function() {
             // Monkeypatch LP to avoid network traffic and to allow
             // insertion of test data.
@@ -128,12 +118,18 @@
 
         test_setup_config_none: function() {
             // The config passed to setup may not be null.
-            module.setup();
+            Y.Assert.throwsError(
+                new Error('Missing config for structural_subscription.'),
+                module.setup.bind(module));
         },
 
         test_setup_config_no_content_box: function() {
             // The config passed to setup must contain a content_box.
-            module.setup({});
+            Y.Assert.throwsError(
+                new Error(
+                    'Structural_subscription configuration has undefined ' +
+                    'properties.'),
+                module.setup.bind(module, {}));
         },
 
         test_anonymous: function() {
@@ -185,10 +181,6 @@
     test_case = new Y.Test.Case({
         name: 'Structural Subscription Overlay save_subscription',
 
-        _should: {
-            error: {}
-        },
-
         setUp: function() {
             // Monkeypatch LP to avoid network traffic and to allow
             // insertion of test data.
@@ -270,11 +262,6 @@
     test_case = new Y.Test.Case({
         name: 'Structural Subscription validation tests',
 
-        _should: {
-            error: {
-                }
-        },
-
         setUp: function() {
             // Monkeypatch LP to avoid network traffic and to allow
             // insertion of test data.
@@ -326,8 +313,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'Dialog title ellipsis',
 
-        _should: {error: {}},
-
         setUp: function() {
             // Monkeypatch LP to avoid network traffic and to allow
             // insertion of test data.
@@ -382,13 +367,6 @@
     test_case = new Y.Test.Case({
         name: 'Structural Subscription interaction tests',
 
-        _should: {
-            error: {
-                test_setup_overlay_missing_content_box: new Error(
-                    'Node not found: #sir-not-appearing-in-this-test')
-                }
-        },
-
         setUp: function() {
             // Monkeypatch LP to avoid network traffic and to allow
             // insertion of test data.
@@ -476,10 +454,12 @@
 
         test_setup_overlay_missing_content_box: function() {
             // Pass in a content_box with a missing id to trigger an error.
-            this.configuration.content_box =
-                '#sir-not-appearing-in-this-test';
+            var content_box = '#sir-not-appearing-in-this-test';
+            this.configuration.content_box = content_box;
             module.setup(this.configuration);
-            module._setup_overlay(this.configuration.content_box);
+            Y.Assert.throwsError(
+                new Error('Node not found: #sir-not-appearing-in-this-test'),
+                module._setup_overlay.bind(module, content_box));
         },
 
         test_initial_state: function() {
@@ -593,11 +573,6 @@
         // Test the setup method.
         name: 'Structural Subscription error handling',
 
-        _should: {
-            error: {
-                }
-        },
-
         setUp: function() {
           // Monkeypatch LP to avoid network traffic and to allow
           // insertion of test data.
@@ -695,8 +670,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'Structural Subscription team contact',
 
-        _should: {error: {}},
-
         setUp: function() {
             // Monkeypatch LP to avoid network traffic and to allow
             // insertion of test data.
@@ -854,8 +827,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'Structural Subscription: deleting failed filters',
 
-        _should: {error: {}},
-
         setUp: function() {
             // Monkeypatch LP to avoid network traffic and to allow
             // insertion of test data.
@@ -930,26 +901,20 @@
     tests.suite.add(new Y.Test.Case({
         name: 'Structural Subscription validate_config',
 
-        _should: {
-            error: {
-                test_setup_config_none: new Error(
-                    'Missing config for structural_subscription.'),
-                test_setup_config_no_content_box: new Error(
-                    'Structural_subscription configuration has undefined '+
-                    'properties.')
-                }
-        },
-
-        // Included in _should/error above.
         test_setup_config_none: function() {
             // The config passed to setup may not be null.
-            module._validate_config();
+            Y.Assert.throwsError(
+                new Error('Missing config for structural_subscription.'),
+                module._validate_config.bind(module));
         },
 
-        // Included in _should/error above.
         test_setup_config_no_content_box: function() {
             // The config passed to setup must contain a content_box.
-            module._validate_config({});
+            Y.Assert.throwsError(
+                new Error(
+                    'Structural_subscription configuration has undefined ' +
+                    'properties.'),
+                module._validate_config.bind(module, {}));
         }
     }));
 
@@ -960,11 +925,6 @@
         // add/edit form are correctly extracted by the extract_form_data
         // function.
 
-        _should: {
-            error: {
-                }
-            },
-
         test_extract_description: function() {
             var form_data = {
                 name: ['filter description'],
@@ -1097,8 +1057,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'Structural Subscription: add subcription workflow',
 
-        _should: {error: {}},
-
         setUp: function() {
             var TestBugFilter = function() {};
             TestBugFilter.prototype = {
@@ -1190,8 +1148,6 @@
         name:
             'Structural Subscription: make_add_subscription_success_handler',
 
-        _should: {error: {}},
-
         setUp: function() {
             this.TestBugFilter = function() {};
             this.TestBugFilter.prototype = {
@@ -1237,8 +1193,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'Structural Subscription: edit subcription workflow',
 
-        _should: {error: {}},
-
         setUp: function() {
             var TestBugFilter = function(data) {
                 if (data !== undefined) {
@@ -1369,8 +1323,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'Structural Subscription: unsubscribing',
 
-        _should: {error: {}},
-
         setUp: function() {
             var TestClient = function() {};
             this.original_lp = monkeypatch_LP();
@@ -1638,11 +1590,6 @@
         // Verify that the mute controls and labels on the edit block
         // render and interact properly
 
-        _should: {
-            error: {
-                }
-            },
-
         setUp: function() {
             // Monkeypatch LP to avoid network traffic and to allow
             // insertion of test data.
@@ -1794,8 +1741,6 @@
     tests.suite.add(new Y.Test.Case({
         name: 'Structural Subscription: enable/disable help link',
 
-        _should: {error: {}},
-
         setUp: function() {
             this.original_lp = monkeypatch_LP();
 

=== modified file 'lib/lp/testing/tests/test_yuixhr_fixture.js'
--- lib/lp/testing/tests/test_yuixhr_fixture.js	2017-07-22 23:50:27 +0000
+++ lib/lp/testing/tests/test_yuixhr_fixture.js	2017-07-22 23:50:27 +0000
@@ -15,13 +15,6 @@
         delete this._lp_fixture_data;
     },
 
-    _should: {
-        error: {
-            test_bad_http_call_raises_error: true,
-            test_bad_http_teardown_raises_error: true
-        }
-    },
-
     test_simple_setup: function() {
         var data = module.setup(this, 'baseline');
         Y.ArrayAssert.itemsAreEqual(['baseline'], this._lp_fixture_setups);
@@ -60,7 +53,9 @@
     },
 
     test_bad_http_call_raises_error: function() {
-        module.setup(this, 'does not exist');
+        var that = this;
+        Y.Assert.throwsError(
+            Error, module.setup.bind(module, that, 'does not exist'));
     },
 
     test_bad_http_call_shows_traceback: function() {
@@ -73,7 +68,8 @@
 
     test_bad_http_teardown_raises_error: function() {
         module.setup(this, 'teardown_will_fail');
-        module.teardown(this);
+        var that = this;
+        Y.Assert.throwsError(Error, module.teardown.bind(module, that));
     },
 
     test_bad_http_teardown_shows_traceback: function() {


Follow ups