Make regression-tests workflow data check non-fatal#236
Conversation
The pre-check required every entry in tests/testdata.yaml to be present on the runner, but the test framework already skips missing testdata via the require_testdata* decorators. With SCIDA_TESTDATA_SILENT_UNAVAILABLE=FALSE and pytest -rs, those skips are visible in the report. Keep the check as diagnostic output (which entries are missing) but stop failing the job when entries are absent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3b98878b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if missing: | ||
| print("Missing regression test data entries:") | ||
| print(f"Missing {len(missing)}/{len(entries)} regression test data entries (tests will be skipped):") | ||
| print("\n".join(missing)) | ||
| raise SystemExit(1) | ||
| else: |
There was a problem hiding this comment.
In .github/workflows/regression-tests.yml, the if missing: branch now only logs and never exits non-zero, so the job can report success even when required regression data is absent. I checked this workflow context and verified that pytest returns exit code 0 when tests are only skipped, which means a broken or partially mounted dataset path can silently produce a green regression run with no meaningful coverage. Keep the diagnostic output, but preserve a failing condition when the missing-count exceeds an acceptable threshold (or when all entries are missing).
Useful? React with 👍 / 👎.
Address Codex review feedback on #236: a fully non-fatal check lets a broken/unmounted volume produce a green regression run, since pytest exits 0 when every test is skipped. Keep individual gaps non-fatal (handled via SCIDA_TESTDATA_SILENT_UNAVAILABLE=FALSE + pytest -rs) but fail loudly when the majority of entries are missing, which is the signature of a broken mount rather than legitimately absent data. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Makes the testdata presence check in the regression-tests workflow non-fatal for individual missing entries, while still failing if a majority are missing (to catch broken mounts). Missing entries are surfaced as pytest skips instead.
Changes:
- Replaces hard failure on any missing testdata entry with diagnostic output
- Adds a safety threshold: aborts if more than half of entries are missing
- Adds explanatory comment about the rationale
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Follow-up to #235. The pre-run check in
.github/workflows/regression-tests.ymlrequired every entry intests/testdata.yamlto be present on the self-hosted runner and failed the job on any missing entry, which caused the first scheduled run to fail before pytest ever started.Missing testdata is already handled at the test level by the
require_testdata*decorators intests/testdata_properties.py. WithSCIDA_TESTDATA_SILENT_UNAVAILABLE=FALSEandpytest -rs, absences surface as visible skips in the report.This PR keeps the check as diagnostic output (which entries are missing, with a count) but no longer fails the job.
Test plan
workflow_dispatchand confirm the job proceeds past the data-check step when entries fromtests/testdata.yamlare absentpytest -rssummary lists the skipped tests with reasons🤖 Generated with Claude Code