classification
Title: Warning -- warnings.filters was modified by test_warnings
Type: Stage: resolved
Components: Tests Versions: Python 3.7, Python 3.6, Python 3.5
process
Status: closed Resolution: out of date
Dependencies: Superseder: Implement comparison (x==y and x!=y) for _sre.SRE_Pattern
View: 28727
Assigned To: Nosy List: martin.panter, python-dev, serhiy.storchaka, vstinner, xiang.zhang
Priority: normal Keywords: patch

Created on 2016-11-14 12:56 by vstinner, last changed 2016-11-22 08:47 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
warnings_key.patch vstinner, 2016-11-17 16:26
immortal-filters.patch martin.panter, 2016-11-18 23:22 review
Messages (17)
msg280767 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-14 12:56
The issue #23839 modified the test runner to always clear caches before running tests. As a side effect, test_warnings started to complain with:

Warning -- warnings.filters was modified by test_warnings

The issue comes from the following function of test_warnings/__init__.py:

def setUpModule():
    py_warnings.onceregistry.clear()
    c_warnings.onceregistry.clear()

I suggest to rewrite this function as a setUp/tearDown method in BaseTest and *restores* the old value of these dictionaries.

I guess that the bug affects all Python versions, even if only Python 3.7 logs a warning.
msg280777 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-11-14 14:12
How you got this warning? I can't reproduce.
msg280782 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-14 15:36
> How you got this warning? I can't reproduce.

Sorry, I forgot to mention that the warning only occurs if you run Python with -Werror:

@selma$ ./python -Werror -m test test_warnings
Run tests sequentially
0:00:00 [1/1] test_warnings
Warning -- warnings.filters was modified by test_warnings
test_warnings failed (env changed)

1 test altered the execution environment:
    test_warnings

Total duration: 2 sec
Tests result: SUCCESS


Moreover, the warning goes away if you don't run tests in verbose mode!?

haypo@selma$ ./python -Werror -m test -v test_warnings
...
1 test OK.

Total duration: 2 sec
Tests result: SUCCESS
msg280798 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-11-14 17:19
test___all__ gets the same behaviour.

./python -Werror -m test test___all__
Run tests sequentially
0:00:00 [1/1] test___all__
Warning -- warnings.filters was modified by test___all__
test___all__ failed (env changed)

1 test altered the execution environment:
    test___all__

Total duration: 1 sec
Tests result: SUCCESS

And on my PC in company I sometimes get the behaviour for test_distutils.

I originally thought this is not a problem.
msg281042 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-17 15:30
Hum, this issue is a regression caused by the issue #23839. The environment warning was already fixed by the issue #18383 (duplicate: issue #26742):

New changeset f57f4e33ba5e by Martin Panter in branch '3.5':
Issue #18383: Avoid adding duplicate filters when warnings is reloaded
https://hg.python.org/cpython/rev/f57f4e33ba5e

The problem is that _sre.SRE_Pattern doesn't import rich compare: so two patterns are only equal if it's exactly the same object... which is likely when re caches the compiled expression... But the Python test runner now starts by clearing the re cache!

I see different options:

* Find something else to not re-initialize warning filters, "_processoptions(sys.warnoptions)" in warnings.py.
* Fix warnings._add_filter() to implement a custom comparator operator for regular expression objects: compare pattern and flags
* Implement comparision in _sre.SRE_Pattern
msg281047 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-17 16:03
> * Implement comparision in _sre.SRE_Pattern

I wrote a patch and opened the issue #28727: "Implement comparison (x==y and x!=y) for _sre.SRE_Pattern".
msg281049 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-17 16:26
> * Fix warnings._add_filter() to implement a custom comparator operator for regular expression objects: compare pattern and flags

