Allow downstream to disable deprecations-as-errors with environment var

[Zac-HD]

Closes #969 - the Arch package has unbundled some upstream dependencies, so this option will be useful for @felixonmars 😄

CC @DRMacIver in case this is one of those things where we have divergent taste in solutions.

[kxepal]

Two questions:

0. Shouldn't we document this?
1. Should we provide pytest cli option to control this?
2. Should we check env variable value our just ensure that this variable is set? For instance, standard Python env variables works in this way, so there is no difference between PYTHONOPTIMIZE=1 and PYTHONOPTIMIZE=true and PYTHONOPTIMIZE=yes-please.

UPD. sorry, nvm, that's internal change.

[alexwlchan]

That’s three questions. 😉

I’d prefer a pytest CLI option – particularly because tox doesn’t pass through environment variables by default, so at minimum this needs a change to tox.ini or I’m pretty sure won’t work. A CLI option is more explicit in that regard.

(Also I just generally dislike environment variables as a way to control behaviour, because they’re easy to set and forget, and then you have behaviour that seems to work by magic.)

[kxepal]

Eh, I was hope I'll fix my misunderstood before anyone noticed (: Restored comment.

[alexwlchan]

Hey, I liked your questions!

[kxepal]

Get all back, no worries (: And sorry for confussion /:

[Zac-HD]

I'd be delighted if someone could show me how to add an argument via pytest - when I tried this it didn't work at all.

I'd document this in the notes added by #988, and will do that once either pull is merged.

I like explicitly checking the value of the var to avoid surprises if it's set to eg "0" or "false" - but I've added a warning for that case.

[kxepal]

@Zac-HD
How about something like this?

diff --git a/src/hypothesis/extra/pytestplugin.py b/src/hypothesis/extra/pytestplugin.py
index 86f01d13..60c1eed2 100644
--- a/src/hypothesis/extra/pytestplugin.py
+++ b/src/hypothesis/extra/pytestplugin.py
@@ -17,6 +17,8 @@
 
 from __future__ import division, print_function, absolute_import
 
+import warnings
+
 import pytest
 
 import hypothesis.core as core
@@ -29,6 +31,7 @@ from hypothesis.internal.detection import is_hypothesis_test
 LOAD_PROFILE_OPTION = '--hypothesis-profile'
 PRINT_STATISTICS_OPTION = '--hypothesis-show-statistics'
 SEED_OPTION = '--hypothesis-seed'
+TEST_NO_WERROR = '--hypothesis-test-no-werror'
 
 
 class StoringReporter(object):
@@ -63,6 +66,12 @@ def pytest_addoption(parser):
         action='store',
         help='Set a seed to use for all Hypothesis tests'
     )
+    group.addoption(
+        TEST_NO_WERROR,
+        action='store_true',
+        help='Stop count warnings as errors',
+        default=False
+    )
 
 
 def pytest_configure(config):
@@ -81,6 +90,8 @@ def pytest_configure(config):
     config.addinivalue_line(
         'markers',
         'hypothesis: Tests which use hypothesis.')
+    if not config.getoption(TEST_NO_WERROR):
+        warnings.filterwarnings('error', category=DeprecationWarning)
 
 
 gathered_statistics = OrderedDict()

[kxepal]

However, may be in this case would better to have WError enable flag rather then disable. I'm not sure if condition is correct and wouldn't be overriden later by someone else.

[kxepal]

And again...if this switch is only for Hypothesis tests and doesn't affects users, it may be not wise to have it on public since it causes no effect for them (or at least it supposed to in original implementation - mine patch makes it global, eventually).

[DRMacIver]

I feel like this might make more sense inverted? i.e. we don't turn warnings on unless an environment variable is set. This feature is mostly useful for our testing, and downstream packagers are almost guaranteed to want to disable it, so why not make it do the right thing by default for them?

I think if you do that, adding a pytest command line option is probably not that useful - if this is just for our testing and not for others' consumption, the option that involves the least amount of code is fine. :-)

[Zac-HD]

...we should probably just set PYTHONWARNINGS in our Tox config. We can even override this for older Django in tox.ini, and it will always run in CI but can be trivially disabled locally.

[Zac-HD]

• Happy path confirmed OK via Travis
• PYTHONWARNINGS=error::DeprecationWarning tox --recreate -e django18 fails as expected; passes without the env var

So we're ready to go!

In summary: if you set the standard PYTHONWARNINGS env var, you get the expected behaviour (including language defaults if it's set to the empty string). If the var is unset (eg on Travis), our Tox config defaults it to deprecation-warnings-as-errors.

[DRMacIver]

🎉 thanks for doing the leg work on this!

[Zac-HD]

No problem - I always learn something from the little fixes, and the tooling around Hypothesis is good enough that I never feel like my time is wasted 😄

[felixonmars]

Works great here, thanks a lot :D