#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"""