-
Notifications
You must be signed in to change notification settings - Fork 58
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
allow wildcard in asdf_schema_root #1756
Conversation
18e2ef8
to
143eb2d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure I understood the problem, so I cloned sunpy and tried running the tests in various ways on my laptop (all with --pyargs sunpy):
editable install, in the sunpy directory - schema tests are collected
editable install, in the /tmp/foo directory - schema tests are not collected
regular install, in the sunpy directory - tests fail to start due to ImportPathMismatchError
regular install, in the /tmp/foo directory - schema tests are not collected
I added a breakpoint in pytest_asdf and noticed that collection is failing before the file path check, at this point:
> /Users/eslavich/repos/asdf/pytest_asdf/plugin.py(283)pytest_collect_file()
282 if not (parent.config.getini("asdf_schema_tests_enabled") or parent.config.getoption("asdf_tests")):
--> 283 return None
284
The asdf_schema_tests_enabled
config item returns false because there is no pytest.ini file. Which makes sense, it's not packaged or in the working directory so it's not accessible to pytest. I think I must be missing something -- how could they ever use pytest_asdf?
@@ -276,6 +276,9 @@ def _parse_test_list(content): | |||
|
|||
|
|||
def pytest_collect_file(file_path, parent): | |||
if file_path.suffix != ".yaml": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.yml
is also commonly used, maybe we should let that one through as well?
Thanks for taking a look. Here's a link to a current sunpy CI run (
This is run in the local clone of the sunpy repo which leads pytest to pick up the pytest.ini in the repo. The log shows:
This creates a kind of weird setup where the
So I think the closest match for the tests you ran (thanks for running those!) would be the
since tox runs the following before the tests:
I just tried the following with
|
That's odd... I noticed that they have tox configured to run the tests from a temporary directory instead of the repo root: https://github.com/sunpy/sunpy/blob/main/tox.ini#L17-L18 but tox must know to grab the pytest.ini before it does that? in any case I think you're right that a pattern is the way to go. What do you you think about accepting regex instead of a glob style pattern? It would give people more options if say they have non schema YAML files in their directory tree. That would probably need a new config item, |
I think it's due to the pytest config file search. Since the temporary directory is in the checked out repo pytest will step "up" the tree, back into the repo root and find pytest.ini (since they don't define a
I'll give that one some thought. |
Well I learnt a lot about how tox and |
143eb2d
to
fb88c92
Compare
I'm closing this PR. The pytest plugin lacks unit tests. I think it makes sense to first consider adding tests for the plugin to verify changes (like those in this PR) don't break other features. As this PR was adding a feature that wasn't requested I don't think it's currently worth the risk. |
Description
This PR adds support for starting the
asdf_schema_root
path with a "wildcard" (*
) to allow the pytest plugin to collect schema files when pytest is run with--pyargs
.The sunpy CI currently runs all of it's tests using --pyargs. Even though
asdf_schema_root
is defined in their pytest.ini no schema tests are run. This is due to the asdf pytest plugin expanding theasdf_schema_root
based on the path of the configuration file (in the sunpy case this becomes/home/runner/work/sunpy/sunpy/sunpy/io/special/asdf/resources/
) which then fails thestartswith
check for every schema file (which all start with something like../../.tox/py310-oldestdeps/lib/python3.10/site-packages/sunpy/
). With this PR sunpy can update theirasdf_schema_root
to*/sunpy/io/special/asdf/resources/
which will allow the asdf pytest plugin to collect the schemas for testing.Checklist: