Skip to content

ci: clean stale .out files after checkout#1308

Merged
sbryngelson merged 1 commit intoMFlowCode:masterfrom
sbryngelson:frontier-runner
Mar 14, 2026
Merged

ci: clean stale .out files after checkout#1308
sbryngelson merged 1 commit intoMFlowCode:masterfrom
sbryngelson:frontier-runner

Conversation

@sbryngelson
Copy link
Copy Markdown
Member

Summary

  • Delete stale .out files after checkout in both the test suite and case-optimization jobs
  • Prevents the Print Logs step (if: always()) from showing output from a previous workflow run when the current run fails early
  • Leaves .slurm_job_id files intact so submit-slurm-job.sh can still detect and cancel stale SLURM jobs

Context

When a multi-step CI job (like case-optimization) fails at the prebuild step, the "Run Case-Optimization Tests" step is skipped. But the Print Logs step runs unconditionally and cats whatever .out files exist — which may be from a previous successful run, making it look like the current run succeeded.

Test plan

  • Verify case-optimization job shows no output in Print Logs when prebuild fails
  • Verify normal successful runs still show correct logs
  • Verify stale SLURM job cancellation still works (.slurm_job_id files preserved)

🤖 Generated with Claude Code

When a multi-step CI job (like case-optimization) fails at an early
step, the 'Print Logs' step (if: always) would cat output files from
a previous successful run, making it appear the current run succeeded.
Delete stale .out files after checkout so logs only show output from
the current workflow run.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

Claude Code Review

Head SHA: b71418a

Files changed: 1

  • .github/workflows/test.yml

Summary:

  • Adds a "Clean stale output files" step (line 237 and 323) in both the test suite and case-optimization jobs immediately after checkout
  • Deletes *.out files to prevent the unconditional Print Logs step from displaying output from a prior run
  • Preserves .slurm_job_id files (targeted rm -f *.out rather than a broader clean) so stale SLURM job cancellation still functions
  • clean: false on checkout is intentional and remains unchanged
  • Change is additive only (6 additions, 0 deletions in workflow YAML)

Findings:

No correctness or logic issues found.

Improvement opportunities:

  1. Working directory assumption (.github/workflows/test.yml, lines ~238 and ~324): rm -f *.out runs in whatever the default working directory is for the runner step (typically the repo root). If .out files are written to a subdirectory (e.g., a job-specific scratch dir or /tmp), this step will silently do nothing and the problem remains. Worth verifying that the .out files being targeted are indeed written to $GITHUB_WORKSPACE / repo root.

  2. Scope of pattern: The glob *.out only matches files in the working directory, not subdirectories. If outputs are ever nested (e.g., logs/*.out), consider find . -name '*.out' -delete for robustness. Low priority if current layout is flat.

  3. Step placement: Both new steps are placed before the build step, which is correct. No concern here — just confirming placement is intentional to cover the full pre-build-to-log window.

Overall: Clean, minimal, and well-motivated fix. The PR description accurately describes the intent and the implementation matches it. LGTM with the working-directory consideration worth a quick sanity check.

@sbryngelson sbryngelson marked this pull request as ready for review March 14, 2026 00:32
Copilot AI review requested due to automatic review settings March 14, 2026 00:32
@sbryngelson sbryngelson enabled auto-merge (squash) March 14, 2026 00:33
@sbryngelson sbryngelson disabled auto-merge March 14, 2026 00:33
@sbryngelson sbryngelson merged commit f528e23 into MFlowCode:master Mar 14, 2026
33 of 41 checks passed
@github-actions
Copy link
Copy Markdown

Claude Code Review

Head SHA: b71418a

Files changed: 1

  • .github/workflows/test.yml

Summary:

  • Adds two rm -f *.out steps (one in the test suite job, one in the case-optimization job) immediately after actions/checkout with clean: false
  • Prevents the always-running Print Logs step from displaying stale .out files from a previous workflow run when the current run fails early
  • Intentionally preserves .slurm_job_id files so submit-slurm-job.sh can still detect and cancel stale SLURM jobs

Findings:

No correctness issues found. A couple of observations:

  1. Scope of glob (.github/workflows/test.yml, lines 237 and 323): rm -f *.out runs in the runner's working directory (repo root by default). If any workflow step cds into a subdirectory before generating .out files, those subdirectory files would not be cleaned. Based on the PR context this appears intentional and correct, but worth confirming that all SLURM-generated .out files land in the repo root.

  2. Consistency: The fix is applied symmetrically to both the test job and the case-optimization job, which is correct. Both jobs have the same pattern of checkout (clean: false) → (potentially stale .out files) → Print Logs (if: always()).

Overall: The change is minimal, targeted, and well-motivated. Logic is sound — -f suppresses errors when no .out files exist, and the glob pattern correctly excludes .slurm_job_id files. Approved.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 14, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 687830cb-6a3e-4ddd-a300-868a7cac5f09

📥 Commits

Reviewing files that changed from the base of the PR and between 92e751f and b71418a.

📒 Files selected for processing (1)
  • .github/workflows/test.yml

📝 Walkthrough

Walkthrough

The change adds a new cleanup step to the test workflow that removes stale output files before running tests. Specifically, a "Clean stale output files" step has been introduced to two workflow jobs, executing rm -f *.out to remove any pre-existing .out files from the workspace. This cleanup runs after cloning the repository and before subsequent workflow steps execute. No modifications were made to existing workflow logic or error handling procedures.

📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can generate a title for your PR based on the changes with custom instructions.

Set the reviews.auto_title_instructions setting to generate a title for your PR based on the changes in the PR with custom instructions.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the CI workflow to remove stale .out log files after checkout (while keeping .slurm_job_id files) so that unconditional log-printing steps don’t accidentally show output from a previous run when the current run fails early.

Changes:

  • Add a “Clean stale output files” step after actions/checkout in the main test-suite job.
  • Add the same cleanup step after actions/checkout in the case-optimization job.

Comment on lines +237 to +239
- name: Clean stale output files
run: rm -f *.out

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 45.34%. Comparing base (598f5a5) to head (b71418a).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1308   +/-   ##
=======================================
  Coverage   45.34%   45.34%           
=======================================
  Files          70       70           
  Lines       20514    20514           
  Branches     1954     1954           
=======================================
  Hits         9303     9303           
  Misses      10084    10084           
  Partials     1127     1127           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants