Cannot enter WarningsChecker twice

[Sup3rGeo]

• Include a detailed description of the bug or suggestion

pytest.warns fails with RuntimeError: Cannot enter WarningsChecker(record=True) twice when parametrizing test function multiple times.

• Minimal example if possible

This works just fine:

import pytest
import warnings

@pytest.mark.parametrize("expectation", [
	pytest.warns(DeprecationWarning,
					  match="Message A"),
	pytest.warns(DeprecationWarning,
					  match="Message B"),					  
])
def test_parametrized_warnings(expectation):
    with expectation:
	    warnings.warn("Message A", DeprecationWarning)

But when parametrizing another argument independently:

import pytest
import warnings

@pytest.mark.parametrize("other", [1, 2])
@pytest.mark.parametrize("expectation", [
	pytest.warns(DeprecationWarning,
					  match="Message A"),
	pytest.warns(DeprecationWarning,
					  match="Message B"),					  
])
def test_parametrized_warnings(other, expectation):
    with expectation:
	    warnings.warn("Message A", DeprecationWarning)

Then I get this error:

RuntimeError: Cannot enter WarningsChecker(record=True) twice

• pytest and operating system versions

pytest 4.0.1 and Windows 10

• pip list of the virtual environment you are using

Package                  Version
------------------------ ----------
alabaster                0.7.12
allure-pytest            2.5.4
allure-python-commons    2.5.4
apipkg                   1.5
asn1crypto               0.24.0
asv                      0.3.1
atomicwrites             1.2.1
attrs                    18.2.0
aws-xray-sdk             0.95
Babel                    2.6.0
backcall                 0.1.0
beautifulsoup4           4.7.1
bleach                   3.0.2
blosc                    1.7.0
boto                     2.49.0
boto3                    1.9.75
botocore                 1.12.75
Bottleneck               1.2.1
certifi                  2018.11.29
cffi                     1.11.5
chardet                  3.0.4
colorama                 0.4.1
coverage                 4.5.2
cpplint                  1.3.0
cryptography             2.4.2
cycler                   0.10.0
Cython                   0.29.2
decorator                4.3.0
docker                   3.6.0
docker-pycreds           0.4.0
docutils                 0.14
ecdsa                    0.13
entrypoints              0.3
et-xmlfile               1.0.1
execnet                  1.5.0
fastparquet              0.2.1
flake8                   3.6.0
flake8-comprehensions    1.4.1
flake8-rst               0.7.0
future                   0.17.1
gitdb2                   2.0.5
GitPython                2.1.11
html5lib                 1.0.1
hypothesis               3.86.7
idna                     2.8
imagesize                1.1.0
ipykernel                5.1.0
ipython                  7.2.0
ipython-genutils         0.2.0
isort                    4.3.4
jdcal                    1.4
jedi                     0.13.2
Jinja2                   2.10
jmespath                 0.9.3
jsondiff                 1.1.1
jsonpickle               1.0
jsonschema               2.6.0
jupyter-client           5.2.4
jupyter-core             4.4.0
kiwisolver               1.0.1
llvmlite                 0.27.0
lxml                     4.3.0
MarkupSafe               1.1.0
matplotlib               3.0.2
mccabe                   0.6.1
mistune                  0.8.4
mock                     2.0.0
more-itertools           4.3.0
moto                     1.3.7
nbconvert                5.3.1
nbformat                 4.4.0
nbsphinx                 0.4.1
numba                    0.42.0
numexpr                  2.6.9
numpy                    1.15.4
numpydoc                 0.8.0
openpyxl                 2.5.12
packaging                18.0
pandas                   0.23.4
pandocfilters            1.4.2
parso                    0.3.1
pbr                      5.1.1
PeakUtils                1.3.0
pickleshare              0.7.5
pip                      10.0.1
pluggy                   0.8.0
prompt-toolkit           2.0.7
py                       1.7.0
pyaml                    18.11.0
pyarrow                  0.11.1
pycodestyle              2.4.0
pycparser                2.19
pycryptodome             3.7.2
pycryptodomex            3.7.2
pyflakes                 2.0.0
Pygments                 2.3.1
pyModbusTCP              0.1.8
pyparsing                2.3.0
pypiwin32                223
pytest                   4.0.1
pytest-cov               2.6.1
pytest-forked            0.2
pytest-runner            4.2
pytest-xdist             1.25.0
python-dateutil          2.7.5
python-jose              2.0.2
pytz                     2018.7
pywin32                  224
PyYAML                   3.13
pyzmq                    17.1.2
requests                 2.21.0
responses                0.10.5
s3fs                     0.2.0
s3transfer               0.1.13
scipy                    1.1.0
seaborn                  0.9.0
setuptools               39.0.1
six                      1.12.0
smmap2                   2.0.5
snowballstemmer          1.2.1
soupsieve                1.6.2
Sphinx                   1.8.3
sphinxcontrib-websupport 1.1.0
SQLAlchemy               1.2.15
statsmodels              0.9.0
tables                   3.4.4
testpath                 0.4.2
thrift                   0.11.0
tornado                  5.1.1
traitlets                4.3.2
Typhoon-HIL-API          1.3.0
TyphoonTest              1.2.1
urllib3                  1.24.1
wcwidth                  0.1.7
webencodings             0.5.1
websocket-client         0.54.0
Werkzeug                 0.14.1
win-inet-pton            1.0.1
wrapt                    1.10.11
xarray                   0.11.2
xlrd                     1.2.0
XlsxWriter               1.1.2
xlwt                     1.3.0
xmltodict                0.11.0

