Extract repeated directory-check assertions into check-dir.sh helper#1127
Open
brunoborges wants to merge 1 commit into
Open
Extract repeated directory-check assertions into check-dir.sh helper#1127brunoborges wants to merge 1 commit into
brunoborges wants to merge 1 commit into
Conversation
The e2e-cache.yml and e2e-cache-dependency-path.yml workflows repeated the same inline shell block many times to assert a cache directory exists (and list it), plus one inverse check that a directory does NOT exist. 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. Per-OS Coursier conditionals and all build steps are left unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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
Comment on lines
146
to
+148
| - name: Check files to cache on ubuntu-latest | ||
| if: matrix.os == 'ubuntu-latest' | ||
| run: | | ||
| if [ ! -d ~/.cache/coursier ]; then | ||
| echo "::error::The ~/.cache/coursier directory does not exist unexpectedly" | ||
| exit 1 | ||
| fi | ||
| run: bash "$GITHUB_WORKSPACE/__tests__/check-dir.sh" "$HOME/.cache/coursier" |
Comment on lines
179
to
+181
| - name: Confirm that ~/.cache/coursier directory has been made | ||
| if: matrix.os == 'ubuntu-latest' | ||
| run: | | ||
| if [ ! -d ~/.cache/coursier ]; then | ||
| echo "::error::The ~/.cache/coursier directory does not exist unexpectedly" | ||
| exit 1 | ||
| fi | ||
| ls ~/.cache/coursier | ||
| run: bash "$GITHUB_WORKSPACE/__tests__/check-dir.sh" "$HOME/.cache/coursier" |
Comment on lines
+11
to
+14
| set -eu | ||
|
|
||
| dir=$1 | ||
| mode=${2:-present} |
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
The
e2e-cache.ymlande2e-cache-dependency-path.ymlworkflows repeated the same inline shell block many times to assert that a cache directory exists (and thenlsit), plus one inverse check (ingradle2-restore) asserting that~/.gradle/cachesdoes 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.if [ ! -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
python3+yaml.safe_load.bash -n __tests__/check-dir.shpasses.present/absentpass and fail (exit 1) as expected.