#280 Add review suggestions

poleno/utils/misc.py

@@ -59,7 +59,7 @@ def random_readable_string(length, vowels=u'aeiouy', consonants=u'bcdfghjklmnprs
     [:vowel:]? ([:consonant:][:vowel:])* [:consonant:]?
 
     Where `[:vowel:]` is the set of all vowels `[aeiouy]` and `['consonant']` is the set of
-    consonants `[bcdfghjklmnprstvxz]`. User can use ``vowels`` and ``consonants`` arguments to set
+    consonants `[bcdfghjklmnprstvxz]`. You can use ``vowels`` and ``consonants`` arguments to set
     your own sets of vowels and consonants.
     """
     res = []
@@ -142,7 +142,7 @@ def flatten(l):
 
 def guess_extension(content_type, default=u'.bin'):
     u"""
-    Guesses file extention based on file content type. Wrapper around ``mimetypes.guess_extension``
+    Guesses file extension based on file content type. Wrapper around ``mimetypes.guess_extension``
     to return ``default`` extension if the given content type is not known by ``mimetypes`` module,
     and to fix stupid guesses like: "text/plain" -> ".ksh".
 
@@ -215,20 +215,20 @@ def filesize(size):
         0 -> "0 bytes"
         1023 -> "1023 bytes"
         49573834547 -> "46.2 GB"
-        -3847 -> "-3.8 kB"
+        -3847 -> "-3.7 kB"
     """
     if size is None:
         return None
 
     for fmt in [u'{:.0f} bytes', u'{:.1f} kB', u'{:.1f} MB', u'{:.1f} GB', u'{:.1f} TB']:
         if abs(size) < 1024.0:
-            return fmt.format(round(size + 0.05, 1))
+            return fmt.format(round(size + 0.04999, 1))
         size /= 1024.0
-    return u'{:.1f} PB'.format(round(size + 0.05, 1))
+    return u'{:.1f} PB'.format(round(size + 0.04999, 1))
 
 def parsefilesize(value):
     u"""
-    Parses files sizes formated with `filesize` function above.
+    Parses files sizes formatted with `filesize` function above.
 
     Example:
         "0 bytes" -> 0.0

poleno/utils/tests/test_forms.py

@@ -272,6 +272,8 @@ def _render(self, template, **context):
     def setUp(self):
         self.tempdir = TempDirectory()
         self.settings_override = override_settings(
+            TEMPLATE_LOADERS=((u'poleno.utils.template.TranslationLoader', u'django.template.loaders.filesystem.Loader'),
+                              (u'poleno.utils.template.TranslationLoader', u'django.template.loaders.app_directories.Loader')),
             TEMPLATE_DIRS=(self.tempdir.path,),
             )
         self.settings_override.enable()
@@ -281,6 +283,7 @@ def tearDown(self):
         self.settings_override.disable()
         self.tempdir.cleanup()
 
+
     def test_new_form(self):
         form = self.Form()
         rendered = self._render(u'{{ form }}', form=form)
@@ -414,11 +417,18 @@ def test_empty_string_is_invalid(self):
             validate_formatted_email(u'')
 
     def test_valid_values(self):
-        validate_formatted_email(u'[email protected]')
-        validate_formatted_email(u'John Smith <[email protected]>')
-        validate_formatted_email(u'"John \\"Agent\\" Smith" <[email protected]>')
-        validate_formatted_email(u'"Smith, John" <[email protected]>')
-        validate_formatted_email(u'"[email protected]" <[email protected]>')
+        validate_formatted_email(u'[email protected]') # // Parsed as: [email protected]
+        validate_formatted_email(u'John Smith <[email protected]>') # // Parsed as: John Smith [email protected]
+        validate_formatted_email(u'"John \\"Agent\\" Smith" <[email protected]>') # // Parsed as: John "Agent" Smith [email protected]
+        validate_formatted_email(u'"Smith, John" <[email protected]>') # // Parsed as: Smith, John [email protected]
+        validate_formatted_email(u'"[email protected]" <[email protected]>') # // Parsed as: [email protected] [email protected]
+        validate_formatted_email(u'<[email protected]>') # // Parsed as: [email protected]
+        validate_formatted_email(u'john [email protected]') # // Parsed as: [email protected]
+        validate_formatted_email(u'"John" <[email protected]>') # // Parsed as: John [email protected]
+        validate_formatted_email(u'"Smith, John" Agent <[email protected]>') # // Parsed as: Smith, John Agent [email protected]
+        validate_formatted_email(u'[email protected] <[email protected]>') # // Parsed as: [email protected]
+        validate_formatted_email(u'[email protected],') # // Parsed as: [email protected]
+        validate_formatted_email(u'John Smith <[email protected]>, [email protected]') # // Parsed as: John Smith [email protected]
 
     def test_invalid_values(self):
         with self.assertRaisesMessage(ValidationError, u'"invalid" is not a valid email address'):
@@ -441,11 +451,17 @@ def test_empty_string_is_valid(self):
         validate_comma_separated_emails(u'')
 
     def test_valid_values(self):
-        validate_comma_separated_emails(u'[email protected]')
-        validate_comma_separated_emails(u'John Smith <[email protected]>, [email protected]')
-        validate_comma_separated_emails(u'"John \\"Agent\\" Smith" <[email protected]>')
-        validate_comma_separated_emails(u'"Smith, John" <[email protected]>')
-        validate_comma_separated_emails(u'"[email protected]" <[email protected]>')
+        validate_comma_separated_emails(u'[email protected]') # // Parsed as [email protected]
+        validate_comma_separated_emails(u'John Smith <[email protected]>, [email protected]') # // Parsed as John Smith [email protected]; [email protected]
+        validate_comma_separated_emails(u'"John \\"Agent\\" Smith" <[email protected]>') # // Parsed as: John "Agent" Smith [email protected]
+        validate_comma_separated_emails(u'"Smith, John" <[email protected]>') # // Parsed as: Smith, John [email protected]
+        validate_comma_separated_emails(u'"[email protected]" <[email protected]>') # // Parsed as: [email protected] [email protected]
+        validate_comma_separated_emails(u'<[email protected]>') # // Parsed as: [email protected]
+        validate_comma_separated_emails(u'john [email protected]') # // Parsed as: [email protected]
+        validate_comma_separated_emails(u'"John" <[email protected]>') # // Parsed as: John [email protected]
+        validate_comma_separated_emails(u'"Smith, John" Agent <[email protected]>') # // Parsed as: Smith, John Agent [email protected]
+        validate_comma_separated_emails(u'[email protected] <[email protected]>') # // Parsed as: [email protected]; [email protected]
+        validate_comma_separated_emails(u'[email protected],') # // Parsed as: [email protected]
 
     def test_invalid_values(self):
         with self.assertRaisesMessage(ValidationError, u'"invalid" is not a valid email address'):

poleno/utils/tests/test_http.py

@@ -8,6 +8,7 @@
 from django.http import HttpResponseNotModified, FileResponse
 from django.utils.http import urlquote, urlencode, http_date
 from django.test import TestCase
+from django.test.utils import override_settings
 
 from poleno.utils.http import send_file_response
 from poleno.utils.misc import random_string
@@ -34,8 +35,13 @@ def file_view(request):
 
     def setUp(self):
         self.tempdir = TempDirectory()
+        self.settings_override = override_settings(
+            TEMPLATE_LOADERS=((u'poleno.utils.template.TranslationLoader', u'django.template.loaders.filesystem.Loader'),),
+        )
+        self.settings_override.enable()
 
     def tearDown(self):
+        self.settings_override.disable()
         self.tempdir.cleanup()
 
 
@@ -64,11 +70,11 @@ def test_regular_file(self):
         self._check_content(response, path)
 
     def test_directory_raises_exception(self):
-        with self.assertRaises(TypeError):
+        with self.assertRaisesMessage(OSError, u'Not a regular file: /'):
             response = self._request_file(u'/')
 
     def test_nonexistent_file_raises_exception(self):
-        with self.assertRaises(TypeError):
+        with self.assertRaisesMessage(OSError, u"[Errno 2] No such file or directory: '/nonexistent.txt'"):
             response = self._request_file(u'/nonexistent.txt')
 
     def test_random_file(self):

poleno/utils/tests/test_misc.py

@@ -190,7 +190,7 @@ def test_list_with_tuples(self):
 
     def test_list_with_strings(self):
         result = flatten([u'one', [u'two', u'three']])
-        self.assertEqual(list(result), ['one', 'two', 'three'])
+        self.assertEqual(list(result), [u'one', u'two', u'three'])
 
     def test_empty_list(self):
         result = flatten([])
@@ -213,7 +213,7 @@ class GuessExtensionTest(TestCase):
     Tests ``guess_extension()`` function.
     """
 
-    # Overriden guesses
+    # Overridden guesses
     def test_text_plain(self):
         self.assertEqual(guess_extension(u'text/plain'), u'.txt')
 
@@ -230,6 +230,9 @@ def test_unknown_content_type(self):
     def test_unknown_content_type_with_default_extension(self):
         self.assertEqual(guess_extension(u'application/nonexistent', u'.obj'), u'.obj')
 
+    def test_unknown_content_type_with_default_extension_None(self):
+        self.assertIsNone(guess_extension(u'application/nonexistent', None))
+
 class FileSize(TestCase):
     u"""
     Tests ``filesize()`` function.
@@ -240,17 +243,17 @@ def test_zero_bytes(self):
 
     def test_supported_sizes(self):
         self.assertEqual(filesize(1023), u'1023 bytes')
-        self.assertEqual(filesize(1024), u'1.1 kB')
+        self.assertEqual(filesize(1024), u'1.0 kB')
         self.assertEqual(filesize(2048), u'2.0 kB')
-        self.assertEqual(filesize(1024*1024), u'1.1 MB')
-        self.assertEqual(filesize(1024*1024*1024), u'1.1 GB')
-        self.assertEqual(filesize(1024*1024*1024*1024), u'1.1 TB')
-        self.assertEqual(filesize(1024*1024*1024*1024*1024), u'1.1 PB')
+        self.assertEqual(filesize(1024*1024), u'1.0 MB')
+        self.assertEqual(filesize(1024*1024*1024), u'1.0 GB')
+        self.assertEqual(filesize(1024*1024*1024*1024), u'1.0 TB')
+        self.assertEqual(filesize(1024*1024*1024*1024*1024), u'1.0 PB')
         self.assertEqual(filesize(49573834547), u'46.2 GB')
 
     def test_too_big_sizes(self):
         self.assertEqual(filesize(1024*1024*1024*1024*1024*1024), u'1024.0 PB')
-        self.assertEqual(filesize(1024*1024*1024*1024*1024*1024*1024), u'1048576.1 PB')
+        self.assertEqual(filesize(1024*1024*1024*1024*1024*1024*1024), u'1048576.0 PB')
 
     def test_random_sizes(self):
         self.assertEqual(filesize(3847), u'3.8 kB')

poleno/utils/tests/test_views.py

@@ -1,9 +1,8 @@
 # vim: expandtab
 # -*- coding: utf-8 -*-
-from django.http import HttpResponse, HttpResponseRedirect
+from django.http import HttpResponse, HttpResponseRedirect, HttpResponseBadRequest, HttpResponseForbidden
 from django.conf.urls import patterns, url
 from django.contrib.auth.models import User
-from django.core.urlresolvers import NoReverseMatch
 from django.test import TestCase
 from django.test.utils import override_settings
 
@@ -22,6 +21,15 @@ def require_ajax_view(request):
         url(r'^require-ajax/$', require_ajax_view),
     ))
 
