ci(smoke): isolated upload-artifact@v7 smoke test (gates #149)#165
Merged
ci(smoke): isolated upload-artifact@v7 smoke test (gates #149)#165
Conversation
Validates whether actions/upload-artifact@v7 (ESM, Node 24) is safe to roll out to the main release pipeline before merging dependabot PR #149 (v4 -> v7, three majors skipped). Coverage: - ubuntu-22.04 (current pipeline OS) - ubuntu-24.04 (target for snap rework, see #164) - windows-latest, macos-latest (parity with build-and-release matrix) Each runner uploads a 1 MiB binary plus a text marker, the verify job downloads everything back and checks file presence and size. Triggers on pull_request edits of this file or workflow_dispatch so re-running on demand is one click.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
📝 WalkthroughWalkthroughA new GitHub Actions workflow file for smoke-testing ChangesArtifact Upload v7 Smoke Test
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
🤖 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/smoke-upload-artifact-v7.yml:
- Around line 47-74: The script currently iterates over existing
downloaded/smoke-* directories and may miss absent artifacts; add an explicit
presence check against the expected matrix artifacts before validating contents:
define an array (e.g., expected_runners) or expected count that lists the
expected downloaded/smoke-<name> directories, iterate that list and if any
expected directory does not exist set fail=1 and print "FAIL: missing artifact
<dir>"; only after confirming all expected directories exist proceed to the
per-artifact validation loop that uses the existing for d in downloaded/smoke-*
logic (or replace that loop to iterate the expected list directly) so missing
artifacts cannot be silently ignored.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fbabab43-9720-4f53-b34b-3d296c4a2a2d
📒 Files selected for processing (1)
.github/workflows/smoke-upload-artifact-v7.yml
Comment on lines
+47
to
+74
| set -euo pipefail | ||
| fail=0 | ||
| for d in downloaded/smoke-*; do | ||
| echo "=== $d ===" | ||
| ls -la "$d" | ||
| if [ ! -f "$d/dummy.txt" ]; then | ||
| echo "FAIL: dummy.txt missing in $d" | ||
| fail=1 | ||
| continue | ||
| fi | ||
| if [ ! -f "$d/binary.bin" ]; then | ||
| echo "FAIL: binary.bin missing in $d" | ||
| fail=1 | ||
| continue | ||
| fi | ||
| size=$(stat -c%s "$d/binary.bin" 2>/dev/null || stat -f%z "$d/binary.bin") | ||
| if [ "$size" != "1048576" ]; then | ||
| echo "FAIL: binary.bin size is $size, expected 1048576" | ||
| fail=1 | ||
| continue | ||
| fi | ||
| echo "PASS: $d" | ||
| done | ||
| if [ "$fail" -ne 0 ]; then | ||
| echo "Smoke test FAILED" | ||
| exit 1 | ||
| fi | ||
| echo "Smoke test PASSED on all matrix runners" |
There was a problem hiding this comment.
Assert all expected matrix artifacts exist before per-artifact validation.
The current verification only checks directories that happen to exist. It should fail explicitly when any expected runner artifact is missing, otherwise this smoke test can produce a false green for partial downloads/uploads.
Proposed patch
- name: Verify each artifact
shell: bash
run: |
set -euo pipefail
+ expected=(
+ "downloaded/smoke-ubuntu-22.04"
+ "downloaded/smoke-ubuntu-24.04"
+ "downloaded/smoke-windows-latest"
+ "downloaded/smoke-macos-latest"
+ )
fail=0
- for d in downloaded/smoke-*; do
+ for d in "${expected[@]}"; do
+ if [ ! -d "$d" ]; then
+ echo "FAIL: missing artifact directory $d"
+ fail=1
+ continue
+ fi
echo "=== $d ==="
ls -la "$d"
if [ ! -f "$d/dummy.txt" ]; then
echo "FAIL: dummy.txt missing in $d"
fail=1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| set -euo pipefail | |
| fail=0 | |
| for d in downloaded/smoke-*; do | |
| echo "=== $d ===" | |
| ls -la "$d" | |
| if [ ! -f "$d/dummy.txt" ]; then | |
| echo "FAIL: dummy.txt missing in $d" | |
| fail=1 | |
| continue | |
| fi | |
| if [ ! -f "$d/binary.bin" ]; then | |
| echo "FAIL: binary.bin missing in $d" | |
| fail=1 | |
| continue | |
| fi | |
| size=$(stat -c%s "$d/binary.bin" 2>/dev/null || stat -f%z "$d/binary.bin") | |
| if [ "$size" != "1048576" ]; then | |
| echo "FAIL: binary.bin size is $size, expected 1048576" | |
| fail=1 | |
| continue | |
| fi | |
| echo "PASS: $d" | |
| done | |
| if [ "$fail" -ne 0 ]; then | |
| echo "Smoke test FAILED" | |
| exit 1 | |
| fi | |
| echo "Smoke test PASSED on all matrix runners" | |
| set -euo pipefail | |
| expected=( | |
| "downloaded/smoke-ubuntu-22.04" | |
| "downloaded/smoke-ubuntu-24.04" | |
| "downloaded/smoke-windows-latest" | |
| "downloaded/smoke-macos-latest" | |
| ) | |
| fail=0 | |
| for d in "${expected[@]}"; do | |
| if [ ! -d "$d" ]; then | |
| echo "FAIL: missing artifact directory $d" | |
| fail=1 | |
| continue | |
| fi | |
| echo "=== $d ===" | |
| ls -la "$d" | |
| if [ ! -f "$d/dummy.txt" ]; then | |
| echo "FAIL: dummy.txt missing in $d" | |
| fail=1 | |
| continue | |
| fi | |
| if [ ! -f "$d/binary.bin" ]; then | |
| echo "FAIL: binary.bin missing in $d" | |
| fail=1 | |
| continue | |
| fi | |
| size=$(stat -c%s "$d/binary.bin" 2>/dev/null || stat -f%z "$d/binary.bin") | |
| if [ "$size" != "1048576" ]; then | |
| echo "FAIL: binary.bin size is $size, expected 1048576" | |
| fail=1 | |
| continue | |
| fi | |
| echo "PASS: $d" | |
| done | |
| if [ "$fail" -ne 0 ]; then | |
| echo "Smoke test FAILED" | |
| exit 1 | |
| fi | |
| echo "Smoke test PASSED on all matrix runners" |
🤖 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/smoke-upload-artifact-v7.yml around lines 47 - 74, The
script currently iterates over existing downloaded/smoke-* directories and may
miss absent artifacts; add an explicit presence check against the expected
matrix artifacts before validating contents: define an array (e.g.,
expected_runners) or expected count that lists the expected
downloaded/smoke-<name> directories, iterate that list and if any expected
directory does not exist set fail=1 and print "FAIL: missing artifact <dir>";
only after confirming all expected directories exist proceed to the per-artifact
validation loop that uses the existing for d in downloaded/smoke-* logic (or
replace that loop to iterate the expected list directly) so missing artifacts
cannot be silently ignored.
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
1 participant
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.
Goal
Decide whether #149 (`actions/upload-artifact` v4 -> v7) is safe to merge into the main release pipeline.
CI green on #149 alone is not enough. v6 bumped runtime to Node 24, v7 switched to ESM. Three majors are skipped in one bump, and the actual upload at tag push time is not exercised on a regular push CI run.
What this PR does
Adds `.github/workflows/smoke-upload-artifact-v7.yml`, an isolated workflow that:
Matrix runners covered:
Triggers: `pull_request` on the workflow file itself (so this PR runs it automatically) and `workflow_dispatch` for manual re-runs from the Actions UI.
Decision matrix
Out of scope
Summary by CodeRabbit