Add testing to CI flow #418
Conversation
for more information, see https://pre-commit.ci
… add-testing-to-ci
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
@pww217 Thanks for the ping about this, I think it's fine at the moment while we don't have automated API documentation.
In case we start using Sphinx' ability to automatically render API reference documentation using the autodoc extension we'd have to remove the limitation of the workflows to also include the regular source code since otherwise changes to function signatures, docstrings etc wouldn't be picked up until anything in docs/* would be changed. Since we haven't really discussed that type of documentation yet, it's totally fine limiting the CI runs like proposed in this PR. Thanks!
@kathatherine @fpliger @idanenglander FYI, there is a Sphinx integration to automatically create API docs for JavaScript files based on the standard JSDoc formatting: https://github.com/mozilla/sphinx-js#use
|
So now that we have conda in the path of the runner, can we talk about the CONDA_EXE env var @fpliger? There's no default value set in the makefile. Since we want this makefile to be usable locally as well as in CI, do we expect local users to have conda in their PATH? Not sure how to best handle this. Phrases another way: what is the friendliest way to handle the binary/PATH issue so as to cause users the least pain? Or maybe we never expect people to use any conda commands really when building locally, in which case it doesn't matter. Just want to align on this. |
| @@ -6,14 +6,15 @@ src_dir ?= $(base_dir)/src | |||
| examples ?= $(base_dir)/examples | |||
| app_dir ?= $(shell git rev-parse --show-prefix) | |||
|
|
|||
| CONDA_EXE := conda | |||
There was a problem hiding this comment.
@jezdez do you have any opinions about this? Is that fine from a pure conda perspective?
There was a problem hiding this comment.
For context, it's basically just conda from PATH as the setup-conda action allows, but this variable existed in the makefile previously and was undefined.
* new testing ci flow * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install conda * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * test conda * pre-commit fix * remove if * makefile fix * fix pytest install * argument for test * restructure * fix sync * full path filter * use setup-condav2 * syntax * test path * add binary argument * remove which * Update Makefile * remove arguments * trigger CI Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* new testing ci flow * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install conda * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * test conda * pre-commit fix * remove if * makefile fix * fix pytest install * argument for test * restructure * fix sync * full path filter * use setup-condav2 * syntax * test path * add binary argument * remove which * Update Makefile * remove arguments * trigger CI Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
See #416
Working for latest here
Will need to test for Alpha on next run.
I tried to limit major changes to the makefile so it's still useful locally. Instead I pass arguments from the CI. It should work locally still, but you may want to test that. Main change is that
make setupwill now include preparing pytest and conda, andmake testwill use them to run automated tests.@jezdez a small, unrelated change is included that adds path filtering for the doc workflows. The reason is so they don't run on every push, just when docs are modified. Let me know if that's no good.