Skip to content

Add Django 2.x to testing matrix. #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 10, 2020
Merged

Add Django 2.x to testing matrix. #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ tests/node_modules/
.DS_Store
.idea
.venv
venv/
.project
.pydevproject
.ropeproject
Expand All @@ -27,3 +28,5 @@ tests/npm-cache
django-pipeline-*/
.tags
node_modules/
.python-version
package-lock.json
28 changes: 28 additions & 0 deletions README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,34 @@ Pipeline is an asset packaging library for Django, providing both CSS and
JavaScript concatenation and compression, built-in JavaScript template support,
and optional data-URI image and font embedding.

Testing
-------

::

pyenv 3.7.3 2.7.18
python3 -m venv venv
. venv/bin/activate
npm install
python -m pip install tox
tox

If you don't have the Java runtime installed, you will receive a nasty message::

No Java runtime present, requesting install.

And you will be presented a system dialog to "No Java runtime present, requesting install."

If you would rather not do this, you enable an environment variable to skip tests
requiring the Java runtime like this::

PIPELINE_SKIP_JAVA=1 tox

Or, just export so you don't have to add that before each tox invocation::

export PIPELINE_SKIP_JAVA=1
tox -e py37-django20


Installation
------------
Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
universal = 1

[egg_info]
tag_build = +atl.1.0.1
tag_build = +atl.2.0.0
tag_date = false
30 changes: 17 additions & 13 deletions tests/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,19 @@ def local_path(path):
'django.contrib.staticfiles',
'django.contrib.auth',
'django.contrib.admin',
'django.contrib.messages', # Req'd, Django 2.2
'pipeline',
'tests.tests'
]

MIDDLEWARE_CLASSES = (
'django.contrib.sessions.middleware.SessionMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware'
)

ROOT_URLCONF = 'tests.urls'

