Add optional Assimulo integration CI#94
Conversation
Add initial core helper smoke tests
bernalde
left a comment
There was a problem hiding this comment.
Review for b7f53d9152acb6c6ea0f05b4e50842a90497fdca.
Blocking issues:
- Blocking: Update the new Assimulo CI lane off deprecated action/runtime inputs before merge. See the inline comment on
.github/workflows/ci.ymlfor the exact GitHub annotations and replacement inputs.
I would not merge this until the blocking issues above are addressed.
Nonblocking issues:
- None.
Questions:
- None.
Tests run and outcomes:
- Local PR-head:
pytest --collect-only -q-> 12 tests collected. - Local PR-head:
pytest tests/ -m 'not assimulo' -q -rs-> 6 passed, 6 deselected. - Local PR-head:
pytest tests/ -m assimulo -q -rs-> 6 skipped, 6 deselected because Assimulo is not installed locally. - Local PR-head:
python -m build --sdist --no-isolation-> passed; the sdist includedTESTING.mdandenvironment.yml. - GitHub PR-head checks for
b7f53d9: CI pull_request run28589354850and push run28589351179both completed successfully forCore testsandAssimulo integration tests; the pull-request run has the CI annotations described in the inline finding.
Merge-readiness:
- The PR is still a draft, so this COMMENT review does not satisfy branch-protection approval. GitHub reports
mergeStateStatusasCLEAN; branch protection formasterreturnedBranch not protected;reviewDecisionis empty. This account is the PR author, so it cannot provide formal approval or requested-changes review if one is needed after the draft and blocking CI issue are cleared.
| sudo apt-get install -y gfortran liblapack-dev libblas-dev | ||
|
|
||
| - name: Set up conda environment | ||
| uses: conda-incubator/setup-miniconda@v3 |
There was a problem hiding this comment.
Blocking: This new CI lane is still using deprecated action/runtime inputs. The current PR-head run for b7f53d9 annotates actions/checkout@v4 and conda-incubator/setup-miniconda@v3 as Node.js 20 actions being forced onto Node.js 24, and it also warns that auto-activate-base is deprecated and that defaults may be added implicitly. Since #6 asks for a stable conda-forge Assimulo lane, please update this job to the current workflow conventions (actions/checkout@v5, conda-incubator/setup-miniconda@v4, the replacement activation input) and remove implicit defaults, for example with conda-remove-defaults: true, before merging. Rebasing on current master will also bring the core job back to checkout@v5/setup-python@v6 for PR-head checks.
There was a problem hiding this comment.
Addressed in a2f1bbc: the workflow now uses checkout@v5, setup-python@v6, setup-miniconda@v4, auto-activate, and conda-remove-defaults: true, while preserving the current master permissions/concurrency hardening. The current pull_request CI run 28606044218 passed without the prior deprecation annotations.
|
Addressed the review feedback on the CI action/runtime deprecations. Commits pushed:
Main changes:
Tests and checks:
Comments intentionally not addressed:
Remaining:
|
Summary
Traceability
Closes #6.
Built on the merged #93 / #9 packaging and dependency-bounds work.
Refs #1, #2, CryPTSys#106, and CryPTSys#107.
Verification
Actions
GitHub Actions now runs both the core job and the informational Assimulo job. The Assimulo job remains marked continue-on-error: true until the conda solver stack is proven stable across follow-up runs.