Repository navigation
ci: speed up GitHub Actions workflows - #5149
Conversation
build-and-test-v3: - Remove no-op check_approval gate that serialised test_js startup without ever conditioning on its approved output - Remove check_approval from test_go needs (now only needs test_js) - Add npm cache (actions/cache@v4 on ~/.npm) before npm ci in test_js, keyed on package-lock.json hash - Add timeout-minutes: 25 to test_go job - Add -timeout 10m flag to all go test invocations - Fix ubuntu xvfb/dbus: replace dbus-update-activation-environment --systemd (hangs without systemd user session, root cause of 22-min spikes) with dbus-run-session which starts a clean session daemon; applies to both GTK3 and GTK4 test steps pr-master: - Fix test_go condition: was approval-only (github.event.review.state == 'approved'), so tests never ran on regular PR pushes; now runs on all pull_request events too - Add types filter to pull_request trigger - Upgrade actions/checkout v3 → v4 (both jobs) - Upgrade actions/setup-go v3 → v5 with cache-dependency-path - Replace bare apt-get with awalsh128/cache-apt-pkgs-action (saves ~30s per Linux job on cache hit, consistent with v3 workflow)
|
Caution Review failedPull request was closed or merged during review WalkthroughRemoved the approval gate from CI, decoupled test job dependencies, added npm and apt caching, upgraded GitHub Actions, and tightened Go test timeouts and session invocation across workflows. (50 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Deploying wails with
|
| Latest commit: |
76eee17
|
| Status: | 🚫 Build failed. |
|
![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
🧹 Nitpick comments (1)
.github/workflows/pr-master.yml (1)
5-5: Addready_for_reviewto PR trigger types.Draft PRs marked ready for review will not trigger this workflow without it. The
ready_for_reviewactivity type must be explicitly listed alongsideopened,synchronize, andreopenedto handle draft-to-ready transitions.Suggested change
- types: [opened, synchronize, reopened] + types: [opened, synchronize, reopened, ready_for_review]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-master.yml at line 5, The workflow trigger currently lists types: [opened, synchronize, reopened] and misses the ready_for_review event, so add "ready_for_review" to that array (i.e., change the types value used in the PR trigger block to include ready_for_review alongside opened, synchronize, and reopened) so draft->ready transitions fire this workflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-master.yml:
- Around line 54-58: Replace the mutable reference "uses:
awalsh128/cache-apt-pkgs-action@latest" with the immutable commit SHA "uses:
awalsh128/cache-apt-pkgs-action@acb598e5ddbc6f68a970c5da0688d2f3a9f04d05" for
both occurrences of the cache-apt-pkgs-action in the workflow so the action is
pinned to a fixed commit SHA instead of `@latest`; update every instance of the
string so both usages are consistent.
---
Nitpick comments:
In @.github/workflows/pr-master.yml:
- Line 5: The workflow trigger currently lists types: [opened, synchronize,
reopened] and misses the ready_for_review event, so add "ready_for_review" to
that array (i.e., change the types value used in the PR trigger block to include
ready_for_review alongside opened, synchronize, and reopened) so draft->ready
transitions fire this workflow.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: be2b0a5d-ef9e-45fc-98d1-75a34fa7cc98
📒 Files selected for processing (2)
.github/workflows/build-and-test-v3.yml.github/workflows/pr-master.yml
| uses: awalsh128/cache-apt-pkgs-action@latest | ||
| if: matrix.os == 'ubuntu-22.04' | ||
| run: sudo apt-get update -y && sudo apt-get install -y libgtk-3-dev libwebkit2gtk-4.0-dev build-essential pkg-config | ||
| with: | ||
| packages: libgtk-3-dev libwebkit2gtk-4.0-dev build-essential pkg-config | ||
| version: 1.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Current workflow refs:"
rg -nP 'uses:\s*awalsh128/cache-apt-pkgs-action@' .github/workflows
echo
echo "Recent upstream tags and commit SHAs:"
curl -s https://api.github.com/repos/awalsh128/cache-apt-pkgs-action/tags \
| jq -r '.[0:10][] | "\(.name)\t\(.commit.sha)"'Repository: wailsapp/wails
Length of output: 1255
Pin awalsh128/cache-apt-pkgs-action to an immutable commit SHA.
Using @latest is mutable and undermines reproducibility and security. Pin both occurrences (lines 54 and 61) to a stable commit SHA. Current latest is acb598e5ddbc6f68a970c5da0688d2f3a9f04d05 (from v1.6.0).
Proposed hardening
- uses: awalsh128/cache-apt-pkgs-action@latest
+ uses: awalsh128/cache-apt-pkgs-action@acb598e5ddbc6f68a970c5da0688d2f3a9f04d05📝 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.
| uses: awalsh128/cache-apt-pkgs-action@latest | |
| if: matrix.os == 'ubuntu-22.04' | |
| run: sudo apt-get update -y && sudo apt-get install -y libgtk-3-dev libwebkit2gtk-4.0-dev build-essential pkg-config | |
| with: | |
| packages: libgtk-3-dev libwebkit2gtk-4.0-dev build-essential pkg-config | |
| version: 1.0 | |
| uses: awalsh128/cache-apt-pkgs-action@acb598e5ddbc6f68a970c5da0688d2f3a9f04d05 | |
| if: matrix.os == 'ubuntu-22.04' | |
| with: | |
| packages: libgtk-3-dev libwebkit2gtk-4.0-dev build-essential pkg-config | |
| version: 1.0 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pr-master.yml around lines 54 - 58, Replace the mutable
reference "uses: awalsh128/cache-apt-pkgs-action@latest" with the immutable
commit SHA "uses:
awalsh128/cache-apt-pkgs-action@acb598e5ddbc6f68a970c5da0688d2f3a9f04d05" for
both occurrences of the cache-apt-pkgs-action in the workflow so the action is
pinned to a fixed commit SHA instead of `@latest`; update every instance of the
string so both usages are consistent.
There was a problem hiding this comment.
Pull request overview
This PR targets CI wall-clock improvements and reliability by reducing unnecessary workflow serialization, adding caching, and hardening Linux GUI test execution to avoid DBus-related hangs and unbounded job runtimes.
Changes:
- Simplifies
build-and-test-v3by removing the no-op approval gate, adding npm caching, adding job/test timeouts, and replacingdbus-update-activation-environmentwithdbus-run-sessionfor Linux test stability. - Fixes
pr-masterGo test job gating so tests run on PR pushes (and on approval reviews), adds apull_request.typesfilter, and upgrades/adjusts actions to enable caching and speed up Linux dependency installs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| .github/workflows/pr-master.yml | Adjusts PR triggers, fixes test job if: logic, upgrades checkout/setup-go, and adds cached apt installs for Linux runners. |
| .github/workflows/build-and-test-v3.yml | Removes unused approval job, adds npm cache, adds job/test timeouts, and updates Linux test execution to use dbus-run-session with xvfb-run. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Install linux dependencies (22.04) | ||
| uses: awalsh128/cache-apt-pkgs-action@latest | ||
| if: matrix.os == 'ubuntu-22.04' | ||
| run: sudo apt-get update -y && sudo apt-get install -y libgtk-3-dev libwebkit2gtk-4.0-dev build-essential pkg-config | ||
| with: | ||
| packages: libgtk-3-dev libwebkit2gtk-4.0-dev build-essential pkg-config | ||
| version: 1.0 |
There was a problem hiding this comment.
Using awalsh128/cache-apt-pkgs-action@latest makes the workflow non-deterministic and introduces supply-chain risk (the action can change without a PR). Pin this to a specific release tag (or preferably a commit SHA) and bump intentionally when needed.
| - name: Run tests (ubuntu) - GTK3 default | ||
| if: matrix.os == 'ubuntu-latest' | ||
| working-directory: v3 | ||
| run: > | ||
| xvfb-run --auto-servernum | ||
| sh -c ' | ||
| dbus-update-activation-environment --systemd --all && | ||
| go test -v ./... | ||
| ' | ||
| run: dbus-run-session -- xvfb-run --auto-servernum --server-args="-screen 0 1024x768x24" go test -v -timeout 10m ./... | ||
|
|
There was a problem hiding this comment.
dbus-run-session is now a hard dependency for the Linux test steps, but the Linux dependency install list doesn't explicitly install a package that provides it. To avoid failures if the runner image changes, add the appropriate package (e.g., dbus/dbus-x11, depending on Ubuntu packaging) to the apt packages list or add a preflight check with a clear error message.
| - name: Run tests (ubuntu) - GTK4 | ||
| if: matrix.os == 'ubuntu-latest' | ||
| working-directory: v3 | ||
| # Skip service tests that hang in CI due to GTK4 display requirements | ||
| run: > | ||
| xvfb-run --auto-servernum | ||
| sh -c ' | ||
| dbus-update-activation-environment --systemd --all && | ||
| go test -tags gtk4 -v -skip "TestService" ./... | ||
| ' | ||
| run: dbus-run-session -- xvfb-run --auto-servernum --server-args="-screen 0 1024x768x24" go test -tags gtk4 -v -timeout 10m -skip "TestService" ./... |
There was a problem hiding this comment.
Same as the GTK3 step: these tests now rely on dbus-run-session, but it's not guaranteed by the explicit apt install list. Consider installing the package that provides dbus-run-session to keep the workflow resilient across runner image updates.
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened] |
There was a problem hiding this comment.
pull_request trigger is now limited to [opened, synchronize, reopened], which means checks will not run when a draft PR is marked "Ready for review" (ready_for_review event). If the intent is to keep draft PRs from running, consider adding ready_for_review here so checks start automatically once the PR is ready, or document that a new push is required to trigger CI.
| types: [opened, synchronize, reopened] | |
| types: [opened, synchronize, reopened, ready_for_review] |
libwebkitgtk-6.0-dev can hit slow apt mirrors, causing 32-41 min installs instead of the normal ~0.1 min. Use awalsh128/cache-apt-pkgs-action for libgtk-4-dev and libwebkitgtk-6.0-dev (same as GTK3 deps already do) to eliminate this flakiness in both test_go and test_templates jobs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
) - Upgrade actions/checkout v3 → v4 in both check_docs and test_go - Upgrade actions/setup-go v3 → v5 with cache-dependency-path: v2/go.sum (enables Go module and build cache, was missing previously) - Replace bare apt-get update/install with awalsh128/cache-apt-pkgs-action for ubuntu-22.04 and ubuntu-24.04 (saves ~30s per Linux job on cache hit, consistent with how v3 workflows handle Linux deps) The test_go condition and trigger types were already correct on master. Companion to #5149 which makes equivalent changes on the v3-alpha branch. Co-authored-by: Wails Documentation Agent <agent@wails.local>
Summary
CI audit (LEA-37) identified several workflow inefficiencies. This PR applies targeted fixes to reduce wall-clock time and eliminate the root cause of p95 spikes.
2 files changed, 33 insertions, 40 deletions.
Recommendations Table
check_approvalgate serialisestest_jsstartup with no benefit (approved output never used)build-and-test-v3needs:dbus-update-activation-environment --systemdhangs without a systemd user session — root cause of 22-min ubuntu test spikesbuild-and-test-v3dbus-run-session --on both GTK3 and GTK4 stepstest_go— a hung dbus/xvfb session runs foreverbuild-and-test-v3timeout-minutes: 25+-timeout 10mtogo testtest_js—npm cidownloads all packages fresh every runbuild-and-test-v3actions/cache@v4on~/.npmkeyed onpackage-lock.jsonhashtest_goif:condition wasgithub.event.review.state == 'approved'— tests never ran on regular PR pushespr-mastergithub.event_name == 'pull_request' || review.state == 'approved'types:filter onpull_requesttriggerpr-mastertypes: [opened, synchronize, reopened]actions/checkout@v3andsetup-go@v3are outdated; no Go module/build cache activepr-master@v4/@v5withcache-dependency-path: v2/go.sumapt-get update && apt-get installuncached on every v2 PR runpr-masterawalsh128/cache-apt-pkgs-action(already used in v3 workflow)Expected Impact
Build + Test v3:
test_jsstarts immediately (nocheck_approvalgate)PR Checks (master):
Notes
dbus-run-sessionships with thedbuspackage, available on all GitHub-hosted ubuntu runnerspr-master.ymlchanges affect themasterbranch workflow; both files exist onv3-alphaand are included in this PR for a single clean reviewon:) are not changed inbuild-and-test-v3Summary by CodeRabbit