Skip to content

Commit

Permalink
Fix tests for Django 1.11
Browse files Browse the repository at this point in the history
All the changes were made in a way that doesn't break the
compatibility of older Django versions. Following changes were
made to ensure that all tests pass for Django 1.11:

- _set_choices method was copied directly to StateEnumField from
  Django's ChoiceField. Using another classes setter method broke
  the reference to the instance. Since the method is copied, there
  should be no functionality changes.

- Django's method render_to_response doesn't exist in version 1.11,
  so it was changed to render. Render is available in the older
  versions of Django [1], and should have the same functionality.
  Also, slight change had to be done to the testcase
  'test_view_zipped_big_html_context' to account for this new method.

- Template loading has a different configuration in the newer
  versions of Django, so dual code path for old and new styles were
  created. 'APP_DIRS' was added because Django uses a default template
  for its logout page.

- HttpRequest.REQUEST was removed in the newer Django versions [2], so
  it was replaced by searching POST followed by GET, as specified in
  the documentation.

- 'permanent' parameter is now explicitly set in the
  'RedirectView.as_view' method because the default value changes
  in Django 1.9 [3].

- Testcases 'test_krb5login' and 'test_krb5login_redirect_to' now
  accept both absolute and relative urls in the 'Location' of the
  redirect response. This is because higher versions of Django return
  relative addresses. It's acceptable by the RFC standard [4].

- The 'TestFixturesJSONField' is skipped Django 1.9+. This is because
  the 'syncdb' command was removed in Django 1.9 [5], so testing the
  automatic fixture loading doesn't make sense.

[1] https://docs.djangoproject.com/en/1.8/topics/http/shortcuts/#render
[2] https://docs.djangoproject.com/en/1.8/ref/request-response/#django.http.HttpRequest.REQUEST
[3] https://docs.djangoproject.com/en/1.8/ref/class-based-views/base/#django.views.generic.base.RedirectView.permanent
[4] https://tools.ietf.org/html/rfc7231#section-7.1.2
[5] https://docs.djangoproject.com/en/2.2/releases/1.9/#features-removed-in-1-9
  • Loading branch information
querti committed Apr 2, 2020
1 parent c37b1e6 commit ea2468e
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 21 deletions.
1 change: 1 addition & 0 deletions constraints-django111.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Django==1.11.29
25 changes: 20 additions & 5 deletions examples/state/settings.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# Django settings for foo project.
from django import VERSION

DEBUG = True
TEMPLATE_DEBUG = DEBUG
Expand Down Expand Up @@ -65,11 +66,25 @@

ROOT_URLCONF = 'state.urls'

TEMPLATE_DIRS = (
# Put strings here, like "/home/html/django_templates" or "C:/www/django/templates".
# Always use forward slashes, even on Windows.
# Don't forget to use absolute paths, not relative paths.
)
# The way to specify the template dirs differs between newer and older versions of Django
if VERSION[0:3] < (1, 9, 0):
TEMPLATE_DIRS = (
# Put strings here, like "/home/html/django_templates" or "C:/www/django/templates".
# Always use forward slashes, even on Windows.
# Don't forget to use absolute paths, not relative paths.
)
if VERSION[0:3] >= (1, 9, 0):
TEMPLATES = [
{
'BACKEND': 'django.template.backends.django.DjangoTemplates',
'DIRS': (
# Put strings here, like "/home/html/django_templates" or "C:/www/django/templates".
# Always use forward slashes, even on Windows.
# Don't forget to use absolute paths, not relative paths.
),
'APP_DIRS': True
}
]

INSTALLED_APPS = (
'django.contrib.auth',
Expand Down
19 changes: 17 additions & 2 deletions kobo/django/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
from django.utils.translation import ugettext_lazy as _
from django.core import exceptions
from django.utils.text import capfirst
from django.forms.widgets import Select
from django.forms.fields import CallableChoiceIterator

import kobo.django.forms
from kobo.types import StateEnum
Expand Down Expand Up @@ -42,11 +44,12 @@ class MyModel(models.Model):

class StateEnumField(models.IntegerField):
'''StateEnum DB encapsulation'''

widget = Select

def __init__(self, state_machine, *args, **kwargs):
super(StateEnumField, self).__init__(*args, **kwargs)
self.state_machine = state_machine
self.widget = Select
if self.has_default:
self.state_machine.set_state(self.default)

Expand Down Expand Up @@ -98,7 +101,19 @@ def to_python(self, value):

def _get_choices(self):
return tuple(self.state_machine.get_next_states_mapping(current=self.state_machine.get_state_id()))
choices = property(_get_choices, kobo.django.forms.StateChoiceFormField._set_choices)

def _set_choices(self, value):
# Setting choices also sets the choices on the widget.
# choices can be any iterable, but we call list() on it because
# it will be consumed more than once.
if callable(value):
value = CallableChoiceIterator(value)
else:
value = list(value)

self._choices = self.widget.choices = value

choices = property(_get_choices, _set_choices)


def get_db_prep_value(self, value, connection, prepared=False):
Expand Down
11 changes: 7 additions & 4 deletions kobo/hub/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
else:
from django.core.urlresolvers import reverse
from django.http import HttpResponse, StreamingHttpResponse, HttpResponseForbidden
from django.shortcuts import render_to_response, get_object_or_404
from django.shortcuts import render, get_object_or_404
from django.template import RequestContext
from django.utils.translation import ugettext_lazy as _
from django.views.generic import RedirectView
Expand Down Expand Up @@ -184,7 +184,8 @@ def _rendered_log_response(request, task, log_name):
"json_url": reverse("task/log-json", args=[task.id, log_name]),
}

