chore: run atoms-e2e only when ready-for-e2e label added to PR#23280
chore: run atoms-e2e only when ready-for-e2e label added to PR#23280sahitya-chandra wants to merge 1 commit intocalcom:mainfrom
Conversation
Walkthrough
Assessment against linked issues
Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
github-actions
bot
added
automated-tests
sahitya-chandra
changed the title
|
@sahitya-chandra is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/22/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (08/22/25)1 label was added to this PR based on Keith Williams's automation. "Add platform team as reviewer" took an action on this PR • (08/22/25)1 reviewer was added to this PR based on Keith Williams's automation. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
.github/workflows/atoms-e2e.yml (1)
37-50: Double-check artifact paths relative to the working directory.The test command runs in packages/platform/examples/base, but the artifact upload steps have no working-directory. If test-results and playwright-report are created under that subfolder, uploads may miss them. Consider scoping the paths or setting working-directory for those steps.
Apply one of the following:
- - name: Upload Test Results + - name: Upload Test Results uses: actions/upload-artifact@v4 if: always() with: name: atoms-e2e-test-results - path: test-results + path: packages/platform/examples/base/test-results retention-days: 7 - - name: Upload Playwright Report + - name: Upload Playwright Report uses: actions/upload-artifact@v4 if: always() with: name: atoms-e2e-playwright-report - path: playwright-report + path: packages/platform/examples/base/playwright-report retention-days: 7.github/workflows/pr.yml (4)
198-204: New atoms-e2e job wiring and gating look good.
- needs includes the necessary build jobs.
- if correctly gates on both run-e2e and has-files-requiring-all-checks.
- uses the reusable atoms-e2e workflow with secrets: inherit.
Optional hardening: If you want least-privilege, add job-level permissions here (e.g., actions: write, contents: read) to avoid relying on top-level defaults.
Example:
atoms-e2e: name: Atoms E2E Tests + permissions: + actions: write + contents: read needs: [changes, check-label, build, build-api-v1, build-api-v2, build-atoms]
214-215: merge-reports may be missing e2e-api-v2 in its dependencies.If merge-reports previously aggregated reports from e2e-api-v2, it should remain in needs to avoid silently dropping that suite from the merged output.
Proposed tweak:
- needs: [changes, check-label, e2e, e2e-embed, e2e-embed-react, e2e-app-store, atoms-e2e] + needs: [changes, check-label, e2e, e2e-api-v2, e2e-embed, e2e-embed-react, e2e-app-store, atoms-e2e]If e2e-api-v2 was intentionally excluded, please ignore.
259-266: required job: confirm “skipped” handling won’t cause false failures when label is absent.required includes atoms-e2e (and other e2e jobs) in needs, and the step fails if any need is skipped. When ready-for-e2e is not present, those jobs will be skipped by design, which could make required fail undesirably.
If the intent is “don’t fail when e2e is intentionally skipped,” adjust the step condition to only consider “skipped” as a failure when run-e2e is true:
# Replace the `if:` on Line 265 with: if: > ${{ needs.changes.outputs.has-files-requiring-all-checks == 'true' && ( (needs.check-label.outputs.run-e2e == 'true' && (contains(needs.*.result, 'failure') || contains(needs.*.result, 'skipped') || contains(needs.*.result, 'cancelled')) ) || (needs.check-label.outputs.run-e2e != 'true' && (contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')) ) ) }}If your policy is to force labeling before merge (i.e., skipping e2e should fail), then current logic is fine—just confirm that’s the intended gate.
262-262: Runner label note from actionlint.actionlint flags buildjet-2vcpu-ubuntu-2204 as unknown. If these are custom self-hosted runners (likely), consider adding them to actionlint.yaml to suppress false positives. Otherwise, switch to a GitHub-hosted label (e.g., ubuntu-22.04).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/atoms-e2e.yml(2 hunks).github/workflows/pr.yml(3 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/pr.yml
262-262: label "buildjet-2vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (1)
.github/workflows/atoms-e2e.yml (1)
4-4: Switch to reusable workflow via workflow_call looks correct.This enables pr.yml to orchestrate when atoms-e2e runs. Note that when a workflow is invoked via uses: ./.github/workflows/..., the caller’s job-level/top-level GITHUB_TOKEN permissions apply; the permissions block inside this file is ignored during reuse. Since pr.yml has permissions at the workflow level, you’re covered, but add job-level permissions in the caller if you ever need to further tighten scope.
1 participant
What does this PR do?
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist