Automatically run checks at the start of tests

[adamchainz]

I've been trying to get Django's test runner to run manage.py check at the start of the test suite in django/django#6294. It actually used to do it in 1.7 and then no one noticed it disappearing in 1.8. There is already a solution in that PR but it requires a lot of secondary effort to fix all the check failures that appear in Django's own test suite.

The reason to run check is to avoid wasting time on tests when there is a setup related issue that shoudl be fixed first. Since tests don't run checks by default, TDD-only workflows (i.e. no runserver etc.) can miss check errors until e.g. CI runs them explicitly, which can waste effort. Also they take a trivial amount of time to run so don't slow tests down really.

I think it would probably be easier to add it to pytest-django before I finish my Django core PR. Also we're using pytest-django at work now, so it would fix it for me for the time being.

[blueyed]

Hmm.
I think you should run it manually.
OTOH we could provide a function/helper or option for it - bit it should not be the default probably.

[adamchainz]

Running it manually before tests begin but after the test database has been created is troublesome, that's why I was thinking patching it into the database creation logic in pytest-django would be easiest.

I can put it behind a flag to make it optional if you like, but bear in mind the upstream django test runner behaviour will change to run it by default when I finish up my PR there!

[adamchainz]

Also there isn't really any concern about slowing down test run, even on the full django test suite with its hundreds of models etc. it takes about 1s to run the full set of checks.

[blueyed]

Ok, then it could be enabled by default, but there should certainly be a switch to toggle it.
I tend to run it manually through manage.py check during CI (as a Makefile task).
manage.py check takes about 2s here on some project - which is quite noticeable.

I would still argue that adding a test which runs the check command might be the way to go, if you want to have this integrated in your tests directly. If this test requests the db fixture the DB should be setup, right?

What do others think?

[ssangervasi]

I was considering creating an issue, but I think this is the same as what I was thinking.

Currently, if there is a syntax error in my models.py, pytest-django itself will crash with a long traceback that goes through importlib. While this does indicate the offending line of code, it does not match how normal pytest handles syntax errors. Specifically, the attempt to test an invalid file will be raised as a normal test failure and the test suite continues.

[adamchainz]

@blueyed the most of the time you're seeing for manage.py check is importing the modules etc. all over again. If you work it into a test run, pre-tests, it's rather minimal - a few hundred milliseconds on my large project, about 1 second on Django's huge test suite.

@ssangervasi I don't think this is quite related - check doesn't directly pick up syntax errors, and your models.py would already have died by being imported by django.setup (presumably inside pytest-django code) before it could possibly run.

[pelme]

@ssangervasi can you please open a separate issue for the import syntax error?

@adamchainz I think it should be enabled by default, it seems useful and nice to follow Django's approach. However I think it should be easy to disable with a ini option and command line flag. If you are interested to work in this, that would be very helpful.

/cc @blueyed

[boxed]

I don't agree with the logic of it only taking a second so it's fine. What about the next guy with a proposal to add just one second? And the next? And the next? Soon people will say "it's only 10 seconds, it's not that much percentage wise"...

We should aim to eliminate 0.1 seconds if we can find it.

[boxed]

To put my comment above in some context.. I've just written PRs for pytest-cov, pytest-base-url and pytest-selenium that can collectively cut down on maybe 0.3 seconds, if I'm lucky 0.4. This is important! Code like this is run millions of times globally every day.

[tolomea]

I case anyone else ends up here I have worked around it's absence by adding this to my top level conftest.py

def pytest_configure(config):
    from django.core import management

    management.call_command("check")