← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~rharding/launchpad/yui35_test into lp:launchpad

 

Richard Harding has proposed merging lp:~rharding/launchpad/yui35_test into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rharding/launchpad/yui35_test/+merge/111911

= Summary =

This branch begins testing against YUI3.5. This branch only looks at making
tests pass in both our current YUI and 3.5 in the app/javascript directory.


== Pre Implementation ==

Talked with Deryck to start work.


== Implementation Notes ==

This leaves out two failing tests
listing_navigator
lp_names

They both have their own methods of running tests and try to setup their own
YUI configs. This causes them to blow up more than the other tests and will
require more refactoring in a follow up branch.

Most of the failures are around the test runner itself, or in selectors based
on attributes not quoting the attribute value.

This updates those cases as well as a few other misc items required to get the
tests to run and display correctly.


== Tests ==

./bin/test -x -cvv --layer=YUITestLayer


== Lint ==

Linting changed files:
  lib/lp/app/javascript/client.js
  lib/lp/app/javascript/banners/tests/test_banner.js
  lib/lp/app/javascript/banners/tests/test_beta_notification.js
  lib/lp/app/javascript/banners/tests/test_privacy.js
  lib/lp/app/javascript/indicator/indicator.js
  lib/lp/app/javascript/picker/person_picker.js
  lib/lp/app/javascript/picker/tests/test_personpicker.js
  lib/lp/app/javascript/testing/testrunner.js
  lib/lp/app/javascript/tests/test_lp_client.js
  lib/lp/app/javascript/tests/test_multicheckboxwidget.js



== LoC Qualification ==

There are two qualifications:

1. Fixing these tests reduces tech debt and eases maintenance.
2. Getting to YUI 3.5 will allow us to use the build in calendar widget and
will remove all of the YUI2 code from the code base which is aroud 12K LoC
(non-minified) and over 27K lines of total files.
-- 
https://code.launchpad.net/~rharding/launchpad/yui35_test/+merge/111911
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/yui35_test into lp:launchpad.
=== modified file 'lib/lp/app/javascript/banners/tests/test_banner.js'
--- lib/lp/app/javascript/banners/tests/test_banner.js	2012-05-22 22:08:07 +0000
+++ lib/lp/app/javascript/banners/tests/test_banner.js	2012-06-26 17:59:21 +0000
@@ -17,7 +17,8 @@
         },
 
         tearDown: function () {
-            Y.one('body').get('children').remove(true);
+            Y.one('#maincontent').remove(true);
+            Y.all('.yui3-banner').remove(true);
         },
 
         test_library_exists: function () {
@@ -47,7 +48,7 @@
             var banner = new Y.lp.app.banner.Banner({ skip_animation: true });
             banner.render();
 
-            var banner_node = Y.one(".global-notification")
+            var banner_node = Y.one(".global-notification");
             Y.Assert.isObject(banner_node);
             Y.Assert.isTrue(banner_node.hasClass('hidden'));
         },
@@ -71,7 +72,7 @@
         test_show: function() {
             var banner = new Y.lp.app.banner.Banner({ skip_animation: true });
             banner.render();
-            banner.show(); 
+            banner.show();
             var banner_node = Y.one(".global-notification");
             Y.Assert.isFalse(banner_node.hasClass('hidden'));
         },
@@ -87,7 +88,7 @@
             var wait_for_anim = 20;
             var check = function () {
                 Y.Assert.isTrue(banner_node.hasClass('hidden'));
-            }
+            };
             banner.hide();
             this.wait(check, wait_for_anim);
         },
@@ -101,7 +102,7 @@
             Y.Assert.areEqual(new_text, banner_node.get('text'));
 
             banner.updateText();
-            var banner_node = Y.one(".global-notification");
+            banner_node = Y.one(".global-notification");
             Y.Assert.areEqual("", banner_node.get('text'));
         }
     }));

=== modified file 'lib/lp/app/javascript/banners/tests/test_beta_notification.js'
--- lib/lp/app/javascript/banners/tests/test_beta_notification.js	2012-05-21 13:52:53 +0000
+++ lib/lp/app/javascript/banners/tests/test_beta_notification.js	2012-06-26 17:59:21 +0000
@@ -24,7 +24,8 @@
         },
 
         tearDown: function () {
-            Y.one('body').get('children').remove(true);
+            Y.one('#maincontent').remove(true);
+            Y.all('.yui3-banner').remove(true);
         },
 
         test_library_exists: function () {
@@ -40,7 +41,7 @@
                     url: 'http://lp.dev/LEP/one'
             }};
             var betabanner = new Y.lp.app.banner.beta.BetaBanner(
-                {skip_animation: true}); 
+                {skip_animation: true});
             betabanner.render();
             betabanner.show();
 
@@ -72,7 +73,7 @@
                     url: ''
                 }};
             var betabanner = new Y.lp.app.banner.beta.BetaBanner(
-                {skip_animation: true}); 
+                {skip_animation: true});
             betabanner.render();
             betabanner.show();
 
@@ -101,7 +102,7 @@
                     title: 'Non-beta feature',
                     url: 'http://example.org'
                 }};
-            Y.lp.app.banner.beta.show_beta_if_needed(); 
+            Y.lp.app.banner.beta.show_beta_if_needed();
             Y.Assert.isNull(Y.one('.global-notification'));
         },
 
@@ -113,7 +114,7 @@
                     url: 'http://lp.dev/LEP/one'
             }};
             var betabanner = new Y.lp.app.banner.beta.BetaBanner(
-                {skip_animation: true}); 
+                {skip_animation: true});
             betabanner.render();
             betabanner.show();
             var body = Y.one('body');
@@ -126,7 +127,7 @@
                 Y.Assert.isTrue(banner.hasClass('hidden'));
                 Y.Assert.isFalse(
                     body.hasClass('global-notification-visible'));
-            }
+            };
             close_link = Y.one('.global-notification-close');
             close_link.simulate('click');
             var wait_time = 20;

=== modified file 'lib/lp/app/javascript/banners/tests/test_privacy.js'
--- lib/lp/app/javascript/banners/tests/test_privacy.js	2012-05-22 22:13:06 +0000
+++ lib/lp/app/javascript/banners/tests/test_privacy.js	2012-06-26 17:59:21 +0000
@@ -19,7 +19,8 @@
         },
 
         tearDown: function () {
-            Y.one('body').get('children').remove(true);
+            Y.one('#maincontent').remove(true);
+            Y.all('.yui3-banner').remove(true);
         },
 
         test_library_exists: function () {
@@ -43,7 +44,7 @@
             Y.Assert.areEqual(1, Y.all('.global-notification').size());
 
             var new_text = 'This is new text';
-            var banner = Y.lp.app.banner.privacy.getPrivacyBanner(new_text);
+            banner = Y.lp.app.banner.privacy.getPrivacyBanner(new_text);
             Y.Assert.areEqual(1, Y.all('.global-notification').size());
             var banner_node = Y.one('.global-notification');
             Y.Assert.areEqual(

=== modified file 'lib/lp/app/javascript/client.js'
--- lib/lp/app/javascript/client.js	2012-03-27 04:36:24 +0000
+++ lib/lp/app/javascript/client.js	2012-06-26 17:59:21 +0000
@@ -1022,7 +1022,7 @@
         }
         // Display the field specific errors.
         Y.each(errors, function(message, field_name) {
-            var label = Y.one('label[for=' + field_name + ']');
+            var label = Y.one('label[for="' + field_name + '"]');
             if (Y.Lang.isValue(label)) {
                 label.ancestor('div').addClass('error');
                 var field = label.next('div');

=== modified file 'lib/lp/app/javascript/indicator/indicator.js'
--- lib/lp/app/javascript/indicator/indicator.js	2012-01-19 02:00:52 +0000
+++ lib/lp/app/javascript/indicator/indicator.js	2012-06-26 17:59:21 +0000
@@ -204,4 +204,4 @@
     indicator.OverlayIndicator = OverlayIndicator;
     indicator.actions = actions;
 
-}, '0.1', {requires: ['base', 'widget']});
+}, '0.1', {requires: ['base', 'node-screen', 'widget']});

=== modified file 'lib/lp/app/javascript/picker/person_picker.js'
--- lib/lp/app/javascript/picker/person_picker.js	2012-06-26 00:15:11 +0000
+++ lib/lp/app/javascript/picker/person_picker.js	2012-06-26 17:59:21 +0000
@@ -123,8 +123,8 @@
 
     _save_new_team: function() {
         var node = this.get('contentBox').one('.new-team-node');
-        var team_name = node.one('[id=field.name]').get('value');
-        var team_display_name = node.one('[id=field.displayname]')
+        var team_name = node.one('[id="field.name"]').get('value');
+        var team_display_name = node.one('[id="field.displayname"]')
             .get('value');
         this.hide_extra_content(node, false);
         // TODO - make back end call to save team

=== modified file 'lib/lp/app/javascript/picker/tests/test_personpicker.js'
--- lib/lp/app/javascript/picker/tests/test_personpicker.js	2012-06-26 00:15:11 +0000
+++ lib/lp/app/javascript/picker/tests/test_personpicker.js	2012-06-26 17:59:21 +0000
@@ -1,4 +1,5 @@
-/* Copyright 2011 Canonical Ltd.  This software is licensed under the * GNU Affero General Public License version 3 (see the file LICENSE).
+/* Copyright 2011 Canonical Ltd.  This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
  */
 
 YUI().use('test', 'console', 'plugin',
@@ -366,8 +367,7 @@
                 'Enter new team details',
                 this.picker.get('headerContent').get('text'));
             Y.Assert.isNotNull(
-                this.picker.get('contentBox')
-                    .one('input[id=field.name]'));
+                this.picker.get('contentBox').one('input[id="field.name"]'));
             Y.Assert.areEqual('none',
                 this.picker.get('contentBox').one('.yui3-widget-bd')
                     .getStyle('display'));
