Issues with import_fresh_module
As part of PEP 399 <https://www.python.org/dev/peps/pep-0399/>, an idiom for testing both C and pure Python versions of a library is suggested making use if import_fresh_module. Unfortunately, I'm finding that this is not amazingly robust. We have this issue: https://bugs.python.org/issue40058, where the tester for datetime needs to do some funky manipulations <https://github.com/python/cpython/blob/302e5a8f79514fd84bafbc44b7c97ec636302322/Lib/test/test_datetime.py#L14-L23>to the state of sys.modules for reasons that are now somewhat unclear, and still sys.modules is apparently left in a bad state. When implementing PEP 615, I ran into similar issues and found it very difficult to get two independent instances of the same module – one with the C extension blocked and one with it intact. I ended up manually importing the C and Python extensions and grafting them onto two "fresh" imports with nothing blocked <https://github.com/pganssle/zoneinfo/blob/ffd21a6d065e04725e04b37bb430c2559f...>. This seems to work most of the time in my repo, but when I import it into CPython, I'm now seeing failures due to this issue. The immediate symptom is that assertRaises is seeing a mismatch between the exception raised by the module and the exception *on* the module. Here's the Travis error <https://travis-ci.org/github/python/cpython/jobs/683080079#L3154-L3186> (ignore the part about `tzdata`, that needs to be removed as misleading), and here's the test <https://github.com/python/cpython/pull/19909/files#diff-885914f7d01a0c33a25e...>. Evidently calling module.ZoneInfo("Bad_Zone") is raising a different module's ZoneInfoNotFoundError in some cases and I have no idea why. Is anyone familiar more familiar with the import system willing to take a look at these issues? Thanks, Paul
I'm drowning in work this month, so if you need me to look at something then I unfortunately need a point-blank link of what you want me to look at with a targeted question. As for import_fresh_module() not being robust: of course it isn't because it's mucking with import stuff in a very non-standard way. 😉 All it's doing is an import and clearing the module from sys.modules. The extras it provides is to shove None into sys.modules to trigger an ImportError and so you can block any acceleration module from being imported and to forcibly use the Python code. That's it.
No worries, I actually seem to have solved the immediately pressing problem that was blocking PEP 615 by doing this: @functools.lru_cache(1) def get_modules(): import zoneinfo as c_module py_module = import_fresh_module("zoneinfo", blocked=["_czoneinfo"]) return py_module, c_module I'll have to dig in to figure out exactly /why/ that works, and why it /doesn't/ work in the reference implementation (which has the the C implementation living at `zoneinfo._czoneinfo` instead of at `_czoneinfo`), and hopefully that will shed some light on the other issues. For the moment I've got something that appears to work and a suggestive pattern of behavior as to why it wasn't working, so that actually seems like it will help me solve my short term goal of getting zoneinfo merged ASAP and my long term goal of ensuring that the tests are robust. Thanks! Paul On 5/6/20 3:55 PM, Brett Cannon wrote:
I'm drowning in work this month, so if you need me to look at something then I unfortunately need a point-blank link of what you want me to look at with a targeted question.
As for import_fresh_module() not being robust: of course it isn't because it's mucking with import stuff in a very non-standard way. 😉 All it's doing is an import and clearing the module from sys.modules. The extras it provides is to shove None into sys.modules to trigger an ImportError and so you can block any acceleration module from being imported and to forcibly use the Python code. That's it. _______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-leave@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/VNQJBFHI... Code of Conduct: http://python.org/psf/codeofconduct/
Maybe an initialization/import side-effect bug which is triggered if the module is imported twice?
To expand on my earlier comment about changing the module under test to make your testing easier, asyncio is one library that has lots of tests of different combinations of its C and Python implementations being used together. As far as I know, it doesn't use import_fresh_module or similar hackery. Instead it exposes a private way of getting at the parallel Python implementation: https://github.com/python/cpython/blob/b7a78ca74ab539943ab11b5c4c9cfab7f5b7f... This is the kind of thing I was suggesting. (It might require more setup than this in your case.) --Chris On Thu, May 7, 2020 at 11:33 AM Brett Cannon <brett@python.org> wrote:
Maybe an initialization/import side-effect bug which is triggered if the module is imported twice? _______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-leave@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/25XFYLIS... Code of Conduct: http://python.org/psf/codeofconduct/
On Wed, May 6, 2020 at 7:52 AM Paul Ganssle <paul@ganssle.io> wrote:
As part of PEP 399, an idiom for testing both C and pure Python versions of a library is suggested making use if import_fresh_module.
Unfortunately, I'm finding that this is not amazingly robust. We have this issue: https://bugs.python.org/issue40058, where the tester for datetime needs to do some funky manipulations to the state of sys.modules for reasons that are now somewhat unclear, and still sys.modules is apparently left in a bad state.
When implementing PEP 615, I ran into similar issues and found it very difficult to get two independent instances of the same module – one with the C extension blocked and one with it intact. I ended up manually importing the C and Python extensions and grafting them onto two "fresh" imports with nothing blocked.
When I've had to deal with similar issues in the past, I've given up on messing with sys.modules and just had one test spawn a subprocess to do the import+run the actual tests. It's a big hammer, but the nice thing about big hammers is that there's no subtle issues, either they smash the thing or they don't. But, I don't know how awkward that would be to fit into Python's unittest system, if you have lots of tests you need to run this way. -n -- Nathaniel J. Smith -- https://vorpus.org
Thanks for the suggestion. I think I tried something similar for tests that involved an environment variable and found that it doesn't play nicely with coverage.py /at all/. Also, I will have to solve this problem at some point anyway because the property tests for the module (not currently included in the PR) include tests that have the C and pure Python version running side-by-side, which would be hard to achieve with subinterpreters. On 5/6/20 4:51 PM, Nathaniel Smith wrote:
As part of PEP 399, an idiom for testing both C and pure Python versions of a library is suggested making use if import_fresh_module.
Unfortunately, I'm finding that this is not amazingly robust. We have this issue: https://bugs.python.org/issue40058, where the tester for datetime needs to do some funky manipulations to the state of sys.modules for reasons that are now somewhat unclear, and still sys.modules is apparently left in a bad state.
When implementing PEP 615, I ran into similar issues and found it very difficult to get two independent instances of the same module – one with the C extension blocked and one with it intact. I ended up manually importing the C and Python extensions and grafting them onto two "fresh" imports with nothing blocked. When I've had to deal with similar issues in the past, I've given up on messing with sys.modules and just had one test spawn a subprocess to do the import+run the actual tests. It's a big hammer, but the nice
On Wed, May 6, 2020 at 7:52 AM Paul Ganssle <paul@ganssle.io> wrote: thing about big hammers is that there's no subtle issues, either they smash the thing or they don't.
But, I don't know how awkward that would be to fit into Python's unittest system, if you have lots of tests you need to run this way.
-n
On Wed, May 6, 2020 at 2:34 PM Paul Ganssle <paul@ganssle.io> wrote:
I think I tried something similar for tests that involved an environment variable and found that it doesn't play nicely with coverage.py at all.
This is a solvable problem: https://coverage.readthedocs.io/en/coverage-5.1/subprocess.html But yeah, convincing your test framework to jump through the necessary hoops might be tricky. (Last time I did this I was using pytest-cov, which automatically takes care of all the details, so I'm not sure how tough it is.) -n -- Nathaniel J. Smith -- https://vorpus.org
Have you also considered changes to the modules under test that might make it easier for both implementations to exist and be tested side-by-side (so with fewer hacks on the testing side)? —Chris On Wed, May 6, 2020 at 2:33 PM Paul Ganssle <paul@ganssle.io> wrote:
Thanks for the suggestion.
I think I tried something similar for tests that involved an environment variable and found that it doesn't play nicely with coverage.py *at all*.
Also, I will have to solve this problem at some point anyway because the property tests for the module (not currently included in the PR) include tests that have the C and pure Python version running side-by-side, which would be hard to achieve with subinterpreters.
On 5/6/20 4:51 PM, Nathaniel Smith wrote:
On Wed, May 6, 2020 at 7:52 AM Paul Ganssle <paul@ganssle.io> <paul@ganssle.io> wrote:
As part of PEP 399, an idiom for testing both C and pure Python versions of a library is suggested making use if import_fresh_module.
Unfortunately, I'm finding that this is not amazingly robust. We have this issue: https://bugs.python.org/issue40058, where the tester for datetime needs to do some funky manipulations to the state of sys.modules for reasons that are now somewhat unclear, and still sys.modules is apparently left in a bad state.
When implementing PEP 615, I ran into similar issues and found it very difficult to get two independent instances of the same module – one with the C extension blocked and one with it intact. I ended up manually importing the C and Python extensions and grafting them onto two "fresh" imports with nothing blocked.
When I've had to deal with similar issues in the past, I've given up on messing with sys.modules and just had one test spawn a subprocess to do the import+run the actual tests. It's a big hammer, but the nice thing about big hammers is that there's no subtle issues, either they smash the thing or they don't.
But, I don't know how awkward that would be to fit into Python's unittest system, if you have lots of tests you need to run this way.
-n
_______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-leave@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/H4TWK574... Code of Conduct: http://python.org/psf/codeofconduct/
The subtle issue is of course performance if you get too cozy with this pattern... On Wed, May 6, 2020 at 1:59 PM Nathaniel Smith <njs@pobox.com> wrote:
On Wed, May 6, 2020 at 7:52 AM Paul Ganssle <paul@ganssle.io> wrote:
As part of PEP 399, an idiom for testing both C and pure Python versions
of a library is suggested making use if import_fresh_module.
Unfortunately, I'm finding that this is not amazingly robust. We have
this issue: https://bugs.python.org/issue40058, where the tester for datetime needs to do some funky manipulations to the state of sys.modules for reasons that are now somewhat unclear, and still sys.modules is apparently left in a bad state.
When implementing PEP 615, I ran into similar issues and found it very
difficult to get two independent instances of the same module – one with the C extension blocked and one with it intact. I ended up manually importing the C and Python extensions and grafting them onto two "fresh" imports with nothing blocked.
When I've had to deal with similar issues in the past, I've given up on messing with sys.modules and just had one test spawn a subprocess to do the import+run the actual tests. It's a big hammer, but the nice thing about big hammers is that there's no subtle issues, either they smash the thing or they don't.
But, I don't know how awkward that would be to fit into Python's unittest system, if you have lots of tests you need to run this way.
-n
-- Nathaniel J. Smith -- https://vorpus.org _______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-leave@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/SDSODK5Z... Code of Conduct: http://python.org/psf/codeofconduct/
-- --Guido van Rossum (python.org/~guido) *Pronouns: he/him **(why is my pronoun here?)* <http://feministing.com/2015/02/03/how-using-they-as-a-singular-pronoun-can-c...>
participants (5)
-
Brett Cannon -
Chris Jerdonek -
Guido van Rossum -
Nathaniel Smith -
Paul Ganssle