+    def setUp(self):
+        self.settings_override = override_settings(
+            TEMPLATE_LOADERS=((u'poleno.utils.template.TranslationLoader', u'django.template.loaders.filesystem.Loader'),),
+        )
+        self.settings_override.enable()
+
+    def tearDown(self):
+        self.settings_override.disable()
+
 
     def test_with_ajax(self):
         u"""
@@ -35,8 +43,9 @@ def test_without_ajax(self):
         u"""
         Tests that ``@require_ajax`` forbids requests without ``XMLHttpRequest`` header.
         """
-        with self.assertRaises(NoReverseMatch):
-            response = self.client.get(u'/require-ajax/')
+        response = self.client.get(u'/require-ajax/')
+        self.assertIs(type(response), HttpResponseBadRequest)
+        self.assertEqual(response.status_code, 400)
 
 class LoginRequiredTest(TestCase):
     u"""
@@ -55,11 +64,11 @@ def login_required_with_exception_view(request):
         url(r'^login-required-with-exception/$', login_required_with_exception_view),
     ))
 
-
     def setUp(self):
         self.settings_override = override_settings(
             PASSWORD_HASHERS=(u'django.contrib.auth.hashers.MD5PasswordHasher',),
-            )
+            TEMPLATE_LOADERS=((u'poleno.utils.template.TranslationLoader', u'django.template.loaders.filesystem.Loader'),),
+        )
         self.settings_override.enable()
 
     def tearDown(self):
