← Back to team overview

launchpad-reviewers team mailing list archive

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

 

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

= Summary =

This branch updates the lp-names module to pass tests in YUI 3.5. It also brings it under the standard test runner.

== Implementation Notes ==

The test failures were due to changes in the YUI test module. The Error() objects used in the _should setup needs to just be a string.

This also updates it to use the shared testrunner so that it doesn't need the same fixes that the testrunner already has. That's the bulk of the LoC diff as it moves the tests under the tests. object.

== Tests ==

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

== Lint ==

Linting changed files:
  lib/lp/app/javascript/tests/test_lp_names.js

== LoC Qualification ==
[Edit: This is actually a negative LoC change]

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/lpnames_yui35/+merge/112347
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/lpnames_yui35 into lp:launchpad.
=== modified file 'lib/lp/app/javascript/tests/test_lp_names.html'
--- lib/lp/app/javascript/tests/test_lp_names.html	2012-03-14 04:41:36 +0000
+++ lib/lp/app/javascript/tests/test_lp_names.html	2012-06-27 13:10:25 +0000
@@ -40,7 +40,7 @@
     <body class="yui3-skin-sam">
         <ul id="suites">
             <!-- <li>lp.large_indicator.test</li> -->
-            <li>lp.lp-names.test</li>
+            <li>lp.names.test</li>
         </ul>
         <div id="container-of-stuff">
         </div>

