classification
Title: test___all__ affects other tests by doing too much importing
Type: behavior Stage: needs patch
Components: Tests Versions: Python 3.2, Python 3.3, Python 3.4
process
Status: open Resolution:
Dependencies: 1674555 Superseder: Create a way to always run tests in subprocesses within regrtest
View: 18906
Assigned To: eli.bendersky Nosy List: Arfrever, asvetlov, danielsh, eli.bendersky, eric.snow, ezio.melotti, mcepl, r.david.murray
Priority: normal Keywords:

Created on 2012-12-30 00:56 by eli.bendersky, last changed 2018-07-21 07:31 by mcepl.

Messages (11)
msg178547 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-12-30 00:56
http://mail.python.org/pipermail/python-dev/2012-December/123368.html

This came up while investigating some test-order-dependency failures in
issue 16076.

test___all__ goes over modules that have `__all__` in them and does 'from
<module> import *' on them. This leaves a lot of modules in sys.modules,
which may interfere with some tests that do fancy things with sys,modules.
In particular, the ElementTree tests have trouble with it because they
carefully set up the imports to get the C or the Python version of etree
(see issues 15083 and 15075).

Would it make sense to save the sys.modules state and restore it in
test___all__ so that sys.modules isn't affected by this test?
msg178551 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-12-30 05:11
As Nick pointed out in the thread, getting the correct module imports for C and python versions is what import_fresh_module in test.support is for.
msg178553 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-12-30 05:42
Yes, but pickle doesn't use this helper function. And the problem is because *pickle* tries to __import__ this module.
msg178574 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-12-30 13:56
Hmm.  What if we made import_fresh_module a context manager, so that the restore of the original module in sys.modules only happened at the end of the context?
msg178575 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-12-30 14:02
David, how would this help pickle not find _elementtree though? It's already in sys.modules *before* fresh_import is used, because test___all__ put it there.

I'm experimenting with just deleting _elementtree from sys.modules before running the tests, so far with little success.
msg178580 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-12-30 14:40
It would help because import_fresh_module would have updated sys.modules to be the module under test, and would not restore the previously existing entry in sys.modules until after your test had completed.
msg178620 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-12-30 22:09
I'm renaming the issue to just mention the problem, not the initially proposed solution (following the python-dev discussion).
msg180642 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-01-26 05:51
So the current solution is to temporarily put the relevant module in place in sys.modules, right?  That seems to be the solution that Stefan recommended and used in the decimal module.  Sounds good to me.

I'm hitting this while doing the PEP 399 two-step for the collections module.  It seems like this will be a problem for testing any module that has had such attention and has __all__.  I'd be a fan of a class decorator that would take care of this and the rest of the PEP 399 stuff for you.

I've created issue #17037 to cover that (so it doesn't get muddled in with this discussion of test___all__).  There's even a proposed patch.
msg180669 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-01-26 13:46
Eric, yes the key code in test_xml_etree that handles this is:

    def pickleRoundTrip(self, obj, name, dumper, loader):
        save_m = sys.modules[name]
        try:
            sys.modules[name] = dumper
            temp = pickle.dumps(obj)
            sys.modules[name] = loader
            result = pickle.loads(temp)
        except pickle.PicklingError as pe:
            # pyET must be second, because pyET may be (equal to) ET.
            human = dict([(ET, "cET"), (pyET, "pyET")])
            raise support.TestFailed("Failed to round-trip %r from %r to %r"
                                     % (obj,
                                        human.get(dumper, dumper),
                                        human.get(loader, loader))) from pe
        finally:
            sys.modules[name] = save_m
        return result

Because pickle does its own import of ElementTree.

The test___all__ problem should be solved by a patch to issue #1674555, but having a separate mechanism in test.support was discussed. I'll take a look at your decorator.
msg199202 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-10-08 13:16
This is superceded by:

http://bugs.python.org/issue18906
msg322081 - (view) Author: Matej Cepl (mcepl) * Date: 2018-07-21 07:31
> This is superceded by:
> 
> http://bugs.python.org/issue18906

Then it should be closed, right?
History
Date User Action Args
2018-07-21 07:31:03mceplsetnosy: + mcepl
messages: + msg322081
2013-10-08 13:16:19eli.benderskysetsuperseder: Create a way to always run tests in subprocesses within regrtest
messages: + msg199202
2013-01-27 01:08:45ezio.melottisetnosy: + ezio.melotti
2013-01-26 13:46:48eli.benderskysetmessages: + msg180669
2013-01-26 05:51:00eric.snowsetnosy: + eric.snow
messages: + msg180642
2013-01-05 15:07:34eli.benderskysetdependencies: + sys.path in tests contains system directories
2012-12-30 22:09:45eli.benderskysettitle: test___all__ has to save and restore sys.modules while it does all the importing -> test___all__ affects other tests by doing too much importing
2012-12-30 22:09:22eli.benderskysetmessages: + msg178620
2012-12-30 21:56:08Arfreversetnosy: + Arfrever
2012-12-30 14:40:07r.david.murraysetmessages: + msg178580
2012-12-30 14:02:43eli.benderskysetmessages: - msg178576
2012-12-30 14:02:23eli.benderskysetmessages: + msg178576
2012-12-30 14:02:21eli.benderskysetmessages: + msg178575
2012-12-30 13:56:59r.david.murraysetmessages: + msg178574
2012-12-30 05:42:53eli.benderskysetmessages: + msg178553
2012-12-30 05:11:24r.david.murraysetnosy: + r.david.murray
messages: + msg178551
2012-12-30 01:05:20asvetlovsetnosy: + asvetlov
2012-12-30 00:58:41danielshsetnosy: + danielsh
2012-12-30 00:56:09eli.benderskycreate