return render_to_response("task/log.html", context, context_instance=RequestContext(request))
#return render_to_response("task/log.html", context, context_instance=RequestContext(request))
return render(request, "task/log.html", context)


def task_log(request, id, log_name):
Expand Down Expand Up @@ -262,8 +263,10 @@ def krb5login(request, redirect_field_name=REDIRECT_FIELD_NAME):
middleware = 'kobo.django.auth.middleware.LimitedRemoteUserMiddleware'
if middleware not in settings.MIDDLEWARE_CLASSES:
raise ImproperlyConfigured("krb5login view requires '%s' middleware installed" % middleware)
redirect_to = request.REQUEST.get(redirect_field_name, "")
redirect_to = request.POST.get(redirect_field_name, "")
if not redirect_to:
redirect_to = request.GET.get(redirect_field_name, "")
if not redirect_to:
redirect_to = reverse("home/index")
return RedirectView.as_view(url=redirect_to)(request)
return RedirectView.as_view(url=redirect_to, permanent=True)(request)

5 changes: 4 additions & 1 deletion tests/fields_test/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@

import json
import unittest
import pytest

from django.core.exceptions import ValidationError
from django import VERSION
from .models import DummyDefaultModel, DummyModel, DummyNotHumanModel


Expand Down Expand Up @@ -77,7 +79,8 @@ def test_complex_assignment_with_save(self):

self.assertEqual(d.field, data)


@unittest.skipUnless(VERSION[0:3] < (1, 9, 0),
"Automatic fixture loading is not possible since syncdb was removed.")
class TestFixturesJSONField(unittest.TestCase):
"""
DO NOT ADD ANYTHING INTO DATABASE IN THIS TESTCASE
Expand Down
23 changes: 19 additions & 4 deletions tests/settings.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Settings for Django testcases against kobo hub
import os
import kobo
from django import VERSION

KOBO_DIR = os.path.normpath(
os.path.join(os.path.dirname(__file__), '..', 'kobo')
Expand Down Expand Up @@ -45,10 +46,24 @@
# - automatic lookup of templates under kobo/hub isn't working for some reason,
# not sure why, but might be related to use of deprecated arguments in
# render_to_string (FIXME)
TEMPLATE_DIRS = (
os.path.join(KOBO_DIR, 'admin/templates/hub/templates'),
os.path.join(KOBO_DIR, 'hub/templates'),
)
#
# The way to specify the template dirs differs between newer and older versions of Django
if VERSION[0:3] < (1, 9, 0):
TEMPLATE_DIRS = (
os.path.join(KOBO_DIR, 'admin/templates/hub/templates'),
os.path.join(KOBO_DIR, 'hub/templates'),
)
if VERSION[0:3] >= (1, 9, 0):
TEMPLATES = [
{
'BACKEND': 'django.template.backends.django.DjangoTemplates',
'DIRS': (
os.path.join(KOBO_DIR, 'admin/templates/hub/templates'),
os.path.join(KOBO_DIR, 'hub/templates')
),
'APP_DIRS': True
}
]

ROOT_URLCONF = 'tests.hub_urls'

Expand Down
4 changes: 2 additions & 2 deletions tests/test_view_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ def test_view_zipped_big_html_context(self):
def render(*args, **kwargs):
return HttpResponse(status=200)

with mock.patch('kobo.hub.views.render_to_response', side_effect=render) as render_mock:
with mock.patch('kobo.hub.views.render', side_effect=render) as render_mock:
self.get_log('zipped_big.log')

mock_call = render_mock.mock_calls[0]
Expand All @@ -255,7 +255,7 @@ def render(*args, **kwargs):
self.assertEqual(mock_call[0], '')

call_args = mock_call[1]
(template_name, context) = call_args
(_, template_name, context) = call_args

self.assertEqual(template_name, 'task/log.html')

Expand Down
4 changes: 2 additions & 2 deletions tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def test_login(self):
def test_krb5login(self):
response = self.client.get('/auth/krb5login/')
self.assertEquals(response.status_code, 301)
self.assertEquals(response['Location'], 'http://testserver/home/')
self.assertIn(response['Location'], ['http://testserver/home/', '/home/'])

@override_settings(MIDDLEWARE_CLASSES=[])
def test_krb5login_missing_middleware(self):
Expand All @@ -52,7 +52,7 @@ def test_krb5login_missing_middleware(self):
def test_krb5login_redirect_to(self):
response = self.client.get('/auth/krb5login/', {REDIRECT_FIELD_NAME: '/auth/login/'})
self.assertEquals(response.status_code, 301)
self.assertEquals(response['Location'], 'http://testserver/auth/login/')
self.assertIn(response['Location'], ['http://testserver/auth/login/', '/auth/login/'])

def test_logout(self):
response = self.client.get('/auth/logout/')
Expand Down
3 changes: 2 additions & 1 deletion tox.ini
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[tox]
envlist = {py27,py34,py35,py36,py37}-django18
envlist = {py27,py34,py35,py36,py37}-{django18,django111}
skip_missing_interpreters = True

[testenv]
Expand All @@ -14,6 +14,7 @@ basepython =
deps =
-rtest-requirements.txt
django18: -cconstraints-django18.txt
django111: -cconstraints-django111.txt

[testenv:py37-cov-travis]
passenv = TRAVIS TRAVIS_*
Expand Down

0 comments on commit ea2468e

Please sign in to comment.