Implement the --django-debug plugin option with tests

[dulacp]

Follows the #228 issue, and the un-merged #231 PR.

I've taken into account @blueyed comment, and included a comprehensive list of tests.

Cheers

[karyon]

fwiw, starting from django 1.11 (to be released in april), django's test runner will have that option: https://docs.djangoproject.com/en/dev/ref/django-admin/#cmdoption-test-debug-mode

[blueyed]

@karyon
Nice find.

It gets passed through to setup_test_environment (https://github.com/django/django/blob/5a6f70b4281817656db2f36c5919036d38fcce7f/django/test/runner.py#L461), which we should do then, too (in

pytest-django/pytest_django/plugin.py

Line 340 in b3d7fd6

setup_test_environment()

).

@dulaccc
Can you change this PR accordingly?
It should pass it in for Django 1.11+, but change the setting (as done currently) for older versions.

[dulacp]

Sure, I will try to update that soon!

[auvipy]

is this going out of radar?

[blueyed]

@dulaccc
Can you update this please? (we might have a release soon)

[codecov-io]

Codecov Report

Merging #463 into master will increase coverage by 0.18%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #463      +/-   ##
==========================================
+ Coverage    97.9%   98.08%   +0.18%     
==========================================
  Files          32       32              
  Lines        1863     1883      +20     
  Branches      147      149       +2     
==========================================
+ Hits         1824     1847      +23     
+ Misses         26       24       -2     
+ Partials       13       12       -1

Impacted Files	Coverage Δ
tests/test_django_settings_module.py	100% <100%> (ø)	⬆️
pytest_django/plugin.py	94.14% <100%> (+0.12%)	⬆️
tests/test_db_setup.py	100% <0%> (ø)	⬆️
tests/conftest.py	100% <0%> (ø)	⬆️
pytest_django/live_server_helper.py	96.87% <0%> (+4.68%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 17cdcda...b64c795. Read the comment docs.

[blueyed]

Should use new API wiith Django 1.11+ (see above).

[dulacp]

I fixed the PR with the Django 1.11 new setup_test_environment API.
And I used the naming convention --django-debug-mode.

Is that okay for you?

Cheers

[karyon]

fyi, the tests failed :)

[dulacp]

@karyon are you sure the tests are stable between builds? because I've only 3 commits above the 4145f73 commit (which is marked as green on master). And the failed tests on travis are totally non-related to my additions.

I'll investigate,
Cheers

[karyon]

sorry, i'm not a maintainer of pytest-django, i just saw that the tests failed and thought i let you know because i myself often don't notice that :)

[dulacp]

Oh, alright. Thanks for the heads-up @karyon! :)

[dulacp]

Hey @blueyed,

All the jobs targeting Python 2.7 are failing on Travis.
I suspect that something in the stack changed during the past 15 days (because 15 days ago the commit 4145f73 was green).

Do you have other feedbacks on such an issue?

[blueyed]

Tests are failing with pytest 3.3 (and the other jobs are using an older version (cache likely?!))
See #550.

[blueyed]

Please rebase, since our tests in master were failing with pytest 3.3.

[dulacp]

@blueyed nice catch for the pytest version and the caching issue!
I guess everything is green now 🍻

[dulacp]

@blueyed can you merge this before master diverge?
Cheers

[blueyed]

Rephrased the doc a bit - please review.
Left some other comments.

[samueldotj]

@dulacp This issue took a solid 45 minutes to track it down - I guess a lot of other developers would have faced the same issue. It would be great if you could get this committed.

[richardARPANET]

Just wasted 30 minutes of my life due to this. Would be great to get this merged.

[auvipy]

please rebase

[blueyed]

@samueldotj @richardARPANET
What exactly took you time to track it down? IIRC this is about adding a new option to control the DEBUG setting only, isn't it?

Also it clearly cannot be "just merged" given 4 conflicting files.

[richardARPANET]

@blueyed My application was configured by env vars. One for the DEBUG setting.

DEBUG = os.environ['DEBUG'].lower() == 'true'

It worked at run-time, but not at test run-time. Which let to me trying/checking all sorts of things, I didn't expect pytest-django to be overriding this setting at all.

[blueyed]

I see.
FWIW this can also be controlled through settings, i.e.

@pytest.fixture(scope="session", autouse=True)
def dj_debug(settings):
    settings.DEBUG = True

(or per test)

Django itself has --debug-mode, used by setup_test_environment (where we should/could pass this then also).

[blueyed]

@dulacp
I'll pick it up later I guess - please let me know if you want / can do it instead.

@auvipy
These kind of (obvious) "reviews" are not useful.

With regard to debugging you might be interested in #725 - I'd appreciate a review of it in return.

[blueyed]

@dulacp
Thanks!

I've pushed a fixup commit, let me know if this looks good to you, and we can finally (squash-)merge this then.

[dulacp]

Thanks for the fixup commit! It looks great :)

[blueyed]

Great. Just wondered about the name - it mentions --django-debug initially, but is django-debug-option. I'd prefer the shorter one, what do you think?
(sorry if this was changed / discussed before, just a quick thought)

[blueyed]

I think we should use --django-debug and --django-no-debug flags here.

[dulacp]

I've made the changes to use django-debug now.

Cheers

[Andrew-Chen-Wang]

@blueyed Should the master branch be merged to incorporate some new tests and pytest 5.4.0 or is this good to go?

[bluetech]

Oops, I was not aware of this PR and added a similar solution in #888. Only I used an ini option and called it django_debug_mode. Since that one is already merged, I'll close this one. Sorry for missing it, but let me know if you have any comments.