Attached patch warnings_key.patch implements this. I really dislike the patch :-(
msg281050 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-17 16:27
FYI Python 2.7 is not impacted by this bug because it seems like reimporting warnings.py twice gets a new fresh list from _warnings.filters. I don't undertand how/why.
msg281079 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-11-18 07:19
I didn’t really like adding the _add_filter() special handling in the first place, but I went ahead because I couldn’t think of a better way to avoid the problem with reloading the warnings modules. So unless anyone can suggest anything better, I am okay with your patch (even if you dislike it :).

+    return (item[0], _pattern_key(item[1]), item[2], _pattern_key(item[3]))

The key is based on (action, message, category, module). I think you should add item[4] (lineno).
msg281080 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-18 07:39
> The key is based on (action, message, category, module). I think you should add item[4] (lineno).

Oops, right!
msg281081 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-18 07:44
Ok, let me propose a plan for 3.5, 3.6 and 3.7:

* Remove warnings.filters test on Python 3.5 from regrtest
* Implement comparison for SRE_Pattern in Python 3.6 and 3.7: issue #28727

I consider that the issue #28727 is a minor enhancement and so is still good for Python 3.6.

So we avoid the ugly *warnings_key.patch*.

Reminder: this issue only occurs in the Python test suite which explicitly reloads modules. It should not occur in the wild.
msg281082 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-11-18 07:46
Your plan LGTM.
msg281179 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-11-18 23:22
Here is another way to remember that the filter list has already been initialized. I made a new immortal _warnings.filters_initialized flag at the C level. It is actually a list so that it can be mutated and remembered across module reloads, but it is either empty (evaluates as false), or a single element: [True].

Also, Python 2 does get duplicated filters, but I guess there is not test that exposes it.

$ python2 -Wall
. . .
>>> import warnings
>>> len(warnings.filters)
5
>>> reload(warnings)
<module 'warnings' from '/usr/lib/python2.7/warnings.pyc'>
>>> len(warnings.filters)
6

I agree there is no need to change Python 2 at this stage.
msg281368 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-11-21 15:46
New changeset 75b1091594f8 by Victor Stinner in branch '3.5':
Issue #28688: Remove warnings.filters check from regrtest
https://hg.python.org/cpython/rev/75b1091594f8

New changeset a2616863de06 by Victor Stinner in branch '3.6':
Issue #28688: Null merge 3.5
https://hg.python.org/cpython/rev/a2616863de06

New changeset da042eec6743 by Victor Stinner in branch 'default':
Issue #28688: Null merge 3.6
https://hg.python.org/cpython/rev/da042eec6743
msg281371 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-21 15:52
I implemented x==y operator for _sre.SRE_Pattern in Python 3.6 and 3.7, it fixed this issue.

For Python 3.5, I removed the warnings.filters test, as we discussed.

@Martin Panter: immortal-filters.patch works because I'm not super excited by the change. Somehow, it looks like a hack... even if I don't see any better solution for Python 3.5.

Since the bug only impacts Python test suite in the practical, is it really worth it to fix it in Python 3.5 which is almost in the "security fix only" stage?


@Martin: It's up to you, I have no strong opinion on your patch.


> I agree there is no need to change Python 2 at this stage.

Ok.
msg281424 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-11-22 01:00
As long as we are restricted by backwards compatibility, it will be hard to find a hack-free solution. The ideal solution IMO is to re-create _warnings.filters from scratch when _warnings is reloaded, but such a change would be safer only for 3.7.

So I am happy to leave things as they are. At least until something upsets things again :)
msg281444 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-11-22 08:47
I'm ok to close this bug :-)

> As long as we are restricted by backwards compatibility, it will be hard to find a hack-free solution. The ideal solution IMO is to re-create _warnings.filters from scratch when _warnings is reloaded, but such a change would be safer only for 3.7.

Currently it's not possible to "reload" the _warnings module since it
doesn't use the "module state" API. I don't see how you want to
trigger an action in the _warnings module itself when it is
"reloaded". Filteres are used internally in the _warnings C module.

Maybe we need to enhance the _warnings C module to use the "module
state" API? (Is it the PEP 489?)
History
Date User Action Args
2016-11-22 08:47:16vstinnersetmessages: + msg281444
2016-11-22 01:00:50martin.pantersetsuperseder: Warning -- warnings.filters was modified by test_warnings -> Implement comparison (x==y and x!=y) for _sre.SRE_Pattern
2016-11-22 01:00:50martin.panterunlinkissue28688 superseder
2016-11-22 01:00:20martin.pantersetstatus: open -> closed
superseder: Warning -- warnings.filters was modified by test_warnings
messages: + msg281424

resolution: out of date
stage: resolved
2016-11-22 01:00:20martin.panterlinkissue28688 superseder
2016-11-21 15:52:21vstinnersetmessages: + msg281371
2016-11-21 15:46:28python-devsetnosy: + python-dev
messages: + msg281368
2016-11-18 23:22:35martin.pantersetfiles: + immortal-filters.patch
2016-11-18 23:22:13martin.pantersetmessages: + msg281179
2016-11-18 07:46:24serhiy.storchakasetmessages: + msg281082
2016-11-18 07:44:08vstinnersetmessages: + msg281081
2016-11-18 07:39:27vstinnersetmessages: + msg281080
2016-11-18 07:19:29martin.pantersetmessages: + msg281079
2016-11-17 16:27:15vstinnersetmessages: + msg281050
versions: + Python 3.5, Python 3.6
2016-11-17 16:26:03vstinnersetfiles: + warnings_key.patch
keywords: + patch
messages: + msg281049
2016-11-17 16:03:29vstinnersetmessages: + msg281047
2016-11-17 15:30:53vstinnersetmessages: + msg281042
2016-11-14 17:19:18xiang.zhangsetnosy: + xiang.zhang
messages: + msg280798
2016-11-14 15:36:32vstinnersetmessages: + msg280782
2016-11-14 14:12:25serhiy.storchakasetmessages: + msg280777
2016-11-14 12:56:46vstinnercreate