-
Notifications
You must be signed in to change notification settings - Fork 306
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
Use nox
as a test runner instead of tox
#1735
Conversation
This is very close to the noxfile.py that I added for XRTpy.
This also came from what I added previously to XRTpy.
Yay it's doing stuff! An issue it's running into now is that it's not finding Python 3.9 for macOS, which is also happening on XRTpy (though it did work in the not-too-distant past, I think). |
.github/workflows/testing.yml
Outdated
uses: codecov/codecov-action@v3 | ||
with: | ||
file: ./coverage.xml | ||
python-version: '3.10' |
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.
This line needs reverting - as it is, it's installing 3.10 in all three currently attempted test envs. 3.8 Seems to already be there for 3.8/Windows, which is why that one isn't failing. I suspect it should fix the 3.9/MacOS failure
so that the python version matrix actually has an effect!
Thank you for contributing to PlasmaPy! The project's future depends deeply on contributors like you, so we deeply appreciate it! 🌱 The following checklist will be used by the code reviewer to help guide the code review process.
|
Based on https://github.com/PlasmaPy/PlasmaPy/actions/runs/3039593035/jobs/4894695172#step:6:22, it looks like the docs environment is failing because it can't import `plasmapy.utils.pytest_helpers`. If so, I guess we should include `pytest` as a dependency of the [docs] extra. Hell, we could even do, in `requirements/docs.txt`, ``` sphinx pyramid hieroglyph ... -r tests.txt ``` or some such, so that `tests` is strictly a subset of `docs`. But getting just pytest in is probably faster on the CI executor.
I hope you don't mind me tinkering with your branch a little 😅 see comment on the latest commit for my reasoning! |
Not a problem! I might not have a chance to look into this for a few days, so please tinker away! |
sphinx_no_notebooks = ["-D", "nbsphinx_execute=never"] | ||
sphinx_nitpicky = ["-n"] | ||
|
||
pytest_options = ["--showlocals"] |
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.
- Make sure to keep all of the options that are currently in
tox.ini
pre-commit.ci autofix |
Hm...there's still an issue with one test for coverage that's seemingly not being run:
|
The main testing functionality I still need to cross over:
Other stuff...
|
Before marking this as ready for review, I'll need to:
|
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.
If we're doing this in halves, then it might make sense to merge this to a feature branch first. Then I'll work off that before we put this on main.
@@ -233,6 +233,7 @@ nest-asyncio==1.5.6 | |||
# jupyter-client | |||
nodeenv==1.7.0 | |||
# via pre-commit | |||
nox |
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.
Remember that you should pin a version here! The command to do that is up top, but it won't work until you add nox to pyproject.toml
first.
|
||
|
||
@nox.session | ||
def build_docs_no_examples(session): |
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.
def build_docs_no_examples(session): | |
def build_docs_no_examples(session): # TODO: "parametrize" this? the same |
I would absolutely not want to hold this up, but I'm curious, and I just want to make sure I notice this when picking it up later 😉
sphinx_opts = sphinx_paths + sphinx_fail_on_warnings + sphinx_builder | ||
sphinx_skip_notebooks = ["-D", "nbsphinx_execute=never"] | ||
|
||
post_doc_build_comments = """ |
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 half expect to see this become tenured_doc_build_comments
later.
This PR adds a prototype testing configuration with
nox
instead oftox
. As described in #1734, the motivations for usingnox
are:The configuration file is
noxfile.py
, which I'm adapting from the test setup for XRTpy.Closes #1734.