← Back to team overview

launchpad-reviewers team mailing list archive

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

 

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

= Summary =

Start to update issues found in testing Bug module code in YUI 3.5.

== Implementation Notes ==

We hit another case of needing to reset history so this creates a new testing module 'helpers' and the method is moved there. The helpers is added to the standard default files for JS testing.

One test was found to not be needed any longer.

Fixes a bunch of the missing quotes around selectors that require them.

Also does some misc linting for trailing spaces.

== Tests ==

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

== Lint ==

Linting changed files:
  standard_test_template.html
  standard_test_template.js
  lib/lp/app/javascript/choice.js
  lib/lp/app/javascript/testing/helpers.js
  lib/lp/app/javascript/tests/test_listing_navigator.html
  lib/lp/bugs/javascript/filebug.js
  lib/lp/bugs/javascript/tests/test_buglisting_utils.html
  lib/lp/bugs/javascript/tests/test_buglisting_utils.js
  lib/lp/bugs/javascript/tests/test_filebug.js
  lib/lp/bugs/javascript/tests/test_information_type_choice.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/bug_yui35_one/+merge/112399
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/bug_yui35_one into lp:launchpad.
=== modified file 'lib/lp/app/javascript/choice.js'
--- lib/lp/app/javascript/choice.js	2012-06-16 21:26:54 +0000
+++ lib/lp/app/javascript/choice.js	2012-06-27 17:41:22 +0000
@@ -126,7 +126,7 @@
  * @param show_description
  */
 namespace.addPopupChoice = function(field_name, choices, show_description) {
-    var legacy_node = Y.one('[id=field.' + field_name + ']');
+    var legacy_node = Y.one('[id="field.' + field_name + '"]');
     if (!Y.Lang.isValue(legacy_node)) {
         return;
     }
@@ -148,15 +148,14 @@
  */
 namespace.addPopupChoiceForRadioButtons = function(field_name, choices,
                                                    show_description) {
-
-    var legacy_node = Y.one('[name=field.' + field_name + ']')
+    var legacy_node = Y.one('[name="field.' + field_name + '"]')
         .ancestor('table.radio-button-widget');
     if (!Y.Lang.isValue(legacy_node)) {
         return;
     }
     var get_fn = function(node) {
         var field_value = choices[0].value;
-        node.all('input[name=field.' + field_name + ']').each(function(node) {
+        node.all('input[name="field.' + field_name + '"]').each(function(node) {
             if (node.get('checked')) {
                 field_value = node.get('value');
             }
@@ -164,7 +163,7 @@
         return field_value;
     };
     var set_fn = function(node, value) {
-        node.all('input[name=field.' + field_name + ']')
+        node.all('input[name="field.' + field_name + '"]')
                 .each(function(node) {
             var node_selected = node.get('value') === value;
             node.set('checked', node_selected);

=== added file 'lib/lp/app/javascript/testing/helpers.js'
--- lib/lp/app/javascript/testing/helpers.js	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/testing/helpers.js	2012-06-27 17:41:22 +0000
@@ -0,0 +1,24 @@
+/* Copyright (c) 2011, Canonical Ltd. All rights reserved. */
+
+YUI.add('lp.testing.helpers', function(Y) {
+
+    var ns = Y.namespace('lp.testing.helpers');
+
+
+    /**
+     * Reset the window history state.
+     *
+     * Useful for tearDown for code that modifies and stores data into the
+     * History.state.
+     * @function
+     */
+    ns.reset_history = function () {
+        var win = Y.config.win;
+        var originalURL = (win && win.location.toString()) || '';
+        win.history.replaceState(null, null, originalURL);
+    };
+
+
+}, '0.1', {
+    'requires': [ 'history']
+});

=== modified file 'lib/lp/app/javascript/tests/test_listing_navigator.html'
--- lib/lp/app/javascript/tests/test_listing_navigator.html	2012-03-14 04:41:36 +0000
+++ lib/lp/app/javascript/tests/test_listing_navigator.html	2012-06-27 17:41:22 +0000
@@ -21,6 +21,8 @@
 
       <script type="text/javascript"
               src="../../../../../build/js/lp/app/testing/testrunner.js"></script>
+      <script type="text/javascript"
+              src="../../../../../build/js/lp/app/testing/helpers.js"></script>
 
       <link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
 

=== modified file 'lib/lp/bugs/javascript/filebug.js'
--- lib/lp/bugs/javascript/filebug.js	2012-06-20 05:25:44 +0000
+++ lib/lp/bugs/javascript/filebug.js	2012-06-27 17:41:22 +0000
@@ -55,13 +55,13 @@
     };
 
     if (LP.cache.show_userdata_as_private) {
-        info_type_descriptions.USERDATA = 'Private'; 
+        info_type_descriptions.USERDATA = 'Private';
     }
-    var text_template = "This report has {info_type} information." + 
+    var text_template = "This report has {info_type} information." +
         " You can change the information type later.";
     value = info_type_descriptions[value];
     return Y.Lang.substitute(text_template, {'info_type': value});
-}
+};
 
 var setup_information_type = function() {
     var itypes_table = Y.one('.radio-button-widget');
@@ -69,7 +69,7 @@
         var banner_text = get_new_banner_text(this.get('value'));
         var private_type = (Y.Array.indexOf(
             LP.cache.private_types, this.get('value')) >= 0);
-        
+
         update_privacy_banner(private_type, banner_text);
     }, "input[name='field.information_type']");
 };

=== modified file 'lib/lp/bugs/javascript/tests/test_buglisting_utils.html'
--- lib/lp/bugs/javascript/tests/test_buglisting_utils.html	2012-03-14 04:41:36 +0000
+++ lib/lp/bugs/javascript/tests/test_buglisting_utils.html	2012-06-27 17:41:22 +0000
@@ -21,6 +21,8 @@
 
       <script type="text/javascript"
               src="../../../../../build/js/lp/app/testing/testrunner.js"></script>
+      <script type="text/javascript"
+              src="../../../../../build/js/lp/app/testing/helpers.js"></script>
 
       <link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
 

=== modified file 'lib/lp/bugs/javascript/tests/test_buglisting_utils.js'
--- lib/lp/bugs/javascript/tests/test_buglisting_utils.js	2011-12-19 19:52:50 +0000
+++ lib/lp/bugs/javascript/tests/test_buglisting_utils.js	2012-06-27 17:41:22 +0000
@@ -1,4 +1,4 @@
-/* Copyright (c) 2011, Canonical Ltd. All rights reserved. */
+/* Copyright (c) 2012, Canonical Ltd. All rights reserved. */
 
 YUI.add('lp.buglisting_utils.test', function(Y) {
 
@@ -14,7 +14,7 @@
 var running_in_webkit = function() {
     // AppleWebKit/531 is the Lucid html5 test browser.
     // AppleWebKit/535 is Chromium 13-17 in Lucid through Oneric.
-    return (/AppleWebKit.53[15]/).test(navigator.userAgent);
+    return (/AppleWebKit.53[157]/).test(navigator.userAgent);
 };
 
 
@@ -86,6 +86,7 @@
         if (running_in_webkit()){
             Y.Cookie._setDoc(Y.config.doc);
         }
+        Y.lp.testing.helpers.reset_history();
     },
 
     /**
@@ -121,22 +122,6 @@
         Assert.areEqual(this.cookie_name, this.list_util.get('cookie_name'));
     },
 
-    test_field_visibility_defaults_readonly: function() {
-        // field_visibility_defaults is a readOnly attribute,
-        // so field_visibility_defaults will always return the same
-        // as LP.cache.field_visibility_defaults.
-        this.list_util = new Y.lp.buglisting_utils.BugListingConfigUtil();
-        var attempted_defaults = {
-            foo: 'bar',
-            baz: 'bop'
-        };
-        this.list_util.get('model').set(
-            'field_visibility_defaults', attempted_defaults);
-        ObjectAssert.areEqual(
-            this.list_util.get('field_visibility_defaults'),
-            this.defaults.field_visibility_defaults);
-    },
-
     test_field_visibility_form_reference: function() {
         // The form created from field_visibility defaults is referenced
         // via BugListingConfigUtil.get('form')
@@ -549,6 +534,7 @@
 
 buglisting_utils.suite = suite;
 
-}, '0.1', {'requires': [
-    'test', 'node-event-simulate', 'cookie', 'lp.buglisting_utils',
-    'lp.ordering']});
+}, '0.1', {
+    'requires': [ 'test', 'lp.testing.helpers', 'node-event-simulate',
+        'cookie', 'lp.buglisting_utils', 'lp.ordering']
+});

=== modified file 'lib/lp/bugs/javascript/tests/test_filebug.js'
--- lib/lp/bugs/javascript/tests/test_filebug.js	2012-06-20 05:25:44 +0000
+++ lib/lp/bugs/javascript/tests/test_filebug.js	2012-06-27 17:41:22 +0000
@@ -79,7 +79,7 @@
             Y.lp.bugs.filebug.setup_filebug(true);
             var banner_hidden = Y.one('.yui3-privacybanner-hidden');
             Y.Assert.isNotNull(banner_hidden);
-            Y.one('[id=field.information_type.2]').simulate('click');
+            Y.one('[id="field.information_type.2"]').simulate('click');
             banner_hidden = Y.one('.yui3-privacybanner-hidden');
             Y.Assert.isNull(banner_hidden);
             var banner_text = Y.one('.banner-text').get('text');
@@ -93,10 +93,10 @@
         test_legacy_select_public_info_type: function () {
             window.LP.cache.bug_private_by_default = true;
             Y.lp.bugs.filebug.setup_filebug(true);
-            Y.one('[id=field.information_type.2]').simulate('click');
+            Y.one('[id="field.information_type.2"]').simulate('click');
             var banner_hidden = Y.one('.yui3-privacybanner-hidden');
             Y.Assert.isNull(banner_hidden);
-            Y.one('[id=field.information_type.0]').simulate('click');
+            Y.one('[id="field.information_type.0"]').simulate('click');
             banner_hidden = Y.one('.yui3-privacybanner-hidden');
             Y.Assert.isNotNull(banner_hidden);
         },
@@ -131,7 +131,7 @@
             Y.Assert.areEqual('New', status_node.get('text'));
             var status_edit_node = Y.one('#status-content a.sprite.edit');
             Y.Assert.isNotNull(status_edit_node);
-            var legacy_dropdown = Y.one('[id=field.status]');
+            var legacy_dropdown = Y.one('[id="field.status"]');
             Y.Assert.isTrue(legacy_dropdown.hasClass('unseen'));
         },
 
@@ -143,7 +143,7 @@
             var importance_edit_node =
                 Y.one('#importance-content a.sprite.edit');
             Y.Assert.isNotNull(importance_edit_node);
-            var legacy_dropdown = Y.one('[id=field.importance]');
+            var legacy_dropdown = Y.one('[id="field.importance"]');
             Y.Assert.isTrue(legacy_dropdown.hasClass('unseen'));
         },
 
@@ -151,7 +151,7 @@
         // This is so fields which the user does not have permission to see
         // can be missing and everything still works as expected.
         test_missing_fields: function () {
-            Y.one('[id=field.status]').remove(true);
+            Y.one('[id="field.status"]').remove(true);
             this.test_importance_setup();
         },
 
@@ -161,9 +161,9 @@
             var status_popup = Y.one('#status-content a');
             status_popup.simulate('click');
             var status_choice = Y.one(
-                '.yui3-ichoicelist-content a[href=#Incomplete]');
+                '.yui3-ichoicelist-content a[href="#Incomplete"]');
             status_choice.simulate('click');
-            var legacy_dropdown = Y.one('[id=field.status]');
+            var legacy_dropdown = Y.one('[id="field.status"]');
             Y.Assert.areEqual('Incomplete', legacy_dropdown.get('value'));
         },
 
@@ -173,9 +173,9 @@
             var status_popup = Y.one('#importance-content a');
             status_popup.simulate('click');
             var status_choice = Y.one(
-                '.yui3-ichoicelist-content a[href=#High]');
+                '.yui3-ichoicelist-content a[href="#High"]');
             status_choice.simulate('click');
-            var legacy_dropdown = Y.one('[id=field.importance]');
+            var legacy_dropdown = Y.one('[id="field.importance"]');
             Y.Assert.areEqual('High', legacy_dropdown.get('value'));
         },
 
@@ -198,9 +198,9 @@
             var information_type_popup = Y.one('#information_type-content a');
             information_type_popup.simulate('click');
             var information_type_choice = Y.one(
-                '.yui3-ichoicelist-content a[href=#USERDATA]');
+                '.yui3-ichoicelist-content a[href="#USERDATA"]');
             information_type_choice.simulate('click');
-            var legacy_radio_button = Y.one('[id=field.information_type.3]');
+            var legacy_radio_button = Y.one('[id="field.information_type.3"]');
             Y.Assert.isTrue(legacy_radio_button.get('checked'));
         },
 
@@ -213,7 +213,7 @@
             var information_type_popup = Y.one('#information_type-content a');
             information_type_popup.simulate('click');
             var information_type_choice = Y.one(
-                '.yui3-ichoicelist-content a[href=#USERDATA]');
+                '.yui3-ichoicelist-content a[href="#USERDATA"]');
             information_type_choice.simulate('click');
             banner_hidden = Y.one('.yui3-privacybanner-hidden');
             Y.Assert.isNull(banner_hidden);
@@ -231,7 +231,7 @@
             var information_type_popup = Y.one('#information_type-content a');
             information_type_popup.simulate('click');
             var information_type_choice = Y.one(
-                '.yui3-ichoicelist-content a[href=#USERDATA]');
+                '.yui3-ichoicelist-content a[href="#USERDATA"]');
             information_type_choice.simulate('click');
             banner_hidden = Y.one('.yui3-privacybanner-hidden');
             Y.Assert.isNull(banner_hidden);
@@ -248,13 +248,13 @@
             var information_type_popup = Y.one('#information_type-content a');
             information_type_popup.simulate('click');
             var information_type_choice = Y.one(
-                '.yui3-ichoicelist-content a[href=#USERDATA]');
+                '.yui3-ichoicelist-content a[href="#USERDATA"]');
             information_type_choice.simulate('click');
             var banner_hidden = Y.one('.yui3-privacybanner-hidden');
             Y.Assert.isNull(banner_hidden);
             information_type_popup.simulate('click');
             information_type_choice = Y.one(
-                '.yui3-ichoicelist-content a[href=#PUBLIC]');
+                '.yui3-ichoicelist-content a[href="#PUBLIC"]');
             information_type_choice.simulate('click');
             banner_hidden = Y.one('.yui3-privacybanner-hidden');
             Y.Assert.isNotNull(banner_hidden);

=== modified file 'lib/lp/bugs/javascript/tests/test_information_type_choice.js'
--- lib/lp/bugs/javascript/tests/test_information_type_choice.js	2012-06-20 05:25:44 +0000
+++ lib/lp/bugs/javascript/tests/test_information_type_choice.js	2012-06-27 17:41:22 +0000
@@ -126,7 +126,7 @@
             var privacy_link = Y.one('#privacy-link');
             privacy_link.simulate('click');
             var private_choice = Y.one(
-                '.yui3-ichoicelist-content a[href=#Private]');
+                '.yui3-ichoicelist-content a[href="#Private"]');
             var orig_save_information_type = ns.save_information_type;
             var function_called = false;
             ns.save_information_type = function(widget, value, lp_client) {

=== modified file 'standard_test_template.html'
--- standard_test_template.html	2012-03-14 04:41:36 +0000
+++ standard_test_template.html	2012-06-27 17:41:22 +0000
@@ -21,6 +21,10 @@
 
       <script type="text/javascript"
               src="../../../../../../build/js/lp/app/testing/testrunner.js"></script>
+      <script type="text/javascript"
+              src="../../../../../build/js/lp/app/testing/helpers.js"></script>
+
+
 
       <link rel="stylesheet" href="../../../../app/javascript/testing/test.css" />
 

=== modified file 'standard_test_template.js'
--- standard_test_template.js	2012-04-06 17:28:25 +0000
+++ standard_test_template.js	2012-06-27 17:41:22 +0000
@@ -18,4 +18,4 @@
 
     }));
 
-}, '0.1', {'requires': ['test', 'console', 'lp.${LIBRARY}']});
+}, '0.1', {'requires': ['test', 'lp.testing.helpers', 'console', 'lp.${LIBRARY}']});


Follow ups