=== modified file 'lib/lp/app/javascript/tests/test_lp_names.js'
--- lib/lp/app/javascript/tests/test_lp_names.js	2011-06-21 05:37:38 +0000
+++ lib/lp/app/javascript/tests/test_lp_names.js	2012-06-27 13:10:25 +0000
@@ -1,250 +1,233 @@
-YUI({
-    base: '../../../../canonical/launchpad/icing/yui/',
-    filter: 'raw', combine: false, fetchCSS: false
-}).use('test', 'console', 'lp.names',
-       function(Y) {
-
-var suite = new Y.Test.Suite("lp.names Tests");
-var names = Y.lp.names;
-
-/**
- * Test conversion of Launchpad names to CSS classes.
- */
-suite.add(new Y.Test.Case({
-    name: 'lp.names.launchpad_to_css() test',
-
-    _should: {
-        error: {
-            test_not_lp_name_error:
-            new Error('Passed value "~" is not a valid Launchpad name.'),
-            test_start_with_plus_error:
-            new Error('Passed value "+name" is not a valid Launchpad name.'),
-            test_start_with_dot_error:
-            new Error('Passed value ".name" is not a valid Launchpad name.'),
-            test_start_with_dash_error:
-            new 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('~');
-    },
-
-    test_start_with_plus_error: function() {
-        // Strings starting with plus character are
-        // invalid LP names and throw an exception.
-        names.launchpad_to_css('+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');
-    },
-
-    test_start_with_dash_error: function() {
-        // Strings starting with dash character are
-        // invalid LP names and throw an exception.
-        names.launchpad_to_css('-name');
-    },
-
-    test_valid_in_both: function() {
-        // If a name is both a valid LP name and CSS class name,
-        // it is returned unmodified.
-        var name = 'name123-today';
-        var expected = name;
-        Y.Assert.areEqual(expected,
-                          names.launchpad_to_css(name));
-    },
-
-    test_starts_with_digits: function() {
-        var name = '2name';
-        var expected = '_2name';
-        Y.Assert.areEqual(expected,
-                          names.launchpad_to_css(name));
-    },
-
-    test_middle_digits: function() {
-        // Digits in the middle and end of string are not touched.
-        var name = 'na2me4';
-        var expected = name;
-        Y.Assert.areEqual(expected,
-                          names.launchpad_to_css(name));
-    },
-
-    test_plus_sign: function() {
-        // Plus sign is allowed in the Launchpad name, but
-        // not in the CSS class name.  It is replaced with '_y'.
-        var name = 'name+lastname';
-        var expected = 'name_ylastname';
-        Y.Assert.areEqual(expected,
-                          names.launchpad_to_css(name));
-    },
-
-    test_multiple_pluses: function() {
-        // Even multiple plus characters are replaced with '_y'.
-        var name = 'name+middle+lastname';
-        var expected = 'name_ymiddle_ylastname';
-        Y.Assert.areEqual(expected,
-                          names.launchpad_to_css(name));
-    },
-
-    test_dot_sign: function() {
-        // Dot sign is allowed in the Launchpad name, but
-        // not in the CSS class name.  It is replaced with '_z'.
-        var name = 'name.lastname';
-        var expected = 'name_zlastname';
-        Y.Assert.areEqual(expected,
-                          names.launchpad_to_css(name));
-    },
-
-    test_multiple_dots: function() {
-        // Even multiple dot characters are replaced with '_z'.
-        var name = 'name.middle.lastname';
-        var expected = 'name_zmiddle_zlastname';
-        Y.Assert.areEqual(expected,
-                          names.launchpad_to_css(name));
-    }
-}));
-
-/**
- * Test conversion of CSS class names as gotten by launchpad_to_css back
- * to Launchpad names.
- */
-suite.add(new Y.Test.Case({
-    name: 'lp.names.css_to_launchpad() test',
-
-    _should: {
-        error: {
-            test_not_css_class_error:
-            new Error('Passed value "+" is not a valid CSS class name.'),
-            test_start_with_digit_error:
-            new Error('Passed value "1name" is not a valid CSS class name.'),
-            test_non_lp_converted_name_error:
-            new Error(
-                'Passed value "_name" is not produced by launchpad_to_css.'),
-            test_non_lp_converted_name_error2:
-            new Error(
-                '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('+');
-    },
-
-    test_start_with_digit_error: function() {
-        // Strings starting with underscore are not valid CSS class names.
-        names.css_to_launchpad('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');
-    },
-
-    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');
-    },
-
-    test_valid_in_both: function() {
-        // If a name is both a valid LP name and CSS class name,
-        // it is returned unmodified.
-        var name = 'name123-today';
-        var expected = name;
-        Y.Assert.areEqual(expected,
-                          names.css_to_launchpad(name));
-    },
-
-    test_starts_with_digits: function() {
-        var name = '_2name';
-        var expected = '2name';
-        Y.Assert.areEqual(expected,
-                          names.css_to_launchpad(name));
-    },
-
-    test_middle_digits: function() {
-        // Digits in the middle and end of string are not touched.
-        var name = 'na2me4';
-        var expected = name;
-        Y.Assert.areEqual(expected,
-                          names.css_to_launchpad(name));
-    },
-
-    test_plus_sign: function() {
-        // Plus sign is represented as '_y' in the CSS class name.
-        var name = 'name_ylastname';
-        var expected = 'name+lastname';
-        Y.Assert.areEqual(expected,
-                          names.css_to_launchpad(name));
-    },
-
-    test_multiple_pluses: function() {
-        // Even multiple plus characters ('_y' strings) are handled.
-        var name = 'name_ymiddle_ylastname';
-        var expected = 'name+middle+lastname';
-        Y.Assert.areEqual(expected,
-                          names.css_to_launchpad(name));
-    },
-
-    test_dot_sign: function() {
-        // Dot sign is represented as '_z' in the CSS class name.
-        var name = 'name_zlastname';
-        var expected = 'name.lastname';
-        Y.Assert.areEqual(expected,
-                          names.css_to_launchpad(name));
-    },
-
-    test_multiple_dots: function() {
-        // Even multiple dot characters ('_z' strings) are handled.
-        var name = 'name_zmiddle_zlastname';
-        var expected = 'name.middle.lastname';
-        Y.Assert.areEqual(expected,
-                          names.css_to_launchpad(name));
-    }
-}));
-
-/**
- * Test idempotency of css_to_launchpad(launchpad_to_css()).
- */
-suite.add(new Y.Test.Case({
-    name: 'Combined idempotency css_to_launchpad(launchpad_to_css()) test',
-
-    test_simple_name: function() {
-        var name = 'name';
-        Y.Assert.areEqual(
-            name,
-            names.css_to_launchpad(names.launchpad_to_css(name)));
-    },
-
-    test_complex_name: function() {
-        var name = '0+name.lastname-44-';
-        Y.Assert.areEqual(
-            name,
-            names.css_to_launchpad(names.launchpad_to_css(name)));
-    }
-}));
-
-
-var handle_complete = function(data) {
-    window.status = '::::' + JSON.stringify(data);
-    };
-Y.Test.Runner.on('complete', handle_complete);
-Y.Test.Runner.add(suite);
-
-var console = new Y.Console({newestOnTop: false});
-console.render('#log');
-
-Y.on('domready', function() {
-    Y.Test.Runner.run();
-});
-});
+/* Copyright (c) 2012, Canonical Ltd. All rights reserved. */
+YUI.add('lp.names.test', function (Y) {
+    var names = Y.lp.names;
+
+    var tests = Y.namespace('lp.names.test');
+    tests.suite = new Y.Test.Suite('LP Name Tests');
+
+    /**
+     * Test conversion of Launchpad names to CSS classes.
+     */
+    tests.suite.add(new Y.Test.Case({
+        name: 'lp.names.launchpad_to_css() test',
+
+        _should: {
+            error: {
+                test_not_lp_name_error:
+                    new Error('"~" is not a valid Launchpad name.'),
+                test_start_with_plus_error:
+                    new Error('"+name" is nt a valid Launchpad name.'),
+                test_start_with_dot_error:
+                    new Error('".name" is not a valid Launchpad name.'),
+                test_start_with_dash_error:
+                    new Error('"-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('~');
+        },
+
+        test_start_with_plus_error: function() {
+            // Strings starting with plus character are
+            // invalid LP names and throw an exception.
+            names.launchpad_to_css('+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');
+        },
+
+        test_start_with_dash_error: function() {
+            // Strings starting with dash character are
+            // invalid LP names and throw an exception.
+            names.launchpad_to_css('-name');
+        },
+
+        test_valid_in_both: function() {
+            // If a name is both a valid LP name and CSS class name,
+            // it is returned unmodified.
+            var name = 'name123-today';
+            var expected = name;
+            Y.Assert.areEqual(expected,
+                              names.launchpad_to_css(name));
+        },
+
+        test_starts_with_digits: function() {
+            var name = '2name';
+            var expected = '_2name';
+            Y.Assert.areEqual(expected,
+                              names.launchpad_to_css(name));
+        },
+
+        test_middle_digits: function() {
+            // Digits in the middle and end of string are not touched.
+            var name = 'na2me4';
+            var expected = name;
+            Y.Assert.areEqual(expected,
+                              names.launchpad_to_css(name));
+        },
+
+        test_plus_sign: function() {
+            // Plus sign is allowed in the Launchpad name, but
+            // not in the CSS class name.  It is replaced with '_y'.
+            var name = 'name+lastname';
+            var expected = 'name_ylastname';
+            Y.Assert.areEqual(expected,
+                              names.launchpad_to_css(name));
+        },
+
+        test_multiple_pluses: function() {
+            // Even multiple plus characters are replaced with '_y'.
+            var name = 'name+middle+lastname';
+            var expected = 'name_ymiddle_ylastname';
+            Y.Assert.areEqual(expected,
+                              names.launchpad_to_css(name));
+        },
+
+        test_dot_sign: function() {
+            // Dot sign is allowed in the Launchpad name, but
+            // not in the CSS class name.  It is replaced with '_z'.
+            var name = 'name.lastname';
+            var expected = 'name_zlastname';
+            Y.Assert.areEqual(expected,
+                              names.launchpad_to_css(name));
+        },
+
+        test_multiple_dots: function() {
+            // Even multiple dot characters are replaced with '_z'.
+            var name = 'name.middle.lastname';
+            var expected = 'name_zmiddle_zlastname';
+            Y.Assert.areEqual(expected,
+                              names.launchpad_to_css(name));
+        }
+    }));
+
+    /**
+     * Test conversion of CSS class names as gotten by launchpad_to_css back
+     * to Launchpad names.
+     */
+    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('+');
+        },
+
+        test_start_with_digit_error: function() {
+            // Strings starting with underscore are not valid CSS class names.
+            names.css_to_launchpad('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');
+        },
+
+        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');
+        },
+
+        test_valid_in_both: function() {
+            // If a name is both a valid LP name and CSS class name,
+            // it is returned unmodified.
+            var name = 'name123-today';
+            var expected = name;
+            Y.Assert.areEqual(expected,
+                              names.css_to_launchpad(name));
+        },
+
+        test_starts_with_digits: function() {
+            var name = '_2name';
+            var expected = '2name';
+            Y.Assert.areEqual(expected,
+                              names.css_to_launchpad(name));
+        },
+
+        test_middle_digits: function() {
+            // Digits in the middle and end of string are not touched.
+            var name = 'na2me4';
+            var expected = name;
+            Y.Assert.areEqual(expected,
+                              names.css_to_launchpad(name));
+        },
+
+        test_plus_sign: function() {
+            // Plus sign is represented as '_y' in the CSS class name.
+            var name = 'name_ylastname';
+            var expected = 'name+lastname';
+            Y.Assert.areEqual(expected,
+                              names.css_to_launchpad(name));
+        },
+
+        test_multiple_pluses: function() {
+            // Even multiple plus characters ('_y' strings) are handled.
+            var name = 'name_ymiddle_ylastname';
+            var expected = 'name+middle+lastname';
+            Y.Assert.areEqual(expected,
+                              names.css_to_launchpad(name));
+        },
+
+        test_dot_sign: function() {
+            // Dot sign is represented as '_z' in the CSS class name.
+            var name = 'name_zlastname';
+            var expected = 'name.lastname';
+            Y.Assert.areEqual(expected,
+                              names.css_to_launchpad(name));
+        },
+
+        test_multiple_dots: function() {
+            // Even multiple dot characters ('_z' strings) are handled.
+            var name = 'name_zmiddle_zlastname';
+            var expected = 'name.middle.lastname';
+            Y.Assert.areEqual(expected,
+                              names.css_to_launchpad(name));
+        }
+    }));
+
+    /**
+     * Test idempotency of css_to_launchpad(launchpad_to_css()).
+     */
+    tests.suite.add(new Y.Test.Case({
+        name: 'Combined idempotency css_to_launchpad(launchpad_to_css()) test',
+
+        test_simple_name: function() {
+            var name = 'name';
+            Y.Assert.areEqual(
+                name,
+                names.css_to_launchpad(names.launchpad_to_css(name)));
+        },
+
+        test_complex_name: function() {
+            var name = '0+name.lastname-44-';
+            Y.Assert.areEqual(
+                name,
+                names.css_to_launchpad(names.launchpad_to_css(name)));
+        }
+    }));
+
+}, '0.1', {'requires': ['test', 'console', 'lp.names']});


Follow ups