@@ -392,7 +392,7 @@
                 'Pick Someone',
                 this.picker.get('headerContent').get('text'));
             Y.Assert.isNotNull(
-                this.picker.get('contentBox').one('input[id=field.name]')
+                this.picker.get('contentBox').one('input[id="field.name"]')
                     .ancestor('div.hidden'));
             Y.Assert.isNotNull(
                 this.picker.get('contentBox').one('.yui3-picker-search'));
@@ -417,7 +417,7 @@
             var new_team =
                 picker_content.one('.yui-picker-new-team-button');
             new_team.simulate('click');
-            var team_name = picker_content.one('input[id=field.name]');
+            var team_name = picker_content.one('input[id="field.name"]');
             team_name.set('value', 'fred');
             var form_buttons = picker_content.one('.extra-form-buttons');
             simulate(
@@ -546,7 +546,10 @@
 
     // Hook for the test runner to get test results.
     var handle_complete = function(data) {
-        window.status = '::::' + JSON.stringify(data);
+        window.status = '::::' + JSON.stringify({
+            results: data.results,
+            type: data.type
+        });
     };
     Y.Test.Runner.on('complete', handle_complete);
     Y.Test.Runner.add(suite);

=== modified file 'lib/lp/app/javascript/testing/testrunner.js'
--- lib/lp/app/javascript/testing/testrunner.js	2011-07-18 11:52:51 +0000
+++ lib/lp/app/javascript/testing/testrunner.js	2012-06-26 17:59:21 +0000
@@ -21,8 +21,11 @@
 
     // Lock, stock, and two smoking barrels.
     var handle_complete = function(data) {
-        window.status = '::::' + JSON.stringify(data);
-        };
+        window.status = '::::' + JSON.stringify({
+            results: data.results,
+            type: data.type
+        });
+    };
     Y.Test.Runner.on('complete', handle_complete);
     Y.Test.Runner.add(suite);
 

=== modified file 'lib/lp/app/javascript/tests/test_lp_client.js'
--- lib/lp/app/javascript/tests/test_lp_client.js	2012-03-27 04:36:24 +0000
+++ lib/lp/app/javascript/tests/test_lp_client.js	2012-06-26 17:59:21 +0000
@@ -74,7 +74,9 @@
     test_append_qs: function() {
         var qs = "";
         qs = Y.lp.client.append_qs(qs, "Pöllä", "Perelló");
-        Assert.areEqual("P%C3%83%C2%B6ll%C3%83%C2%A4=Perell%C3%83%C2%B3", qs);
+        Assert.areEqual(
+            "P%C3%83%C2%B6ll%C3%83%C2%A4=Perell%C3%83%C2%B3", qs,
+            'This tests is known to fail in Chrome, it is browser specific.');
     },
 
     test_append_qs_with_array: function() {
@@ -494,13 +496,15 @@
     _assert_error_rendering: function() {
         var label = Y.one('label[for="field.test"]');
         var field_error = label.next('div').next('.message');
-        Y.Assert.isTrue(Y.one('#field_div').hasClass('error'));
+        Y.Assert.isTrue(Y.one('#field_div').hasClass('error'),
+                       'Field div has class error');
         Y.Assert.areEqual('Field error', field_error.getContent());
         Y.all('.error.message').each(function(error_node) {
             var error_message = error_node.getContent();
             Y.Assert.isTrue(
                 error_message === '<p>Form error</p>' ||
-                error_message === 'Some errors');
+                error_message === 'Some errors',
+                'Each error message has the correct content.');
         });
     },
 

=== modified file 'lib/lp/app/javascript/tests/test_multicheckboxwidget.js'
--- lib/lp/app/javascript/tests/test_multicheckboxwidget.js	2011-07-08 05:12:39 +0000
+++ lib/lp/app/javascript/tests/test_multicheckboxwidget.js	2012-06-26 17:59:21 +0000
@@ -110,7 +110,7 @@
         var x;
         for (x = 0; x < 2; x++) {
             var item = Y.one(
-                  'input[id="field.multicheckboxtest.'+x+'][type=checkbox]');
+                  'input[id="field.multicheckboxtest.'+x+'"][type="checkbox"]');
             Y.Assert.areEqual(item.getAttribute('value'), x);
             Y.Assert.areEqual(item.get('checked'), x === 0);
         }
@@ -121,7 +121,7 @@
         this.createWidget();
         var x;
         for (x = 0; x < 2; x++) {
-            var item = Y.one('label[for="field.multicheckboxtest.'+x+']');
+            var item = Y.one('label[for="field.multicheckboxtest.'+x+'"]');
             var txt = item.get('textContent');
             //remove any &nbsp in the text
             txt = txt.replace(/^[\s\xA0]+/g,'').replace(/[\s(&nbsp;)]+$/g,'');


Follow ups