Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add python 3.3 to python and allow_failures #148 #149

Closed
wants to merge 14 commits into from

3 participants

@graingert

No description provided.

@graingert

Don't merge this until unittest2 problems are solved

@graingert graingert remove dependency on unittest2
This allows support for both python 2.6 and 3.3
de34807
@graingert

The skip_if function was taken from the MIT licensed https://github.com/eventlet/eventlet/blob/master/tests/__init__.py

In my opinion not a "significant portion of the code"

@willkg
Owner

We're already using nose and we only need a skip_if that takes a boolean. Something like this is probably better:

from functools import wraps

from nose.plugins.skip import SkipTest


def skip_if(condition):
    """Skips tests if condition is True.
    """
    def skipped_wrapper(func):
        @wraps(func)
        def wrapped(*args, **kwargs):
            if condition:
                def skip_function(*args, **kwargs):
                    raise SkipTest()
                return skip_function
            return func(*args, **kwargs)
        return wrapped
    return skipped_wrapper

I haven't tested that, but the gist is there and it's in the style of the rest of django-browserid.

@willkg
Owner

This is awesome work! I'm really psyched about supporting python 3.3 and I'm glad someone else is doing it because I just haven't had time, yet.

@graingert

It looks like with @willkg's change only 45 tests are run even under django 1.5 (not 47 as expected)

It's also broken with my skip_if fix, looks like it breaks classes

django_browserid/tests/__init__.py
@@ -87,3 +88,18 @@ def inner(*args, **kwargs):
with self:
return func(*args, **kwargs)
return inner
+
+
+def skip_if(condition):
+ """Skips tests if condition is True.
+ """
+ def skipped_wrapper(func):
+ @wraps(func)
+ def wrapped(*args, **kwargs):
+ if condition:
+ def skip_function(*args, **kwargs):
+ raise SkipTest()
+ return skip_function
@willkg Owner
willkg added a note

This might need to be:

if condition:
    raise SkipTest()
return func(*args, **kwargs)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@graingert

Woo, now we support Python 3.3! (at least on paper)

@Osmose
Owner

This looks good to me, if @willkg agrees I'll merge this in. :D

@graingert Oh yeah, can you add 3.3 back in as an allowed failure? I want to have at least one release with it as an allowed failure before I make it mandatory.

@willkg
Owner

Super sorry about not seeing this until just now. I'll check it out tomorrow.

.travis.yml
((12 lines not shown))
install:
+ - pip install -e git+git://github.com/sobotklp/django-nose@b3f485e914965e0a7b7012c309864d6a6dcac3ed#egg=django-nose
@willkg Owner
willkg added a note

What's this line for? django-nose is already in requirements.txt.

That's because django-nose by default does not support py3k, I'll add this to requirements.txt instead

@willkg Owner
willkg added a note

Oh, hrm... It'd be better if we were using a released version of django-nose. Erik maintains django-nose. It's probably a good idea to push python 3 support in there. Having said that, I just remembered that "go through the django-nose python 3 support patches" is in my queue of things to do. :(

I don't think it should get added to requirements.txt since it's not something you need to have installed to use django-browserid. We should have a requirements-dev.txt file which has development-centric things in it.

@willkg Owner
willkg added a note

Ahh... ok. This is different than all the other projects I work on--but that's fine. Awesome!

I'll get to the django-nose stuff next week. Whenever that settles out, I'll do a PR to update django-browserid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
django_browserid/auth.py
@@ -29,7 +33,9 @@ def default_username_algo(email):
# this protects against data leakage because usernames are often
# treated as public identifiers (so we can't use the email address).
username = base64.urlsafe_b64encode(
- hashlib.sha1(email).digest()).rstrip('=')
+ hashlib.sha1(
+ smart_bytes(email)
+ ).digest()).rstrip(b'=')
@willkg Owner
willkg added a note

Nit: I think these three lines could all be on the same line. It'd be easier to read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
django_browserid/forms.py
@@ -19,7 +23,7 @@ class Media:
def clean_assertion(self):
try:
- return str(self.cleaned_data['assertion'])
+ return smart_bytes(self.cleaned_data['assertion'], encoding='ascii')
@willkg Owner
willkg added a note

Nit: This line is too long.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
django_browserid/tests/test_auth.py
@@ -105,34 +99,34 @@ def test_verify_called_with_browserid_extra(self, user_verify):
user_verify.assert_called_with('asdf', 'asdf', extra_params=dic)
+if get_user_model:
# Only run custom user model tests if we're using a version of Django that
# supports it.
@willkg Owner
willkg added a note

Nit: These two lines should either go before the if get_user_model: or be indented 4 spaces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
requirements.txt
@@ -5,7 +5,6 @@ fancy_tag==0.2.0
mock>=0.8.0
Django>=1.3
django-nose
-unittest2==0.5.1
@willkg Owner
willkg added a note

Yay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
setup.py
@@ -5,7 +5,7 @@
def read(*parts):
- return codecs.open(os.path.join(os.path.dirname(__file__), *parts)).read()
+ return codecs.open(os.path.join(os.path.dirname(__file__), *parts), encoding="utf8").read()
@willkg Owner
willkg added a note

Nit: We use single quotes instead of double-quotes to denote strings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
.travis.yml
@@ -6,7 +6,16 @@ env:
- DJANGO_VERSION=1.3.7
- DJANGO_VERSION=1.4.5
- DJANGO_VERSION=1.5
+matrix:
+ include:
+ - python: 3.2
+ env: DJANGO_VERSION=1.5
+ - python: 3.3
+ env: DJANGO_VERSION=1.5
@willkg Owner
willkg added a note

The 1.5 in this file should probably be 1.5.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
.travis.yml
((12 lines not shown))
install:
+ - pip install -e git+git://github.com/sobotklp/django-nose@b3f485e914965e0a7b7012c309864d6a6dcac3ed#egg=django-nose
- pip install -e git+git://github.com/django/django.git@${DJANGO_VERSION}#egg=django
@willkg Owner
willkg added a note

Instead of pulling from git, I think you can do:

pip install django=={DJANGO_VERSION}

Would that work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@willkg
Owner

I can't seem to get a functional python3.3 virtual environment set up for some reason.

I mentioned a few nits some of which should get fixed while others are subjective. Other than that, it looks ok to me.

@Osmose
Owner

Merged in ee8a1e3. Nice job, @graingert ! :+1:

@Osmose Osmose closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
13 .travis.yml
@@ -5,8 +5,17 @@ python:
env:
- DJANGO_VERSION=1.3.7
- DJANGO_VERSION=1.4.5
- - DJANGO_VERSION=1.5
+ - DJANGO_VERSION=1.5.1
+matrix:
+ include:
+ - python: 3.2
+ env: DJANGO_VERSION=1.5.1
+ - python: 3.3
+ env: DJANGO_VERSION=1.5.1
+ allow_failures:
+ - python: 3.2
install:
- - pip install -e git+git://github.com/django/django.git@${DJANGO_VERSION}#egg=django
+ - pip install -e git+git://github.com/sobotklp/django-nose@b3f485e914965e0a7b7012c309864d6a6dcac3ed#egg=django-nose
+ - pip install django==${DJANGO_VERSION}
- pip install -r requirements.txt --use-mirrors
script: python setup.py test
View
10 django_browserid/auth.py
@@ -8,6 +8,10 @@
from django.conf import settings
from django.core.exceptions import ImproperlyConfigured
from django.utils.importlib import import_module
+try:
+ from django.utils.encoding import smart_bytes
+except ImportError:
+ from django.utils.encoding import smart_str as smart_bytes
from django_browserid.base import verify
from django_browserid.signals import user_created
@@ -28,9 +32,9 @@ def default_username_algo(email):
# store the username as a base64 encoded sha1 of the email address
# this protects against data leakage because usernames are often
# treated as public identifiers (so we can't use the email address).
- username = base64.urlsafe_b64encode(
- hashlib.sha1(email).digest()).rstrip('=')
- return username
+ return base64.urlsafe_b64encode(
+ hashlib.sha1(smart_bytes(email)).digest()
+ ).rstrip(b'=')
class BrowserIDBackend(object):
View
9 django_browserid/forms.py
@@ -4,6 +4,10 @@
from django import forms
from django.conf import settings
+try:
+ from django.utils.encoding import smart_bytes
+except ImportError:
+ from django.utils.encoding import smart_str as smart_bytes
FORM_JAVASCRIPT = ('browserid/browserid.js',)
BROWSERID_SHIM = getattr(settings, 'BROWSERID_SHIM',
@@ -19,7 +23,10 @@ class Media:
def clean_assertion(self):
try:
- return str(self.cleaned_data['assertion'])
+ return smart_bytes(
+ self.cleaned_data['assertion'],
+ encoding='ascii'
+ )
except UnicodeEncodeError:
# not ascii :(
raise forms.ValidationError('non-ascii string')
View
3  django_browserid/helpers.py
@@ -12,6 +12,7 @@
FORM_JAVASCRIPT)
from django_browserid.util import static_url
+from six import string_types
# If funfactory is available, we want to use it's locale-aware reverse instead
# of Django's reverse, so we try to import funfactory's first and fallback to
@@ -61,7 +62,7 @@ def browserid_button(text=None, next=None, link_class=None, attrs=None,
href to use for the link.
"""
attrs = attrs or {}
- if isinstance(attrs, basestring):
+ if isinstance(attrs, string_types):
attrs = json.loads(attrs)
attrs.setdefault('class', link_class)
View
68 django_browserid/tests/test_auth.py
@@ -10,12 +10,6 @@
from django_browserid.auth import BrowserIDBackend, default_username_algo, verify
from django_browserid.tests import mock_browserid
-# Support Python 2.6 by using unittest2
-try:
- from unittest import skipIf
-except ImportError:
- from unittest2 import skipIf
-
try:
from django.contrib.auth import get_user_model
from django_browserid.tests.models import CustomUser
@@ -105,34 +99,34 @@ def test_verify_called_with_browserid_extra(self, user_verify):
user_verify.assert_called_with('asdf', 'asdf', extra_params=dic)
-# Only run custom user model tests if we're using a version of Django that
-# supports it.
-@patch.object(settings, 'AUTH_USER_MODEL', 'tests.CustomUser')
-@skipIf(not get_user_model, 'Not supported in Django < 1.5')
-class CustomUserModelTests(TestCase):
- def _auth(self, backend=None, verified_email=None):
- if backend is None:
- backend = BrowserIDBackend()
-
- with mock_browserid(verified_email):
- return backend.authenticate(assertion='asdf', audience='asdf')
-
- def test_existing_user(self):
- """If a custom user exists with the given email, return them."""
- user = CustomUser.objects.create(email='a@test.com')
- authed_user = self._auth(verified_email='a@test.com')
- self.assertEqual(user, authed_user)
-
- @patch.object(settings, 'BROWSERID_CREATE_USER', True)
- def test_create_new_user(self):
- """
- If a custom user does not exist with the given email, create a new
- user and return them.
- """
- class CustomUserBrowserIDBackend(BrowserIDBackend):
- def create_user(self, email):
- return CustomUser.objects.create(email=email)
- user = self._auth(backend=CustomUserBrowserIDBackend(),
- verified_email='b@test.com')
- self.assertTrue(isinstance(user, CustomUser))
- self.assertEqual(user.email, 'b@test.com')
+if get_user_model:
+ # Only run custom user model tests if we're using a version of Django that
+ # supports it.
+ @patch.object(settings, 'AUTH_USER_MODEL', 'tests.CustomUser')
+ class CustomUserModelTests(TestCase):
+ def _auth(self, backend=None, verified_email=None):
+ if backend is None:
+ backend = BrowserIDBackend()
+
+ with mock_browserid(verified_email):
+ return backend.authenticate(assertion='asdf', audience='asdf')
+
+ def test_existing_user(self):
+ """If a custom user exists with the given email, return them."""
+ user = CustomUser.objects.create(email='a@test.com')
+ authed_user = self._auth(verified_email='a@test.com')
+ self.assertEqual(user, authed_user)
+
+ @patch.object(settings, 'BROWSERID_CREATE_USER', True)
+ def test_create_new_user(self):
+ """
+ If a custom user does not exist with the given email, create a new
+ user and return them.
+ """
+ class CustomUserBrowserIDBackend(BrowserIDBackend):
+ def create_user(self, email):
+ return CustomUser.objects.create(email=email)
+ user = self._auth(backend=CustomUserBrowserIDBackend(),
+ verified_email='b@test.com')
+ self.assertTrue(isinstance(user, CustomUser))
+ self.assertEqual(user.email, 'b@test.com')
View
2  django_browserid/tests/test_forms.py
@@ -11,5 +11,5 @@ def test_invalid_assertion():
def test_valid_assertion():
- form = BrowserIDForm({'assertion': 'xxx'})
+ form = BrowserIDForm({'assertion': b'xxx'})
assert form.is_valid()
View
3  django_browserid/tests/test_views.py
@@ -9,6 +9,7 @@
from django_browserid import BrowserIDException, views
from django_browserid.tests import mock_browserid, patch_settings
+from six.moves import reload_module
factory = RequestFactory()
@@ -32,7 +33,7 @@ def verify(request_type, success_url=None, failure_url=None, **kwargs):
# We need to reload verify for the setting changes to take effect.
with patch_settings(**patches):
- reload(views)
+ reload_module(views)
verify_view = views.Verify.as_view()
with patch.object(auth, 'login'):
response = verify_view(request)
View
11 django_browserid/views.py
@@ -2,7 +2,14 @@
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
import logging
-import urlparse
+
+import six
+
+if six.PY3:
+ from urllib import parse as urllib_parse
+else:
+ import urlparse as urllib_parse
+
from django.conf import settings
from django.contrib import auth
@@ -56,7 +63,7 @@ def login_success(self):
# Do not accept redirect URLs pointing to a different host.
if redirect_to:
- netloc = urlparse.urlparse(redirect_to).netloc
+ netloc = urllib_parse.urlparse(redirect_to).netloc
if netloc and netloc != self.request.get_host():
redirect_to = None
View
3  requirements.txt
@@ -4,8 +4,7 @@ fancy_tag==0.2.0
# Tests
mock>=0.8.0
Django>=1.3
-django-nose
-unittest2==0.5.1
+-e git+git://github.com/sobotklp/django-nose@b3f485e914965e0a7b7012c309864d6a6dcac3ed#egg=django-nose
pyquery==1.2.4
# Documentation
View
4 setup.py
@@ -5,7 +5,7 @@
def read(*parts):
- return codecs.open(os.path.join(os.path.dirname(__file__), *parts)).read()
+ return codecs.open(os.path.join(os.path.dirname(__file__), *parts), encoding='utf8').read()
def find_version(*file_paths):
@@ -27,7 +27,7 @@ def find_version(*file_paths):
author_email='mkelly@mozilla.com',
url='https://github.com/mozilla/django-browserid',
license='MPL v2.0',
- install_requires=['requests>=0.9.1', 'fancy_tag==0.2.0'],
+ install_requires=['requests>=0.9.1', 'fancy_tag==0.2.0', 'six>=1.3'],
test_suite="runtests.runtests",
include_package_data=True,
classifiers=[
Something went wrong with that request. Please try again.