Skip to content

tests/run-tests.py: Ensure correct cwd for mpy tests. #11468

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

Merged
merged 1 commit into from
May 18, 2023

Conversation

jimmo
Copy link
Member

@jimmo jimmo commented May 11, 2023

This is necessary for #11456 which currently fails the --via-mpy tests due import micropython now attempting to import tests/micropython, but I think it's overall a good fix to improve consistency anyway.

Previously when using --via-mpy, the file was compiled to tests/.mpy and then run using micropython -m <tmp> in the current cwd (usually tests/). This meant that an import in the test would be resolved relative to tests/.

This is different to regular (non-via-mpy) tests, where we run (for example) micropython basics/test.py which means that an import would be resolved relative to basics/.

Now --via-mpy matches the .py behavior. This is important because: a) It makes it so import tests do the right thing. b) There are directory names in tests/ that match built-in module names.

Furthermore, it always ensures the cwd (for both micropython and cpython) is the test directory (e.g. basics/) rather than being left unset. This also makes it clearer inside the test that e.g. file access is relative to the Python file. Updated tests with file paths to match.

This work was funded through GitHub Sponsors.

@jimmo
Copy link
Member Author

jimmo commented May 11, 2023

Related to #6909 which I still like the idea of (because it's easier to reason about a test that runs in an isolated environment), but I don't think is necessary.

@dpgeorge dpgeorge added the tests Relates to tests/ directory in source label May 11, 2023
@dpgeorge
Copy link
Member

Thanks, this looks good to me.

@jimmo
Copy link
Member Author

jimmo commented May 12, 2023

I don't think this is strictly necessary for #11456 any more since we decided to make import micropython bypass the filesystem, but I still think it's a worthwhile change.

Previously when using --via-mpy, the file was compiled to tests/<tmp>.mpy
and then run using `micropython -m <tmp>` in the current cwd
(usually tests/).  This meant that an import in the test would be resolved
relative to tests/.

This is different to regular (non-via-mpy) tests, where we run (for
example) `micropython basics/test.py` which means that an import would be
resolved relative to basics/.

Now --via-mpy matches the .py behavior.  This is important because:
a) It makes it so import tests do the right thing.
b) There are directory names in tests/ that match built-in module names.

Furthermore, it always ensures the cwd (for both micropython and cpython)
is the test directory (e.g. basics/) rather than being left unset.  This
also makes it clearer inside the test that e.g. file access is relative to
the Python file.

Updated tests with file paths to match.

This work was funded through GitHub Sponsors.

Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
@dpgeorge dpgeorge force-pushed the tests-via-mpy-cwd branch from c4da44c to 17127bb Compare May 18, 2023 03:53
@dpgeorge dpgeorge merged commit 17127bb into micropython:master May 18, 2023
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.

2 participants