Update test suite

[rpkilby]

Hi @dfunckt. Taking a stab at resolving #76 and #75. Happy to go into more detail if you have questions about choices/changes. This also supersedes/closes #52.

• Tests are now Django 1.11+
• Drops nose for Django's builtin test runner (Django has shipped its discovery runner since 1.6)
• Refactored nose-based test setup into TestData mixin.
• Refactored nose-style tests into unittest-style test cases. This adds a lot to the linecount, but it's straightforward spacing + function => method conversion. I couldn't manage to get the predicate test diffs to look clean. Even when breaking up into multiple steps, something trips up the diff.
• It's no longer necessary to use the runtests.py script, tox now just calls manage.py test directly.
• It's not necessary to add the project and tests directories to the PYTHONPATH, since the script's (manage.py) directory is automatically inserted, and tox will install the rules into the test env.
• Simplified the travis build (can revert this if preferred). By using tox-travis, we can run all builds for the same python version on the same run. Most of the CI runtime is spent creating the VMs. This should reduce that significantly.
• Removed the compatibility chart from the Travis config. I figure this should just be "all supported versions of Django".
• Added a dedicated packaging build. Can discuss further, but you need either a coverage or packaging-specific build.

[dfunckt]

Hey @rpkilby, this is amazing, thank you so much! I've only left a single comment/question, otherwise looks great.

Added a dedicated packaging build. Can discuss further, but you need either a coverage or packaging-specific build.

What is the "packaging build"? Is it something tox-specific? Why is it needed?

[dfunckt]

Can you please move the Django requirement back to "tests_require" so it's still optional? Rules should be able to be used standalone without Django.

[rpkilby]

Right. Should I just remove it entirely then? The tests_require isn't used by tox.

Sorry - to be clear, I'm suggesting that tests_require isn't necessary, since tests are ran through tox, not python setup.py test.

[dfunckt]

Gotcha. Let’s remove it completely then.

[rpkilby]

Sorry - this requires a bit of context. One of the common problems with projects is that they aren't actually testing their built distributions (the sdist/wheel). Looking at django-rules's current project structure,

django-rules/
├ rules/
│ └ ...
├ tests/
│ └ ...
├ runtests.py
└ setup.py

If I execute the below, am I testing the installed distribution (via pip install .), or am I testing the package code located in the ./rules/ directory?

$ python3 -m venv .venv
$ source .venv/bin/activate
$ pip install .
$ python runtests.py

It's the latter. Python inserts the script's directory as the first item in the PYTHONPATH. While django-rules was installed into the virtual environment, Python will find the local ./rules/ directory before it searches the virtual environment's site-packages. Because of this, the distribution's files aren't actually tested, which may result in unintentionally shipping a broken package. Some examples:

• It's fairly common for new django apps to not properly include their template, static, or locale files.
• django-rules may add a new sub-package, but forget to include it to the setup.py's packages list.

So, how do we fix this? Basically, make the local package directory not directly importable by the test script. Either move the package code into an intermediate directory, such as ./src/rules/, or move the test script into ./tests/. The latter was easier in this case, given that ./tests/manage.py already existed.

Now that ./rules/ is no longer importable, Python will find the distribution files installed into the virtual environment. However, we now run into a new problem - test coverage. Technically, test coverage still works, but the code is now located in the test virtual environment. The coverage report will now have files prefixed with something like .venv/lib/pythonX.Y/site-packages/rules/ instead of ./rules/. This is a) ugly, and b) prevents combining reports across multiple test builds (e.g., py36-django111/py36-django20).

How do we fix coverage? By using an editable install. An editable install symlinks to ./rules/, fixing the directory prefix issue. In the case of tox, editable installs are enabled with usedevlop = True.

However, by using an editable install, we go back to the original issue - we're not testing the distribution. In general, the test matrix either needs to prioritize testing the package's distribution, or testing with coverage enabled. You then need either a specialized packaging or coverage build to cover the other case. The existing matrix already prioritizes coverage, so it made sense to add a packaging build.

---

tl;dr, this isn't tox-specific, most projects don't properly test their build artifacts (sdist, wheel) and may unintentionally ship a broken package. The packaging build is an extra build that ensures the build artifacts are specifically tested against/are not broken.

[rpkilby]

Hi @dfunckt - did you have any additional questions/concerns about the PR? I'm planning on working on a few additional features/fixes, but am waiting for this to be merged first, given how many files this PR affects.

In general, my goal is to get django-rules up-to-date, so that we can replace django-guardian in the DRF test suite with django-rules (django-guardian integration has been split into a separate package).

[dfunckt]

Hey @rpkilby — the PR looks great. I’d like to add a couple of things in the rules 1.x line before merging this to start 2.x and I just didn’t get round to it yet unfortunately. It’s on my radar though, I haven’t forgotten.

It seems you’re blocked, what you can do is base your next PR on this branch instead of master to get going and I’ll make sure master catches up soon.

[lorddaedra]

Django 2.1rc1 is out. Final Django 2.1 will be released in less than 12 days based on https://code.djangoproject.com/wiki/Version2.1Roadmap ... Looks like django-rules will not work without fixes for Django 2.1. And fixes will require us to drop old Django versions... ( I created issue #74 )

[dfunckt]

@rpkilby this PR landed in c7c1d32 -- rebased, with slight modifications. This and a few other things are released as rules 2.0. Thank you very much for this, great stuff :)

cc @lorddaedra