-
Notifications
You must be signed in to change notification settings - Fork 1k
unittest/discover: Reverse order of parent/top in sys.path. #872
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Any idea what CPython does here? |
Yeah I wanted to check that myself, it should be trivial for me to make an example test case to demonstrate this |
TLDR: it seems cpython places |
That seems logical enough. Imo micropython should follow that behavior? |
Yep I was just about to remove the parent folder from the path, then remembered why it's there.
The test file is imported here by name and will only be found if its parent dir is on the path. I don't know of there is a different import mechanism / args in micropython that would allow this to work without the folder on path. |
As discussed, see if it's possible to add only |
7d3dbb4
to
2644da4
Compare
I've tested this and yes it does work, in a roundabout fashion!
Dunder-importing the "full dotted path" of a module returns you the base module object, not the final / sub package module you actually want! The thought of then having to iterate through each sub package getattr'ing each one seemed painfully tedious.
I still didn't find this particularly clear, however this gives what we need:
This now works with import path matching cpython behavior, with a unit test added for this subfolder package import behavior. |
2644da4
to
797f835
Compare
ab85cb2
to
c9ebe61
Compare
When running tests from subfolders, import by "full dotted path" rather than just module name, removing the need to add the test parent folder to `sys.path`. This matches CPython more closely, which places `abspath(top)` at the start of `sys.path` but doesn't include the test file parent dir at all. It fixes issues where projects may include a `test_xxx.py` file in their distribution which would (prior to this change) be unintentionally found by unittest-discover. Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
Nice work! |
When running tests from subfolders, import by "full dotted path" rather than just module name, removing the need to add the test parent folder to sys.path.
I found a problem with unittest-discover in a project which uses microdot. A recent update in microdot includes a
test_client.py
in the src folder, which caused it to be pulled in when freezing the module via manifest, and this test file was also then run withunittest discover
.The issue is related to import path ordering. The unit test in question starts with
from microdot.microdot import Request, Response, AsyncBytesIO
which seems reasonable. The microdot package has thismicrodot/microdot.py
file and that import line works fine normally.However currently, when unittest discover finds a test file in a subdirectory
micropython-lib/python-stdlib/unittest-discover/unittest/__main__.py
Line 48 in 50ed36f
it inserts the test file parent folder
path
at the start ofsys.path
, followed bytop
, resulting in an import path like['./microdot', '.', '', '.frozen', '/home/.micropython/lib', '/usr/lib/micropython']
This means from this point, an
import microdot
is finding themicrodot.py
file, not the parentmicrodot
package. Hence,import microdot.microdot
fails.I believe I wrote the existing code with
reversed
on the path insertions by mistake and it's only now I've found a situation that shows the bug. As such I think it makes the most sense to havetop
as the first element on path, espectially as this is a value that can be directly overridden on the command line when runningunittest discover
, so putting it highest priority gives the user the best flexibility.