Skip to content

tests/run-tests: Run tests inside a temporary chdir. #6909

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

Closed
wants to merge 2 commits into from

Conversation

jimmo
Copy link
Member

@jimmo jimmo commented Feb 16, 2021

I think this is a good idea just for making sure the tests run in a self-contained environment, and possibly we should consider a unique temp dir per test invocation.

However, this is specifically to workaround a situation where I want to write a test for uasyncio (#6885), where uasyncio now does "import io", but this ends up importing tests/io instead.

I've tried to ensure that this doesn't break anything from #5877 (@stinos ), and in particular that running this from a out-of-tree directory continues to work.

Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
@jimmo jimmo force-pushed the tests-run-in-chdir branch 3 times, most recently from 812e25f to 71d0814 Compare February 16, 2021 02:45
@jimmo
Copy link
Member Author

jimmo commented Feb 16, 2021

Updated to use a unique temp dir per invocaton. This is to allow copying required data files into the test dir. Any test in a directory that has a "data" directory will have this copied into the working dir.

@jimmo jimmo force-pushed the tests-run-in-chdir branch from 71d0814 to 8b6310a Compare February 16, 2021 02:54
Any required data files are copied into the working dir.

This prevents "import foo" from finding tests/foo, for example "io".
Individual tests could previously work around this by importing "uio"
instead, but tests can now depend on other libraries (e.g. uasyncio).

Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
@jimmo jimmo force-pushed the tests-run-in-chdir branch from 8b6310a to 831d0c0 Compare February 16, 2021 03:30
def base_path(*p):
return os.path.abspath(os.path.join(BASEPATH, *p)).replace('\\', '/')
return os.path.abspath(os.path.join(BASEPATH, *p)).replace("\\", "/")

Copy link
Contributor

@stinos stinos Feb 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you add these newlines, or black? (just wondering)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

black -- The first commit is entirely black's changes (nothing from me).

@dpgeorge dpgeorge added the tests Relates to tests/ directory in source label Nov 30, 2021
@jimmo
Copy link
Member Author

jimmo commented Apr 21, 2022

The underlying issue here was that run-tests.py used to do:

os.environ["MICROPYPATH"] = os.pathsep + base_path("../extmod")

which was intended to make it possible to import frozen modules (and extmod modules), but had the additional undesirable effect of including TOP/tests in the module search path.

But now #8079 allows us to specifically add frozen modules to the path without using "", so this fix is no longer required.

@jimmo jimmo closed this Apr 21, 2022
Wind-stormger pushed a commit to BPI-STEAM/micropython that referenced this pull request Oct 13, 2022
…unneeded-code

Remove unneeded workaround for Feather ESP32-S2 TFT
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Relates to tests/ directory in source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants