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.

How you got this warning? I can't reproduce.

> 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

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.

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

> * 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".

> * 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 :-(

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.

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).

> The key is based on (action, message, category, module). I think you should add item[4] (lineno).

Oops, right!

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.

Your plan LGTM.

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.

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

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.

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 :)

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?)