ci: upload integration logs on failure#933
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughIntegration tests record failed reports and preserve selected CCM logs and configuration before cluster cleanup. The CI workflow configures the artifact directory and uploads collected files after pytest failures, using matrix-specific names and 14-day retention. Sequence Diagram(s)sequenceDiagram
participant Pytest
participant Teardown
participant CCM
participant GitHubActions
participant ArtifactStorage
Pytest->>Teardown: report failure
Teardown->>CCM: preserve logs and configuration
Teardown->>CCM: clean up clusters
GitHubActions->>ArtifactStorage: upload preserved artifacts
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/integration-tests.yml (1)
108-114: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winEnsure the glob covers future pytest output.
These patterns only collect selected extensions and files beneath
conf/orlogs/. If redirected pytest output is later written as.out,.txt,.xml, or another format under~/.ccm, it will be omitted. Add the concrete output path/extension or use a narrowly scoped catch-all for the relevant cluster directories.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/integration-tests.yml around lines 108 - 114, Update the artifact collection paths in the workflow’s `path` block to include future pytest output under `/home/runner/.ccm`, either by adding the concrete output extensions/locations or a narrowly scoped catch-all for the relevant cluster directories while preserving the existing patterns.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/integration-tests.yml:
- Line 115: Update the artifact-upload configuration around if-no-files-found so
missing CCM logs are surfaced instead of silently ignored. Change its value to
warn at minimum, or error if CCM artifacts are required for the integration test
workflow.
---
Nitpick comments:
In @.github/workflows/integration-tests.yml:
- Around line 108-114: Update the artifact collection paths in the workflow’s
`path` block to include future pytest output under `/home/runner/.ccm`, either
by adding the concrete output extensions/locations or a narrowly scoped
catch-all for the relevant cluster directories while preserving the existing
patterns.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: aacc41ba-820b-43df-9e49-844ce5c26b61
📒 Files selected for processing (1)
.github/workflows/integration-tests.yml
e90fbd5 to
04829ab
Compare
04829ab to
6f2d4c2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/integration/__init__.py`:
- Around line 115-117: Wrap the shutil.copy2 operation in the file-copying loop
around destination_file with a try/except that catches OSError. Handle copy
failures without propagating them so teardown and cluster cleanup continue even
when source logs are deleted or inaccessible.
- Around line 89-113: Scope preserve_ccm_logs_on_failure to one cluster by
adding optional cluster_name=None and appending it to both ccm_root and
ccm_destination when provided. Update the calls at tests/integration/__init__.py
lines 429, 502, and 519 to pass CCM_CLUSTER.name, cluster_name, and cluster_name
respectively.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 936b8f50-cd50-4034-b18a-a5e6ea830780
📒 Files selected for processing (3)
.github/workflows/integration-tests.ymltests/integration/__init__.pytests/integration/conftest.py
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/integration-tests.yml
6f2d4c2 to
64b4521
Compare
| def preserve_ccm_logs_on_failure(cluster_name=None): | ||
| if not _failed_reports: | ||
| return | ||
|
|
There was a problem hiding this comment.
I don't get the idea here. I checked the docs, pytest_runtest_logreport will be called during each test. That means, after first test failure, _failed_reports will be non-empty. preserve_ccm_logs_on_failure will be called, it will save fail reports and cluster files.
But then, preserve_ccm_logs_on_failure will be called after each test, _failed_reports will still be non-empty, so we will save cluster again and again, and re-save failed-tests.txt.
Am I wrong? What is the idea here?
64b4521 to
525feeb
Compare
Preserve CCM log and config files before integration cleanup clears cluster directories, then upload them as failure-only artifacts for integration test jobs.
525feeb to
724350d
Compare
|
Caution Review failedAn error occurred during the review process. Please try again later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2 participants
Summary
Preserve CCM/Scylla logs for failed integration tests before pytest cleanup clears or removes CCM cluster directories, then upload them as a GitHub Actions artifact.
Changes
tests/integration/conftest.py: records failed pytest reports through the shared integration helper.tests/integration/__init__.py: before cluster cleanup clears/removes clusters, copies CCM logs/config fromtests/integration/ccmintointegration-test-logs/.tests/integration/__init__.py: writesintegration-test-logs/failed-tests.txtso the artifact identifies which test phase/nodeid failed..github/workflows/integration-tests.yml: setsINTEGRATION_LOG_ARTIFACT_DIR=integration-test-logsfor pytest..github/workflows/integration-tests.yml: uploadsintegration-test-logs, any remainingtests/integration/ccmlog/config files, and matching/home/runner/.ccmlog/config files when a matrix job fails..github/workflows/integration-tests.yml: warns if no matching files are found and keeps artifacts for 14 days.Commit Structure
test: preserve failed integration logsci: upload integration logs on failureDriver Surface
CI/test infrastructure only. No driver runtime, protocol, or compatibility behavior changes.
Verification
git diff --check origin/master...HEADruby -e 'require "yaml"; YAML.load_file(".github/workflows/integration-tests.yml")'uv run python -m py_compile tests/integration/__init__.py tests/integration/conftest.pypreserve_ccm_logs_on_failure()against a temp fake CCM tree withSCYLLA_VERSION=release:2026.1actionlintis not installed in the local environment.Fixes: #932
Pre-review checklist
./docs/source/.Fixes:annotations to PR description.