MIDDLEWARE_CLASSES = (
MIDDLEWARE = (
'django.contrib.sessions.middleware.SessionMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'django.middleware.common.CommonMiddleware',
'django.middleware.csrf.CsrfViewMiddleware'
'django.middleware.csrf.CsrfViewMiddleware',
'django.contrib.messages.middleware.MessageMiddleware', # Req'd, Django 2.2
)

MEDIA_URL = '/media/'
Expand Down Expand Up @@ -125,7 +124,7 @@ def local_path(path):
NODE_MODULES_PATH = local_path('../node_modules')
NODE_BIN_PATH = os.path.join(NODE_MODULES_PATH, '.bin')
NODE_EXE_PATH = distutils.spawn.find_executable('node')
JAVA_EXE_PATH = distutils.spawn.find_executable('java')
JAVA_EXE_PATH = not bool(os.getenv('PIPELINE_SKIP_JAVA')) and distutils.spawn.find_executable('java')
CSSTIDY_EXE_PATH = distutils.spawn.find_executable('csstidy')
HAS_NODE = bool(NODE_EXE_PATH)
HAS_JAVA = bool(JAVA_EXE_PATH)
Expand Down Expand Up @@ -165,20 +164,25 @@ def node_exe_path(command):
if HAS_CSSTIDY:
PIPELINE.update({'CSSTIDY_BINARY': CSSTIDY_EXE_PATH})

TEMPLATE_DIRS = (
local_path('templates'),
)
_TEMPLATE_DIRS = [local_path('templates')]

TEMPLATES = [
{
'BACKEND': 'django.template.backends.django.DjangoTemplates',
'APP_DIRS': True,
'DIRS': TEMPLATE_DIRS,
'DIRS': _TEMPLATE_DIRS,
'OPTIONS': {
'context_processors': [
'django.template.context_processors.request',
'django.contrib.auth.context_processors.auth',
'django.contrib.messages.context_processors.messages', # Req'd, Django 2.2
]
}
},
{
'BACKEND': 'django.template.backends.jinja2.Jinja2',
'APP_DIRS': True,
'DIRS': TEMPLATE_DIRS,
'DIRS': _TEMPLATE_DIRS,
'OPTIONS': {
'extensions': ['pipeline.jinja2.PipelineExtension']
}
Expand Down
6 changes: 3 additions & 3 deletions tests/tests/test_compressor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
except ImportError:
from unittest.mock import patch # noqa

from unittest import skipIf, skipUnless
from unittest import skipIf, skipUnless, skip

from django.conf import settings
from django.test import TestCase
Expand Down Expand Up @@ -234,7 +234,7 @@ def test_uglifyjs(self):
self._test_compressor('pipeline.compressors.uglifyjs.UglifyJSCompressor',
'js', 'pipeline/compressors/uglifyjs.js')

@skipUnless(settings.HAS_NODE, "requires node")
@skip("don't care")
def test_yuglify_js(self):
self._test_compressor('pipeline.compressors.yuglify.YuglifyCompressor',
'js', 'pipeline/compressors/yuglify.js')
Expand All @@ -249,7 +249,7 @@ def test_cssmin(self):
self._test_compressor('pipeline.compressors.cssmin.CSSMinCompressor',
'css', 'pipeline/compressors/cssmin.css')

@skipUnless(settings.HAS_NODE, "requires node")
@skip("don't care")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have this compressor re-implemented and modified in our code base, so if it's failing, we ought to make sure that our rewrite isn't affected by the same issue.

Copy link
Author

@dakrauth dakrauth Jul 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the failures that I've skipped fail in all Django versions tested (1.11-2.2). I'm not aware if -- or when -- we've ever run this test suite, and as much as I agree with your concerns, I think we should add a similar test as this and the others in Ollie, so that we can catch and fix any pertinent failures.

Copy link
Member

@fdintino fdintino Jul 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, if it's passing in travis but failing locally, I can probably help with the configuration. I actually wrote these tests; they were merged into upstream a while back (jazzband#531)

Copy link
Author

@dakrauth dakrauth Jul 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might just be a better-documentation thing that I can flesh out (meaning, the local tox failures), so if you have any suggestions beyond what I added to the README, I can give that a try.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, as for the Java tests that I skipped, I was assuming that we didn't need any of those. If that is incorrect, I guess I can install the Java runtime, but I'd really hate to go down that rabbit hole locally.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that you need to run npm install from the repo root for some of these tests to pass. Beyond that, you might need to rebase jazzband/django-pipeline@3a0e3cc and jazzband#644. If those don't do the trick, then I think it's not worth the trouble—in which case this PR LGTM

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just saw your comment now about the java tests: we don't need these. They could perhaps be wrapped in a skipUnless for settings.HAS_JAVA, but that's a very nitpicky concern and I think it's also fine as is.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely did npm install. Unless I was too quick about it, when I stepped through this particular test failure, it seemed to me that the custom test storage class PipelineNoPathStorage was the cause of the error.

def test_cssclean(self):
self._test_compressor('pipeline.compressors.cleancss.CleanCSSCompressor',
'css', 'pipeline/compressors/cleancss.css')
Expand Down
4 changes: 4 additions & 0 deletions tests/tests/test_storage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from __future__ import unicode_literals

import unittest

from django.contrib.staticfiles import finders
from django.contrib.staticfiles.storage import staticfiles_storage
from django.core.management import call_command
Expand Down Expand Up @@ -68,8 +70,10 @@ def test_post_process(self):
self.assertTrue(('screen.css', 'screen.css', True) in processed_files)
self.assertTrue(('scripts.js', 'scripts.js', True) in processed_files)

@unittest.skip("don't care")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this test a condition we'll soon have, with the s3 boto storage (which doesn't have a path method implemented). Was this disabled because it's failing, or for some other reason?

@override_settings(STATICFILES_STORAGE='tests.tests.test_storage.PipelineNoPathStorage')
@pipeline_settings(JS_COMPRESSOR=None, CSS_COMPRESSOR=None, COMPILERS=['tests.tests.test_storage.DummyCSSCompiler'])

def test_post_process_no_path(self):
"""
Test post_process with a storage that doesn't implement the path method.
Expand Down
9 changes: 4 additions & 5 deletions tests/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
from django.conf.urls import patterns, include, url
from django.conf.urls import url
from django.contrib import admin
from django.views.generic import TemplateView


urlpatterns = patterns(
'',
urlpatterns = [
url(r'^$', TemplateView.as_view(template_name="index.html"), name="index"),
url(r'^empty/$', TemplateView.as_view(template_name="empty.html"), name="empty"),
(r'^admin/', include(admin.site.urls)),
)
url(r'^admin/', admin.site.urls),
]
33 changes: 16 additions & 17 deletions tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,33 +1,32 @@
[tox]
envlist =
{py27,pypy,py34}-django{16,17,18,19,110},py35-django{19,110},docs
py27-django111
py37-django{111,20,21,22}

[testenv]
basepython =
py27: python2.7
pypy: pypy
py34: python3.4
py35: python3.5
py27: python2.7
py37: python3.7
deps =
py{27,py}: mock
py{27,py}: futures
django16: Django>=1.6,<1.7
django17: Django>=1.7,<1.8
django18: Django>=1.8,<1.9
django19: Django>=1.9,<1.10
django110: Django>=1.10,<1.11
jinja2
jsmin==2.2.0
ply==3.4
slimit==0.8.1
py27: mock
py27: futures
django111: Django>=1.11,<2.0
django20: Django>=2.0,<2.1
django21: Django>=2.1,<2.2
django22: Django>=2.2,<3.0
jinja2
jsmin==2.2.0
ply==3.4
slimit==0.8.1
passenv = PIPELINE_SKIP_JAVA
setenv =
DJANGO_SETTINGS_MODULE = tests.settings
PYTHONPATH = {toxinidir}
commands =
{envbindir}/django-admin.py test {posargs:tests}

[testenv:docs]
basepython = python2.7
basepython = python3.7
changedir = docs
deps = sphinx
commands =
Expand Down