feat: move artifacts copying to shell launchers - #3600
Merged
Conversation
Full test runs are launched only through the CI shell launchers, so copying of the collected artifacts doesn't need to happen inside the pytest session anymore. Remove the `--artifacts-base-dir` pytest option together with the Python-side copying, and copy the pytest temp dir to the artifacts dir in the CI scripts instead, after pytest exits. This way the artifacts are collected also when the pytest process is killed, e.g. on session timeout. The new runner/copy_artifacts.sh resolves the `pytest-current` symlink, refuses to copy a temp dir that predates the current pytest run, and copies through a hidden staging dir so that consumers of the artifacts dir cannot see a partially copied tree. Saving of cluster artifacts on a dev cluster is expensive and now opt-in via the new FORCE_SAVE_CLUSTER_ARTIFACTS env variable, set only for the node upgrade testing steps that need it. The NO_ARTIFACTS env variable is no longer needed and was removed. The status databases are now numbered by their modification time, as the pytest temp dir numbers cannot be relied on for run ordering.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR shifts artifact collection out of the pytest session and into CI shell launchers, so artifacts can still be collected after pytest exits or is killed (e.g., timeouts). It also makes saving dev-cluster artifacts opt-in to avoid expensive per-invocation log/config copying.
Changes:
- Remove the
--artifacts-base-dirpytest option and Python-side artifact copying; collect artifacts by copying the pytest temp dir after pytest exits. - Add
runner/copy_artifacts.shto safely copy pytest temp dirs into the artifacts directory using a staging dir and staleness checks. - Make dev-cluster artifact saving opt-in via
FORCE_SAVE_CLUSTER_ARTIFACTS, and adjust status-db ordering to use mtimes.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/setup_test_env.sh | Stops exporting NO_ARTIFACTS for dev-cluster setups. |
| runner/status_dbs.sh | Orders status DB copies by mtime and avoids hidden staging dirs. |
| runner/run_tests.sh | Removes artifact-dir handling and the pytest --artifacts-base-dir plumbing. |
| runner/regression.sh | Copies pytest temp dir into artifacts after pytest exits (using stamp guard). |
| runner/node_upgrade_pytest.sh | Enables cluster-artifact saving for upgrade steps; copies artifacts post-pytest. |
| runner/grep_errors.sh | Skips hidden staging dirs when grepping logs for errors. |
| runner/copy_artifacts.sh | New: resolves pytest-current, guards against stale dirs, copies via hidden staging dir. |
| cardano_node_tests/utils/configuration.py | Adds FORCE_SAVE_CLUSTER_ARTIFACTS env toggle. |
| cardano_node_tests/utils/artifacts.py | Removes Python-side copy_artifacts and the artifacts-base-dir constant. |
| cardano_node_tests/tests/conftest.py | Removes pytest option registration and switches dev-cluster artifact saving to opt-in. |
| .source.dev | Removes NO_ARTIFACTS from exported dev environment. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Use tab as the field delimiter of the mtime-sorted find output and read the paths with unmodified IFS. Paths with spaces already worked (`cut -f2-` keeps the separators of the remaining fields), but paths with leading or trailing whitespace would have been mangled by `read`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Full test runs are launched only through the CI shell launchers, so copying of the collected artifacts doesn't need to happen inside the pytest session anymore. Remove the
--artifacts-base-dirpytest option together with the Python-side copying, and copy the pytest temp dir to the artifacts dir in the CI scripts instead, after pytest exits. This way the artifacts are collected also when the pytest process is killed, e.g. on session timeout.The new runner/copy_artifacts.sh resolves the
pytest-currentsymlink, refuses to copy a temp dir that predates the current pytest run, and copies through a hidden staging dir so that consumers of the artifacts dir cannot see a partially copied tree.Saving of cluster artifacts on a dev cluster is expensive and now opt-in via the new FORCE_SAVE_CLUSTER_ARTIFACTS env variable, set only for the node upgrade testing steps that need it. The NO_ARTIFACTS env variable is no longer needed and was removed.
The status databases are now numbered by their modification time, as the pytest temp dir numbers cannot be relied on for run ordering.