ci(ui): guard e2e fork secrets#11739
Conversation
📝 WalkthroughWalkthroughThe UI E2E workflow adds ChangesUI E2E Workflow Fork-Aware Secret Gate
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
✅ Conflict Markers Resolved All conflict markers have been successfully resolved in this pull request. |
|
✅ All necessary |
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/ui-e2e-tests-v2.yml:
- Around line 239-256: Move the forked-PR secrets check out of the Run E2E tests
step in the ui-e2e-tests-v2 workflow so it short-circuits before the expensive
setup runs. Add an earlier guard around the e2e-tests job itself, or introduce a
lightweight precondition step/job that checks PR_HEAD_REPOSITORY against
BASE_REPOSITORY and the required E2E_* secrets before the API image build,
Docker startup, UI build, and Playwright install steps. Keep the existing
skip/summary behavior for forked PRs, but ensure the job never reaches the heavy
setup when the secrets are unavailable.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 18ac11fb-0b76-4885-a05e-12ac379bc7ac
📒 Files selected for processing (1)
.github/workflows/ui-e2e-tests-v2.yml
| - name: Run E2E tests | ||
| working-directory: ./ui | ||
| env: | ||
| PR_HEAD_REPOSITORY: ${{ github.event.pull_request.head.repo.full_name }} | ||
| BASE_REPOSITORY: ${{ github.repository }} | ||
| run: | | ||
| if [[ -z "${E2E_ADMIN_USER}" || -z "${E2E_ADMIN_PASSWORD}" || -z "${E2E_NEW_USER_PASSWORD}" ]]; then | ||
| if [[ "${PR_HEAD_REPOSITORY}" != "${BASE_REPOSITORY}" ]]; then | ||
| echo "Required E2E secrets are unavailable for forked PRs. Skipping E2E tests for this PR context." | ||
| echo "## E2E Tests Skipped" >> $GITHUB_STEP_SUMMARY | ||
| echo "" >> $GITHUB_STEP_SUMMARY | ||
| echo "Required E2E secrets are unavailable for forked PRs, so auth/signup E2E tests cannot run." >> $GITHUB_STEP_SUMMARY | ||
| exit 0 | ||
| fi | ||
|
|
||
| echo "Required E2E secrets are missing for an internal PR context." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial
Forked PRs still run the full expensive setup before this gate.
The secrets gate lives in the "Run E2E tests" step, so a forked PR (which legitimately lacks the secrets) executes all upstream work — building the API image (Lines 151-156), starting Docker services, installing dependencies, building the UI (Line 223), and installing Playwright browsers (Line 237) — only to exit 0 immediately here. That is a large amount of wasted runner time per fork PR.
Since this PR is scoped to match #11565 line-for-line, this likely belongs in a follow-up, but consider promoting the fork/secret check to an earlier guard (e.g., a job-level if or a lightweight precondition job that gates e2e-tests) so forked PRs short-circuit before the heavy steps run.
🤖 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/ui-e2e-tests-v2.yml around lines 239 - 256, Move the
forked-PR secrets check out of the Run E2E tests step in the ui-e2e-tests-v2
workflow so it short-circuits before the expensive setup runs. Add an earlier
guard around the e2e-tests job itself, or introduce a lightweight precondition
step/job that checks PR_HEAD_REPOSITORY against BASE_REPOSITORY and the required
E2E_* secrets before the API image build, Docker startup, UI build, and
Playwright install steps. Keep the existing skip/summary behavior for forked
PRs, but ensure the job never reaches the heavy setup when the secrets are
unavailable.
|
Superseded by #11569. |
Context
This PR is the first slice of the exact upstream split of #11565.
The original PR #11565 is the source of truth. The four chained PRs together must contain the same line-level changes as #11565, with no extra or missing changes.
Description
This slice adds the platform/CI guard for UI E2E tests so forked pull requests without repository secrets do not fail on unavailable secret-dependent steps.
Changed files:
.github/workflows/ui-e2e-tests-v2.ymlSteps to review
.github/workflows/ui-e2e-tests-v2.yml.Checklist
Community Checklist
SDK/CLI
UI
API
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Chain Context
mastermasterChain Overview
Scope
Autonomy