Cleanup sys.modules after a Hy specific import failure. #1524
Conversation
When there is a HyTypeError or a LexException, the None placeholder inserted in sys.modules is not removed. This prevents future import attempts because the Python import machinery finds the None reference during the "fast path" of the default importer. This fixes #1523.
|
Close. The hard parts are done, the branch works. I just need to revert some of my experiments (patching But I got a busy week ahead of me with renovations, so I'll probably not have too much programming time. I'd have no problem merging this in the meanwhile. No point making this painful when the fix is just a few lines. And on that note, if you guys decide to merge this in the interim, have a look at #1513 as well, @etanol, and add that to this PR. Might as well work around two issues at once. |
| "Test that a failed import does not prevents from re trying" | ||
| module_name = "again" | ||
|
|
||
| f = open("again.hy", "wb") |
There was a problem hiding this comment.
@Kodiologist what would you think about starting to use the pytester plugin for this kind of stuff? Comes stock with pytest for this kind of stuff, just needs to be turned on:
def test_import_failure_retryable(testdir):
testdir.makefile("hy", again="""(import sys)""")
testdir.syspathinsert() # magically cleans up post test too
import againKeeps current working directory clean and makes sure everything is fresh each pytest run. I suspect stale .pyc files could cause a false pass here.
There was a problem hiding this comment.
I'm fine with using pytest plugins, sure; just make sure everything works.
There was a problem hiding this comment.
Okay, in which case pytest_plugins = "pytester" needs to go in our conftest.py and its loaded.
| f.write(b'(import "sys")') | ||
| f.flush() | ||
|
|
||
| assert _import_test() == "Failed" |
There was a problem hiding this comment.
Probably better to do:
with pytest.raises(HyTypeError):
import again
# ...
import again
|
Dropping in favor of #1518 |
When there is a HyTypeError or a LexException, the None placeholder inserted in
sys.modules is not removed. This prevents future import attempts because the
Python import machinery finds the None reference during the "fast path" of the
default importer.
This fixes #1523.