@@ -78,8 +87,9 @@ def test_anonymous_with_exception(self):
         u"""
         Tests that ``@login_required(raise_exception=True)`` forbids requests with anonymous users.
         """
-        with self.assertRaises(NoReverseMatch):
-            response = self.client.get(u'/login-required-with-exception/')
+        response = self.client.get(u'/login-required-with-exception/')
+        self.assertIs(type(response), HttpResponseForbidden)
+        self.assertEqual(response.status_code, 403)
 
     def test_authentificated_with_redirect(self):
         u"""
@@ -123,6 +133,15 @@ def secure_required_with_exception_view(request):
         url(r'^secure-required-with-exception/$', secure_required_with_exception_view),
     ))
 
+    def setUp(self):
+        self.settings_override = override_settings(
+            TEMPLATE_LOADERS=((u'poleno.utils.template.TranslationLoader', u'django.template.loaders.filesystem.Loader'),),
+        )
+        self.settings_override.enable()
+
+    def tearDown(self):
+        self.settings_override.disable()
+
 
     def test_insecure_with_redirect(self):
         u"""
@@ -137,8 +156,9 @@ def test_insecure_with_exception(self):
         u"""
         Tests that ``@secure_required(raise_exception=True)`` forbids insecure requests.
         """
-        with self.assertRaises(NoReverseMatch):
-            response = self.client.get(u'/secure-required-with-exception/')
+        response = self.client.get(u'/secure-required-with-exception/')
+        self.assertIs(type(response), HttpResponseForbidden)
+        self.assertEqual(response.status_code, 403)
 
     def test_secure_with_redirect(self):
         u"""