fix(ci): verify desktop E2E jobs actually ran in release gate#258
Conversation
When e2e-desktop.yml runs on a push with no desktop changes, the workflow succeeds but the actual test jobs are skipped. The release gate was treating these skipped runs as passing, potentially masking previous desktop E2E failures. Now both Tier 1 (HEAD check) and Tier 2 (range search) query the jobs API to verify at least one "Desktop E2E" job completed with conclusion "success". Runs where tests were skipped are ignored and the search continues to find a run that actually executed tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: 09a3511d2724
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe release-gate workflow is enhanced to verify that E2E desktop tests actually executed rather than being skipped. A helper function determines test execution status, HEAD run verification now polls for completion and validates test execution, and the in-range search logic requires confirmed test execution before accepting a run. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/release-gate.yml (1)
184-191: Consider reordering the PREV_TAG check before the search message.Currently, if
PREV_TAGis empty, the user sees "Searching for a Desktop E2E run..." immediately followed by an error saying no previous tag exists. Moving the PREV_TAG check before the search message would improve clarity.💡 Suggested reordering
# No desktop E2E run for HEAD (or HEAD run skipped tests) — find the # most recent run in the release range that actually executed tests - echo "Searching for a Desktop E2E run that executed tests since ${PREV_TAG}..." if [ -z "$PREV_TAG" ]; then echo "::error::No previous release tag and no Desktop E2E run for HEAD. Cannot verify desktop changes." exit 1 fi + echo "Searching for a Desktop E2E run that executed tests since ${PREV_TAG}..." + # Get commit SHAs in the release range🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-gate.yml around lines 184 - 191, Move the PREV_TAG emptiness check to run before printing the "Searching for a Desktop E2E run that executed tests since ${PREV_TAG}..." message: check if PREV_TAG is empty (the existing if [ -z "$PREV_TAG" ] block) and emit the error/exit early if so, then only print the search message and proceed; update the order around the echo "Searching for a Desktop E2E run..." and the if [ -z "$PREV_TAG" ] block so the check happens first.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/release-gate.yml:
- Around line 184-191: Move the PREV_TAG emptiness check to run before printing
the "Searching for a Desktop E2E run that executed tests since ${PREV_TAG}..."
message: check if PREV_TAG is empty (the existing if [ -z "$PREV_TAG" ] block)
and emit the error/exit early if so, then only print the search message and
proceed; update the order around the echo "Searching for a Desktop E2E run..."
and the if [ -z "$PREV_TAG" ] block so the check happens first.
There was a problem hiding this comment.
Pull request overview
Tightens the release gate’s validation of e2e-desktop.yml by ensuring “successful” workflow runs are only accepted if at least one Desktop E2E job actually executed (i.e., was not skipped due to path-based change detection).
Changes:
- Added a helper that queries the GitHub Jobs API for a workflow run to confirm
Desktop E2E*jobs completed successfully. - Updated the “HEAD check” to fall back to range search when the HEAD run succeeded but only skipped tests.
- Updated the range search to ignore skipped-test runs and continue searching until it finds a run that executed tests (or fails if none exist).
- run_executed_tests now fails fast (return 2) on gh API errors instead of silently treating failures as "tests skipped" - All callers check for the new error return code and exit 1 - Polling loop uses `gh run view $HEAD_RUN_ID` instead of re-querying by SHA, preventing inconsistency if a rerun is triggered mid-poll Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: 885c3828eac6
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #258 +/- ##
==========================================
+ Coverage 46.31% 46.68% +0.37%
==========================================
Files 106 106
Lines 8266 8255 -11
Branches 591 594 +3
==========================================
+ Hits 3828 3854 +26
+ Misses 4271 4233 -38
- Partials 167 168 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Summary
e2e-desktop.ymlruns on a push with no desktop changes, the workflow "succeeds" but test jobs are skippedDesktop E2Ejob completed withconclusion: "success"Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit