Extract repeated directory-check assertions into check-dir.sh helper#1127
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR deduplicates repeated “assert cache directory present/absent” shell logic in the cache E2E GitHub Actions workflows by introducing a shared __tests__/check-dir.sh helper and updating the workflows to call it.
Changes:
- Added
__tests__/check-dir.shto validate a directory ispresent(andlsit) orabsent(and fail if it exists). - Updated
.github/workflows/e2e-cache.ymlto replace repeated inlineif [ -d ... ]assertions with calls to the helper. - Updated
.github/workflows/e2e-cache-dependency-path.ymlto use the helper for the same directory assertions, including the one “absent” check.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/e2e-cache.yml | Replaces repeated inline directory checks with calls to __tests__/check-dir.sh across gradle/maven/sbt jobs. |
| .github/workflows/e2e-cache-dependency-path.yml | Switches gradle cache dependency-path assertions to use the shared directory-check helper. |
| tests/check-dir.sh | New reusable script implementing present/absent directory assertions for workflow reuse. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Low
The e2e-cache.yml workflow repeated the same inline shell block many times to assert a cache directory exists (and list it), plus inverse checks that a directory does NOT exist (the gradle2/maven2/sbt2 cache-miss jobs). Add `__tests__/check-dir.sh` (POSIX sh, executable) with a `check-dir.sh <dir> [present|absent]` interface and replace every inline check with a call to it, passing already-expanded $HOME paths to avoid tilde-expansion pitfalls. The sbt jobs override working-directory, so they call the helper via $GITHUB_WORKSPACE. Per-OS Coursier conditionals and all build steps are left unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
brunoborges
force-pushed
the
brunoborges-extract-check-dir-helper
branch
from
July 15, 2026 14:50
d1b0ad1 to
0040423
Compare
…ditionals - check-dir.sh: with set -u, invoking without a <dir> argument failed with an opaque "parameter not set" error. Add an explicit argc check that prints a usage message and exits 2 (distinct from the assertion failure code 1). - e2e-cache.yml: the sbt-save and sbt-restore jobs run on an ubuntu-22.04 matrix entry, but their coursier-cache steps were guarded by 'if: matrix.os == "ubuntu-latest"', so those checks never executed on Ubuntu. Align the conditionals with the matrix (ubuntu-22.04), matching the newer sbt1/sbt2 jobs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.
What
e2e-cache.ymlrepeated the same inline shell block many times to assert that a cache directory exists (and thenlsit), plus several inverse checks (thegradle2/maven2/sbt2cache-miss jobs) asserting a directory does not exist.This PR deduplicates that logic into a single helper script.
Changes
__tests__/check-dir.sh(POSIX sh, marked executable):check-dir.sh <dir> [present|absent], default modepresent.present: fail with::error::The <dir> directory does not exist unexpectedly+exit 1when the directory is missing; otherwiselsit.absent: fail with::error::The <dir> directory exists unexpectedly+exit 1when the directory exists.e2e-cache.ymlnow calls the helper instead of inlineif [ ! -d ... ]blocks, keeping the exact same directories and present/absent semantics. Steps that only did check-and-lscollapse to a single line.$HOME(e.g.bash __tests__/check-dir.sh "$HOME/.gradle/caches") to avoid tilde-expansion pitfalls. The sbt jobs use aworking-directoryoverride, so they reference the script via$GITHUB_WORKSPACE.if:OS conditionals are preserved (Coursier:~/Library/Caches/Coursieron macOS,~/AppData/Local/Coursier/Cacheon Windows,~/.cache/coursieron Ubuntu). Themvn/gradle/sbtbuild steps are unchanged.Validation
e2e-cache.ymlparses viapython3+yaml.safe_load.bash -n __tests__/check-dir.shpasses.present/absentpass and fail (exit 1) as expected.