Merge lp:~allenap/convoy/python3-clean-up into lp:convoy

[Gavin Panella]

Tidy up after the Python 3 porting effort.

[Fabrice Matrat (fabricematrat)]

LGTM

review: Approve

=== modified file 'convoy/combo.py'
--- convoy/combo.py	2015-11-11 19:53:19 +0000
+++ convoy/combo.py	2015-11-11 19:53:19 +0000
@@ -15,13 +15,17 @@
 #   along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 
-import cgi
 import logging
 import os.path
 import re
 import sys
 
 try:
+    from urllib.parse import parse_qsl
+except ImportError:
+    from cgi import parse_qsl
+
+try:
     import urllib.parse as urlparse
 except ImportError:
     import urlparse
@@ -56,7 +60,7 @@
 
     Returns the list of arguments in the original order.
     """
-    params = cgi.parse_qsl(query, keep_blank_values=True)
+    params = parse_qsl(query, keep_blank_values=True)
     return tuple([param for param, value in params])
 
 
@@ -124,12 +128,13 @@
                         parts = relative_parts + url.split(b"/")
                         result = []
                         for part in parts:
-                            if part == b".." and result and result[-1] != b"..":
+                            if part == b".." and result[-1:] != [b".."]:
                                 result.pop(-1)
                                 continue
                             result.append(part)
                         return b"url(x)".replace(b"x", b"/".join(
-                            part for part in [resource_prefix] + result if part))
+                            part for part in [resource_prefix] + result
+                            if part))
 
                     file_content = URL_RE.sub(fix_relative_url, file_content)
                     yield file_content
@@ -143,7 +148,8 @@
                         yield chunk
 
 
-def combo_app(root, resource_prefix="", rewrite_urls=True, additional_headers=None):
+def combo_app(
+        root, resource_prefix="", rewrite_urls=True, additional_headers=None):
     """A simple YUI Combo Service WSGI app.
 
     Serves any files under C{root}, setting an appropriate
 
=== modified file 'convoy/meta.py'
--- convoy/meta.py	2015-11-11 19:53:19 +0000
+++ convoy/meta.py	2015-11-11 19:53:19 +0000
@@ -170,11 +170,10 @@
                 return match.group(1) + literals_map[literal] + match.group(3)
             return match.group(0)
 
-
         linebreak = ",\n  "
         variables_decl = "var SKIN_SAM_PREFIX = 'skin-sam-'" + linebreak
         if self.prefix:
-             variables_decl += "PREFIX = '%s'%s" % (self.prefix, linebreak)
+            variables_decl += "PREFIX = '%s'%s" % (self.prefix, linebreak)
         extra_variables = []
         for literal, variable in sorted(literals_map.items()):
             extra_variable = "%s = %s" % (
@@ -220,8 +219,8 @@
                     # It's easy to think that doing 'CORE_CSS +  %(values)s'
                     # instead of using concat would work, but it doesn't;
                     # you'll end up with a string instead of a list.
-                    module_decl.append("after_list = CORE_CSS");
-                    module_decl.append("after_list.concat(%s)" % value);
+                    module_decl.append("after_list = CORE_CSS")
+                    module_decl.append("after_list.concat(%s)" % value)
                     value = "after_list"
                 if key == "path":
                     value = value.replace(
@@ -369,15 +368,15 @@
 
 
 def main():
-   options, args = get_options()
-   if options.src_dir is None:
-       options.src_dir = os.getcwd()
-   Builder(
-       name=options.name,
-       src_dir=os.path.abspath(options.src_dir),
-       output=options.output,
-       prefix=options.prefix,
-       exclude_regex=options.exclude_regex,
-       ext=options.ext,
-       include_skin=not options.no_skin,
-       ).do_build()
+    options, args = get_options()
+    if options.src_dir is None:
+        options.src_dir = os.getcwd()
+    Builder(
+        name=options.name,
+        src_dir=os.path.abspath(options.src_dir),
+        output=options.output,
+        prefix=options.prefix,
+        exclude_regex=options.exclude_regex,
+        ext=options.ext,
+        include_skin=not options.no_skin,
+    ).do_build()
 
=== modified file 'convoy/tests/test_combo.py'
--- convoy/tests/test_combo.py	2015-11-11 19:53:19 +0000
+++ convoy/tests/test_combo.py	2015-11-11 19:53:19 +0000
@@ -88,10 +88,11 @@
                               "/* event-custom/event-custom-min.js */",
                               "** event-custom-min **"))
         self.assertEqual(
-            b"".join(combine_files(["yui/yui-min.js",
-                                   "oop/oop-min.js",
-                                   "event-custom/event-custom-min.js"],
-                                  root=test_dir)).strip(),
+            b"".join(combine_files(
+                ["yui/yui-min.js",
+                 "oop/oop-min.js",
+                 "event-custom/event-custom-min.js"],
+                root=test_dir)).strip(),
             expected.encode("ascii"))
 
     def test_combine_files_yields_only_byte_strings(self):
@@ -144,9 +145,10 @@
             }
             """
         self.assertTextEquals(
-            b"".join(combine_files(["widget/assets/skins/sam/widget.css",
-                                   "editor/assets/skins/sam/editor.css"],
-                                  root=test_dir)).strip(),
+            b"".join(combine_files(
+                ["widget/assets/skins/sam/widget.css",
+                 "editor/assets/skins/sam/editor.css"],
+                root=test_dir)).strip(),
             expected)
 
     def test_combine_css_leaves_absolute_urls_untouched(self):
@@ -189,9 +191,10 @@
             }
             """
         self.assertTextEquals(
-            b"".join(combine_files(["widget/assets/skins/sam/widget.css",
-                                   "editor/assets/skins/sam/editor.css"],
-                                  root=test_dir)).strip(),
+            b"".join(combine_files(
+                ["widget/assets/skins/sam/widget.css",
+                 "editor/assets/skins/sam/editor.css"],
+                root=test_dir)).strip(),
             expected)
 
     def test_combine_css_leaves_data_uris_untouched(self):
@@ -234,9 +237,10 @@
             }
             """
         self.assertTextEquals(
-            b"".join(combine_files(["widget/assets/skins/sam/widget.css",
-                                   "editor/assets/skins/sam/editor.css"],
-                                  root=test_dir)).strip(),
+            b"".join(combine_files(
+                ["widget/assets/skins/sam/widget.css",
+                 "editor/assets/skins/sam/editor.css"],
+                root=test_dir)).strip(),
             expected)
 
     def test_no_parent_hack(self):
@@ -336,10 +340,10 @@
             }
             """
         self.assertTextEquals(
-            b"".join(combine_files(["widget/assets/skins/sam/widget.css",
-                                   "editor/assets/skins/sam/editor.css"],
-                                  root=test_dir,
-                                  resource_prefix="/static/")).strip(),
+            b"".join(combine_files(
+                ["widget/assets/skins/sam/widget.css",
+                 "editor/assets/skins/sam/editor.css"],
+                root=test_dir, resource_prefix="/static/")).strip(),
             expected)
 
     def test_combine_css_adds_custom_prefix_minified(self):
@@ -367,20 +371,18 @@
     def test_rewrite_url_normalizes_parent_references(self):
         """URL references in CSS files get normalized for parent dirs."""
         test_dir = self.makeDir()
-        files = [
-            self.makeSampleFile(
-                test_dir,
-                os.path.join("yui", "base", "base.css"),
-                ".foo{background-image:url(../../img.png)}"),
-            ]
+        self.makeSampleFile(
+            test_dir, os.path.join("yui", "base", "base.css"),
+            ".foo{background-image:url(../../img.png)}"),
 
         expected = """
             /* yui/base/base.css */
             .foo{background-image:url(img.png)}
             """
         self.assertTextEquals(
-            b"".join(combine_files(["yui/base/base.css"],
-                                  root=test_dir)).strip(),
+            b"".join(combine_files(
+                ["yui/base/base.css"],
+                root=test_dir)).strip(),
             expected)
 
 
@@ -437,8 +439,10 @@
             ["yui/yui-min.js",
              "oop/oop-min.js",
              "event-custom/event-custom-min.js"]), status=200)
-        self.assertEqual(res.headers, [("Content-Type", "text/javascript"),
-                                        ("X-Content-Type-Options", "nosniff")])
+        self.assertEqual(res.headers, [
+            ("Content-Type", "text/javascript"),
+            ("X-Content-Type-Options", "nosniff"),
+        ])
         self.assertEqual(res.body.strip(), expected.encode("ascii"))
 
     def test_combo_app_sets_content_type_for_css(self):
@@ -452,8 +456,10 @@
 
         res = self.app.get("/?" + "&".join(
             ["widget/skin/sam/widget.css"]), status=200)
-        self.assertEqual(res.headers, [("Content-Type", "text/css"),
-                                        ("X-Content-Type-Options", "nosniff")])
+        self.assertEqual(res.headers, [
+            ("Content-Type", "text/css"),
+            ("X-Content-Type-Options", "nosniff"),
+        ])
         self.assertEqual(res.body.strip(), expected.encode("ascii"))
 
     def test_no_filename_gives_404(self):
@@ -467,12 +473,16 @@
         Content-Type and X-Content-Type-Options headers set for
         non-existent files.
         """
-        res = self.app.get("/?" + "&".join(
-                ["foo/bar/baz",
-                 "<html><script>alert(document.domain)</script></html>"]),
-                           status=400)
-        self.assertEqual(res.headers, [("Content-Type", "text/plain"),
-                                        ("X-Content-Type-Options", "nosniff")])
+        res = self.app.get(
+            "/?" + "&".join([
+                "foo/bar/baz",
+                "<html><script>alert(document.domain)</script></html>",
+            ]),
+            status=400)
+        self.assertEqual(res.headers, [
+            ("Content-Type", "text/plain"),
+            ("X-Content-Type-Options", "nosniff"),
+        ])
 
     def test_js_comment_escape_hack(self):
         """Attacks to break out of the JS comment will get a 400 error."""
@@ -528,9 +538,14 @@
         app.get("/../etc?yui-min&passwd", status=400)
 
     def test_cache_headers_set(self):
-        app = TestApp(combo_app(self.root, additional_headers=[('Cache-Control', 'max-age=3600, public')]))
+        app = TestApp(combo_app(
+            self.root, additional_headers=[
+                ('Cache-Control', 'max-age=3600, public'),
+            ]))
         res = app.get("/?" + "&".join(
             ["widget/skin/sam/widget.css"]), status=400)
-        self.assertEqual(res.headers, [("Content-Type", "text/css"),
-                                        ("X-Content-Type-Options", "nosniff"),
-                                        ('Cache-Control', 'max-age=3600, public')])
+        self.assertEqual(res.headers, [
+            ("Content-Type", "text/css"),
+            ("X-Content-Type-Options", "nosniff"),
+            ('Cache-Control', 'max-age=3600, public'),
+        ])
 
=== modified file 'convoy/tests/test_meta.py'
--- convoy/tests/test_meta.py	2015-11-11 19:53:19 +0000
+++ convoy/tests/test_meta.py	2015-11-11 19:53:19 +0000
@@ -126,7 +126,8 @@
         metadata = extract_metadata("""\
         YUI.add('lazr.anim', function(Y){
            Y.log('Hello World');
-        }, '0.1', {"use": ["dom"], "requires": [ "node" ,"anim"  ,  "event" ]});
+        }, '0.1', {"use": ["dom"], "requires": [
+                        "node" ,"anim"  ,  "event" ]});
         """)
 
         self.assertEqual(len(metadata), 1)
@@ -141,7 +142,8 @@
         metadata = extract_metadata("""\
         YUI.add('lazr.anim', function(Y){
            Y.log('Hello World');
-        }, '0.1', {"requires": [ "node" ,"anim"  ,  "event" ], "use": ["dom"]});
+        }, '0.1', {"requires": [ "node" ,"anim"  ,  "event" ],
+                   "use": ["dom"]});
         """)
 
         self.assertEqual(len(metadata), 1)
@@ -177,7 +179,9 @@
         """
         metadata = extract_metadata("""
         YUI.add('test', function(Y) {
-            this._dds[h[i]] = new YAHOO.util.DragDrop(this._handles[h[i]], this.get('id') + '-handle-' + h, { useShim: this.get('useShim') });
+            this._dds[h[i]] = new YAHOO.util.DragDrop(
+              this._handles[h[i]], this.get('id') + '-handle-' + h, {
+                useShim: this.get('useShim') });
         }, '1.0', {requires: ['dom']});
         """)
 
@@ -205,7 +209,6 @@
         self.assertEqual(metadata[0]["requires"], ["node", "anim", "event"])
         self.assertEqual(metadata[0]["use"], ["dom"])
 
-
     def test_extract_has_no_quotes(self):
         """
         If the javascript is using object literals, we should adjust for
@@ -312,6 +315,7 @@
         self.assertEqual(metadata[0]["supersedes"], ["old-anim"])
         self.assertEqual(metadata[0]["after"], ["lazr.base"])
 
+
 class GenerateMetadataTest(ConvoyTestCase):
 
     def readFile(self, path):
 
=== modified file 'setup.py'
--- setup.py	2015-11-11 19:53:19 +0000
+++ setup.py	2015-11-11 19:53:19 +0000
@@ -1,5 +1,5 @@
 #   Convoy is a WSGI app for loading multiple files in the same request.
-#   Copyright (C) 2010-2015 Canonical, Ltd.
+#   Copyright (C) 2011-2015 Canonical, Ltd.
 #
 #   This program is free software: you can redistribute it and/or modify
 #   it under the terms of the GNU Affero General Public License as