[nicoddemus]

Howdy @Sup3rGeo!

I believe it is just a matter of setting self._entered = False at the end of WarningsRecorder.__exit__:

pytest/src/_pytest/recwarn.py

Lines 186 to 193 in a4c426b

	def __exit__(self, *exc_info):
	if not self._entered:
	__tracebackhide__ = True
	raise RuntimeError("Cannot exit %r without entering first" % self)
	# see above where `self._saved_warn` is assigned
	if six.PY2:
	warnings.warn = self._saved_warn
	super(WarningsRecorder, self).__exit__(*exc_info)

Would you like to open a PR? 😁

[Sup3rGeo]

Sure, working on it.

Isn't this a bug instead of enhancement?

Also, WarningsRecorder inherits from warnings.catch_warnings. The _entered flag is dealt with in the inherited class itself, so although we can add this to WarningsRecorder, shouldn't it be actually changed in warnings.catch_warnings itself?

[RonnyPfannschmidt]

the stdlib "bug" exists since python2.7 - as far as i can tell catch_warnings was never meant to be reentreant

[RonnyPfannschmidt]

based on the overall code im inclined to declare the warnings recorders non-reentreant instead

[Sup3rGeo]

@RonnyPfannschmidt what would be the drawback of making them reentrant with the fix @nicoddemus proposed?

[RonnyPfannschmidt]

among other things inconsistent state of the object, as far as i can tell its already tricky with all the stdlib monkeypatching catch_warning does, then i happens twice and we drop additional extra state on theo bject

i really dislike the complexity properties of that (its a big mix and match mess)

[nicoddemus]

among other things inconsistent state of the object,

Not sure, if it gets inconsistent somehow it is due to its complexity and a bug in the implementation, not the fact that it is reentrant. I agree it is kind of messy though.

About being non-reentrant being a "feature", I was about to propose for @Sup3rGeo to open an issue/PR on CPython itself making it reentrant.

[RonnyPfannschmidt]

@nicoddemus we could elevate part of that inconsistency by switching up the enter/exit logic,

if we decouple the recorder from the interaction object, and return only the interaction object, we are down to traceback filtering and py2 fixup

the py2 fixups should be gone by the end of the year, and the traceback filtering has a longstanding issue on framework sections (like stdlib, sqla, ...)

if traceback filtering goes there we will end up no longer requiring most oft hat and can rely on a consistent stdlib impl in some sense (as our own reusable context manager would then yield different objects

[nicoddemus]

@RonnyPfannschmidt I see what you mean, but TBH I'm not sure it is worth the trouble because we are only resetting the flag back to its initial state, which has no impact in future maintenance.

[RonnyPfannschmidt]

lets go for the current idea then and take a look at my more complex plan when trouble happens