Skip to content

Conversation

@UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Nov 14, 2025

Issue being fixed or feature implemented

CI started to fail with System.IO.IOException: No space left on device for otherwise successful jobs recently. Example: https://github.com/dashpay/dash/actions/runs/19371409140/job/55429155189.

What was done?

Remove test datadir after extracting logs to free up CI disk space.

How Has This Been Tested?

See CI for this PR

Breaking Changes

n/a, we don't store test datadirs in artifacts or anywhere else afaik

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@UdjinM6 UdjinM6 added this to the 23.1 milestone Nov 14, 2025
@github-actions
Copy link

github-actions bot commented Nov 14, 2025

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

@UdjinM6 UdjinM6 marked this pull request as ready for review November 14, 2025 23:40
@coderabbitai
Copy link

coderabbitai bot commented Nov 14, 2025

Walkthrough

This change adds a cleanup operation to the CI integration test script. After extracting logs from each test directory, the script now removes the corresponding test data directory using rm -rf to reclaim disk space in the CI environment. The deletion step executes after log file handling for each test iteration, targeting the testdatadirs/$BASEDIR/$d path.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant CI as CI job
  participant FS as Filesystem
  participant Logs as Logs archive

  rect rgba(200,230,255,0.4)
    CI->>FS: iterate test directories (for each $d)
    CI->>FS: move/copy logs from testdatadirs/$BASEDIR/$d -> logs/
    FS-->>Logs: logs placed
  end

  rect rgba(230,255,220,0.4)
    CI->>FS: rm -rf testdatadirs/$BASEDIR/$d
    FS-->>CI: confirm deletion
  end

  note over CI,FS: repeat for each test directory
Loading

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single file modification with a single command addition
  • Review focus:
    • Verify rm -rf target path is correct (testdatadirs/$BASEDIR/$d)
    • Ensure cleanup occurs after log extraction to avoid data loss

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the main change: removing test datadirs after log extraction to free CI disk space, which directly addresses the issue of CI failures due to disk space.
Description check ✅ Passed The description is directly related to the changeset, providing context about the disk space issue, explaining what was done, and addressing testing and breaking changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 68d00e4 and 6084c37.

📒 Files selected for processing (1)
  • ci/dash/test_integrationtests.sh (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: linux64_tsan-build / Build source
  • GitHub Check: linux64_nowallet-build / Build source
  • GitHub Check: mac-build / Build source
  • GitHub Check: linux64_ubsan-build / Build source
  • GitHub Check: linux64-build / Build source
  • GitHub Check: linux64_sqlite-build / Build source
  • GitHub Check: linux64_fuzz-build / Build source
  • GitHub Check: win64-build / Build source
  • GitHub Check: arm-linux-build / Build source
  • GitHub Check: Lint / Run linters
🔇 Additional comments (1)
ci/dash/test_integrationtests.sh (1)

71-72: Cleanup properly handles rm failures while preserving test results.

The || echo pattern correctly ensures that rm -rf failures do not mask the actual test result. When the removal fails, the echo command succeeds, allowing the script to continue and eventually exit with the captured $RESULT code at line 78. This addresses the critical concern from the prior review.


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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d3fa315 and 68d00e4.

📒 Files selected for processing (1)
  • ci/dash/test_integrationtests.sh (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: linux64_multiprocess-test / Test source

@UdjinM6 UdjinM6 marked this pull request as draft November 15, 2025 09:07
@UdjinM6 UdjinM6 removed this from the 23.1 milestone Nov 15, 2025
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

LGTM; ready to merge?

@UdjinM6
Copy link
Author

UdjinM6 commented Nov 15, 2025

We can merge this one too but it's not enough, pls review #6981

@UdjinM6
Copy link
Author

UdjinM6 commented Nov 16, 2025

closing in fav of #6986

@UdjinM6 UdjinM6 closed this Nov 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants