feat: e2e tests for Connect atoms#23433
Conversation
WalkthroughAdds dotenv as a devDependency and loads environment variables in the Playwright config (uses Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
keithwillcode
added
core
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
packages/platform/examples/base/src/pages/index.tsx (1)
37-39: LGTM: navigate to /calendars on Apple connect successSimple and effective for the e2e. Consider standardizing redirect strategy across providers in a follow-up (router vs absolute
redirURLs) to reduce config drift.packages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.ts (4)
13-13: Prefer role/test-id locators over text CSS for resilience.Reduces flakiness from copy changes and i18n.
- await page.locator('[data-testid="connect-atoms"] button:has-text("Connect Apple Calendar")').click(); + await page.getByTestId("connect-atoms").getByRole("button", { name: /connect apple calendar/i }).click();
33-33: Use accessible role-based assertions.Stronger semantics; less brittle than tag + has-text.
- await expect(page.locator('h2:has-text("Add to calendar")')).toBeVisible(); + await expect(page.getByRole("heading", { level: 2, name: /add to calendar/i })).toBeVisible(); - await expect(page.locator('label:has-text("Add events to")')).toBeVisible(); + await expect(page.getByLabel(/add events to/i)).toBeVisible(); - await expect(page.locator('h4:has-text("Check for conflicts")')).toBeVisible(); + await expect(page.getByRole("heading", { level: 4, name: /check for conflicts/i })).toBeVisible();Also applies to: 35-35, 43-43
9-9: Drop redundant body visibility checks.They don’t add signal and slow the run.
- await expect(page.locator("body")).toBeVisible(); @@ - await expect(page.locator("body")).toBeVisible();Also applies to: 29-29
3-3: Mark test as slow.Long external flow; avoids CI flakes.
test("connect calendar using the apple connect atom", async ({ page }) => { + test.slow();
📜 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 (6)
packages/platform/examples/base/package.json(1 hunks)packages/platform/examples/base/playwright.config.ts(1 hunks)packages/platform/examples/base/src/pages/calendars.tsx(1 hunks)packages/platform/examples/base/src/pages/index.tsx(3 hunks)packages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.ts(1 hunks)turbo.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.tspackages/platform/examples/base/playwright.config.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.tspackages/platform/examples/base/src/pages/calendars.tsxpackages/platform/examples/base/playwright.config.tspackages/platform/examples/base/src/pages/index.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.tspackages/platform/examples/base/src/pages/calendars.tsxpackages/platform/examples/base/playwright.config.tspackages/platform/examples/base/src/pages/index.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/platform/examples/base/src/pages/calendars.tsxpackages/platform/examples/base/src/pages/index.tsx
⏰ 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). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
🔇 Additional comments (6)
packages/platform/examples/base/package.json (1)
29-29: Verify dotenv version and Node compatibility before mergeEnsure
^17.2.1exists and matches our minimum Node version across workspaces; align with any existing dotenv version in the monorepo to avoid duplicate installs.packages/platform/examples/base/src/pages/calendars.tsx (1)
12-12: LGTM: stable test selector added
data-testid="calendars-settings-atom"is clear and scoped for e2e.packages/platform/examples/base/playwright.config.ts (1)
2-4: LGTM: dotenv loaded once at module scopeThis correctly exposes
.envvars to both tests and the spawned web server process.packages/platform/examples/base/src/pages/index.tsx (2)
3-4: Intentional use of next/router acknowledged—confirm pages router scopeSince this is under
pages/,next/routeris appropriate. Keep the disable comment narrowly scoped and avoid using it inapp/codepaths.
26-26: LGTM: test hook for connect atoms
data-testid="connect-atoms"provides a stable selector for the e2e flow.packages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.ts (1)
1-60: Nice use of dialog roles and test IDs.Good accessibility-minded hooks and flow coverage.
| const appleId = process.env.APPLE_ID; | ||
| const appSpecificPassword = process.env.APPLE_CONNECT_APP_SPECIFIC_PASSCODE; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Skip when APPLE_ env vars are missing; avoid filling empty strings.*
Prevents false failures on CI and avoids sending empty creds.
+const appleId = process.env.APPLE_ID;
+const appSpecificPassword = process.env.APPLE_CONNECT_APP_SPECIFIC_PASSCODE;
+test.skip(!appleId || !appSpecificPassword, "Set APPLE_ID and APPLE_CONNECT_APP_SPECIFIC_PASSCODE to run this test");
-test("connect calendar using the apple connect atom", async ({ page }) => {
- const appleId = process.env.APPLE_ID;
- const appSpecificPassword = process.env.APPLE_CONNECT_APP_SPECIFIC_PASSCODE;
+test("connect calendar using the apple connect atom", async ({ page }) => {
@@
- await page.locator('[data-testid="apple-calendar-email"]').fill(appleId ?? "");
+ await page.locator('[data-testid="apple-calendar-email"]').fill(appleId!);
@@
- await page.locator('[data-testid="apple-calendar-password"]').fill(appSpecificPassword ?? "");
+ await page.locator('[data-testid="apple-calendar-password"]').fill(appSpecificPassword!);Also applies to: 20-20, 23-23
🤖 Prompt for AI Agents
In packages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.ts
around lines 4-6 (and also adjust lines 20 and 23), the test currently reads
APPLE_* values directly and may use empty strings which causes false CI
failures; update the file to treat empty strings as missing by trimming and
checking length (e.g., const appleId = process.env.APPLE_ID?.trim() ||
undefined), and if either required env var is missing/empty, skip the test early
(use the test framework's skip mechanism or return) so the test only runs when
valid APPLE_* credentials are present.
| const appleId = process.env.APPLE_ID; | ||
| const appSpecificPassword = process.env.APPLE_CONNECT_APP_SPECIFIC_PASSCODE; | ||
|
|
||
| await page.goto("/"); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm Playwright baseURL is set for the examples app.
Without baseURL, page.goto("/") will fail locally/CI.
Run to verify:
🏁 Script executed:
#!/bin/bash
# Check baseURL and dotenv loading in Playwright config
fd -a 'playwright.config.ts' | xargs -I{} sh -c 'echo "== {} =="; rg -n "baseURL|dotenv|config\\(" {} -n -C2'
# Ensure APPLE_* appears in turbo.json globalEnv and .env.example files
fd -a 'turbo.json' | xargs -I{} sh -c 'echo "== {} =="; cat "{}" | jq .globalEnv'
fd -a '.env.example' | xargs -I{} sh -c 'echo "== {} =="; rg -n "APPLE_ID|APPLE_CONNECT_APP_SPECIFIC_PASSCODE" "{}" -n'Length of output: 9199
Playwright baseURL is configured; add Apple credential placeholders to the example’s .env.example
packages/platform/examples/base/playwright.config.tsalready setsbaseURL: "http://localhost:4322", soawait page.goto("/")will work as expected.packages/platform/examples/base/.env.exampleis missingAPPLE_IDandAPPLE_CONNECT_APP_SPECIFIC_PASSCODEentries—add these so developers know to supply the credentials when running the Apple-connect E2E tests.
🤖 Prompt for AI Agents
In packages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.ts
around line 7, the test uses await page.goto("/") relying on playwright baseURL
but the example’s .env.example is missing Apple credential placeholders; update
packages/platform/examples/base/.env.example to add two commented placeholder
entries APPLE_ID=your_apple_id and
APPLE_CONNECT_APP_SPECIFIC_PASSCODE=your_app_specific_passcode (or similar names
matching test env usage) with brief comments so developers know to supply those
credentials when running the Apple-connect E2E tests.
|
|
||
| await page.locator('[data-testid="apple-calendar-login-button"]').click(); | ||
|
|
||
| await expect(page).toHaveURL("/calendars"); |
There was a problem hiding this comment.
Fix brittle URL assertion (exact-match vs full URL).
Use regex/suffix to avoid strict equality mismatch with baseURL.
- await expect(page).toHaveURL("/calendars");
+ await expect(page).toHaveURL(/\/calendars$/);📝 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.
| await expect(page).toHaveURL("/calendars"); | |
| await expect(page).toHaveURL(/\/calendars$/); |
🤖 Prompt for AI Agents
In packages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.ts
around line 27, the test uses a brittle exact URL equality; replace the strict
equality assertion with a pattern match (regex or suffix match) that only
asserts the path ends with "/calendars" (allowing for full baseURL, query
strings or trailing slashes) so the test passes regardless of baseURL
differences.
| await expect(page.locator('[data-testid="select-control"]')).toBeVisible(); | ||
| await page.locator('[data-testid="select-control"]').click(); | ||
|
|
||
| await page.keyboard.press("ArrowDown"); | ||
| await page.keyboard.press("Enter"); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make select interaction deterministic.
Wait for listbox and pick a concrete option instead of key presses.
- await expect(page.locator('[data-testid="select-control"]')).toBeVisible();
- await page.locator('[data-testid="select-control"]').click();
-
- await page.keyboard.press("ArrowDown");
- await page.keyboard.press("Enter");
+ await expect(page.getByTestId("select-control")).toBeVisible();
+ await page.getByTestId("select-control").click();
+ await expect(page.getByRole("listbox")).toBeVisible();
+ await page.getByRole("option").first().click();📝 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.
| await expect(page.locator('[data-testid="select-control"]')).toBeVisible(); | |
| await page.locator('[data-testid="select-control"]').click(); | |
| await page.keyboard.press("ArrowDown"); | |
| await page.keyboard.press("Enter"); | |
| await expect(page.getByTestId("select-control")).toBeVisible(); | |
| await page.getByTestId("select-control").click(); | |
| await expect(page.getByRole("listbox")).toBeVisible(); | |
| await page.getByRole("option").first().click(); |
🤖 Prompt for AI Agents
In packages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.ts
around lines 37 to 42, the test uses keyboard ArrowDown/Enter which is
non-deterministic; instead wait for the listbox element to appear after opening
the select and click a specific option directly. Replace the keyboard presses
with: await page.locator('[role="listbox"]').waitFor({ state: 'visible' }); then
find the concrete option by text or index (e.g. await
page.locator('[role="option"]', { hasText: 'Option Label' }).click() or await
page.locator('[role="option"]').nth(index).click()) and await its
visibility/click to ensure deterministic selection.
| await page.locator('[data-testid="list"] button:has(svg[data-name="start-icon"])').click(); | ||
| await page.locator('[data-testid="dialog-rejection"]').click(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Await dialog visibility before acting on it.
Eliminates race conditions.
- await page.locator('[data-testid="list"] button:has(svg[data-name="start-icon"])').click();
- await page.locator('[data-testid="dialog-rejection"]').click();
+ await page.locator('[data-testid="list"] button:has(svg[data-name="start-icon"])').click();
+ await expect(page.locator('[data-testid="dialog-rejection"]')).toBeVisible();
+ await page.locator('[data-testid="dialog-rejection"]').click();
@@
- await page.locator('[data-testid="list"] button:has(svg[data-name="start-icon"])').click();
- await page.locator('[data-testid="dialog-confirmation"]').click();
+ await page.locator('[data-testid="list"] button:has(svg[data-name="start-icon"])').click();
+ await expect(page.locator('[data-testid="dialog-confirmation"]')).toBeVisible();
+ await page.locator('[data-testid="dialog-confirmation"]').click();Also applies to: 58-59
🤖 Prompt for AI Agents
In packages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.ts
around lines 46-47 (and similarly for lines 58-59), the test clicks the dialog
rejection button immediately which can race with the dialog appearing; wait for
the dialog to be visible before interacting with it. Modify the test to await
the dialog locator being visible (e.g. waitForSelector or locator.waitFor with
state 'visible') prior to calling click, and apply the same change to the other
occurrence on lines 58-59.
| await expect(page.locator('[data-testid="list"] button[role="switch"]').first()).toBeVisible(); | ||
| await page.locator('[data-testid="list"] button[role="switch"]').first().click(); | ||
|
|
||
| await expect(page.locator('[data-testid="list"] button[role="switch"]').last()).toBeVisible(); | ||
| await page.locator('[data-testid="list"] button[role="switch"]').last().click(); | ||
|
|
||
| await expect(page.locator('[data-testid="list"] button[role="switch"]').first()).toBeVisible(); | ||
| await page.locator('[data-testid="list"] button[role="switch"]').first().click(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Assert switch state changes, not just clicks.
Validates behavior via aria-checked.
- await expect(page.locator('[data-testid="list"] button[role="switch"]').first()).toBeVisible();
- await page.locator('[data-testid="list"] button[role="switch"]').first().click();
-
- await expect(page.locator('[data-testid="list"] button[role="switch"]').last()).toBeVisible();
- await page.locator('[data-testid="list"] button[role="switch"]').last().click();
-
- await expect(page.locator('[data-testid="list"] button[role="switch"]').first()).toBeVisible();
- await page.locator('[data-testid="list"] button[role="switch"]').first().click();
+ const list = page.getByTestId("list");
+ const firstSwitch = list.getByRole("switch").first();
+ await expect(firstSwitch).toBeVisible();
+ const prevFirst = await firstSwitch.getAttribute("aria-checked");
+ await firstSwitch.click();
+ await expect(firstSwitch).toHaveAttribute("aria-checked", prevFirst === "true" ? "false" : "true");
+
+ const lastSwitch = list.getByRole("switch").last();
+ await expect(lastSwitch).toBeVisible();
+ const prevLast = await lastSwitch.getAttribute("aria-checked");
+ await lastSwitch.click();
+ await expect(lastSwitch).toHaveAttribute("aria-checked", prevLast === "true" ? "false" : "true");
+
+ await firstSwitch.click(); // revert
+ await expect(firstSwitch).toHaveAttribute("aria-checked", prevFirst);📝 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.
| await expect(page.locator('[data-testid="list"] button[role="switch"]').first()).toBeVisible(); | |
| await page.locator('[data-testid="list"] button[role="switch"]').first().click(); | |
| await expect(page.locator('[data-testid="list"] button[role="switch"]').last()).toBeVisible(); | |
| await page.locator('[data-testid="list"] button[role="switch"]').last().click(); | |
| await expect(page.locator('[data-testid="list"] button[role="switch"]').first()).toBeVisible(); | |
| await page.locator('[data-testid="list"] button[role="switch"]').first().click(); | |
| const list = page.getByTestId("list"); | |
| const firstSwitch = list.getByRole("switch").first(); | |
| await expect(firstSwitch).toBeVisible(); | |
| const prevFirst = await firstSwitch.getAttribute("aria-checked"); | |
| await firstSwitch.click(); | |
| await expect(firstSwitch).toHaveAttribute("aria-checked", prevFirst === "true" ? "false" : "true"); | |
| const lastSwitch = list.getByRole("switch").last(); | |
| await expect(lastSwitch).toBeVisible(); | |
| const prevLast = await lastSwitch.getAttribute("aria-checked"); | |
| await lastSwitch.click(); | |
| await expect(lastSwitch).toHaveAttribute("aria-checked", prevLast === "true" ? "false" : "true"); | |
| await firstSwitch.click(); // revert | |
| await expect(firstSwitch).toHaveAttribute("aria-checked", prevFirst); |
🤖 Prompt for AI Agents
In packages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.ts
around lines 49 to 56, the test only clicks the switch elements without
asserting their state change; update the test to assert the switch's
aria-checked state after each interaction (use
locator.getAttribute('aria-checked') or Playwright's toHaveAttribute matcher) to
confirm toggling (e.g., check aria-checked flips from "false" to "true" after
click and back after subsequent click) so each click is followed by an explicit
assertion of the new state.
turbo.json
Outdated
| "BUILD_STANDALONE", | ||
| "APPLE_ID", | ||
| "APPLE_CONNECT_APP_SPECIFIC_PASSCODE" |
There was a problem hiding this comment.
Remove APPLE_ from globalEnv to prevent secret sprawl*
Adding APPLE_ID and APPLE_CONNECT_APP_SPECIFIC_PASSCODE to globalEnv propagates these secrets to all tasks in the monorepo, increasing leakage risk. Keep them scoped to the examples app via .env loaded by Playwright; they don’t need to be global.
Apply this diff:
- "BUILD_STANDALONE",
- "APPLE_ID",
- "APPLE_CONNECT_APP_SPECIFIC_PASSCODE"
+ "BUILD_STANDALONE"I can add entries to packages/platform/examples/base/.env.example and a short README note for local setup if you want.
📝 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.
| "BUILD_STANDALONE", | |
| "APPLE_ID", | |
| "APPLE_CONNECT_APP_SPECIFIC_PASSCODE" | |
| "BUILD_STANDALONE" |
🤖 Prompt for AI Agents
In turbo.json around lines 257 to 259, remove "APPLE_ID" and
"APPLE_CONNECT_APP_SPECIFIC_PASSCODE" from the globalEnv array so these secrets
are not propagated to all tasks; instead, document and add them to
packages/platform/examples/base/.env.example and instruct Playwright to load
them from the example app's .env (and add a short README note for local setup).
supalarry
left a comment
supalarry
left a comment
There was a problem hiding this comment.
Good stuff Rajiv! Some comments:
| import dotenv from "dotenv"; | ||
| import path from "path"; | ||
|
|
||
| dotenv.config({ path: path.resolve(__dirname, ".env.tests.ci") }); |
There was a problem hiding this comment.
We need to add ATOMS_E2E_APPLE_ID and ATOMS_E2E_APPLE_CONNECT_APP_SPECIFIC_PASSCODE on line 45 if process.env.CI is true - right now connect atom would not have credentials in the CI.
Also, can you ask Morgan do add ATOMS_E2E_APPLE_ID and ATOMS_E2E_APPLE_CONNECT_APP_SPECIFIC_PASSCODE to CI env so these values are available?
| import dotenv from "dotenv"; | ||
| import path from "path"; | ||
|
|
||
| const envPath = process.env.CI ? path.resolve(__dirname, ".env") : path.resolve(__dirname, ".env.local"); |
There was a problem hiding this comment.
In prod we would not have an .env file - all of the variables are already in CI environment but its fine - lets just get this merged and I have started refactoring e2e setup locally with separate test database, database cleanup etc.
There was a problem hiding this comment.
FYI there should be env.local comitted to this PR containing all the keys necessary for the local e2e test and then dev fills them manually.
usually you call it env.local.example and commit it and then in gitignore ignore env.local because you rename env.local.example to env.local and fill necessary keys.
There was a problem hiding this comment.
Its fine for now as longs as e2e in ci pass because i will fix it in separate PR
E2E results are ready! |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/e2e-atoms.yml (1)
45-51: Avoid leaking secrets via Playwright artifacts.Reports, screenshots, videos, and traces can capture typed Apple ID/passcode. Either disable artifact upload for secret‑entering flows or enable masking/scrubbing in Playwright config (mask inputs, disable video/screenshots for that spec, scrub HAR).
Suggested actions:
- In Playwright config/spec: set tracing with scrubbed data and mask inputs for Apple fields; disable video/screenshots for this test file.
- In CI: conditionally skip uploading playwright-report/test-results when Apple connect spec runs or post‑process to redact.
Want me to propose concrete Playwright config snippets to mask inputs and scrub traces?
🧹 Nitpick comments (3)
.github/workflows/e2e-atoms.yml (3)
21-37: Scope secrets to the test step to reduce exposure.These creds are currently available to all steps (including third‑party actions). Limit them to the Playwright run step.
jobs: e2e-atoms: @@ - steps: + steps: - uses: docker/login-action@v3 with: username: ${{ secrets.DOCKERHUB_USERNAME }} password: ${{ secrets.DOCKERHUB_TOKEN }} - uses: actions/checkout@v4 - uses: ./.github/actions/dangerous-git-checkout - uses: ./.github/actions/yarn-install - uses: ./.github/actions/yarn-playwright-install - name: Run E2E Atoms Tests working-directory: packages/platform/examples/base + env: + APPLE_ID: ${{ secrets.ATOMS_E2E_APPLE_ID }} + APPLE_CONNECT_APP_SPECIFIC_PASSCODE: ${{ secrets.ATOMS_E2E_APPLE_CONNECT_APP_SPECIFIC_PASSCODE }} + ATOMS_E2E_APPLE_ID: ${{ secrets.ATOMS_E2E_APPLE_ID }} # if turbo globalEnv still relies on these + ATOMS_E2E_APPLE_CONNECT_APP_SPECIFIC_PASSCODE: ${{ secrets.ATOMS_E2E_APPLE_CONNECT_APP_SPECIFIC_PASSCODE }} run: yarn test:e2e
10-10: Nit: use standard 8192 MiB value.Likely intended 8192 instead of 8096.
- NODE_OPTIONS: --max-old-space-size=8096 + NODE_OPTIONS: --max-old-space-size=8192
23-23: Consider bumping timeout to 30 minutes to reduce E2E flakiness.Cloud latency + Apple auth can spike; 15 min can be tight.
- timeout-minutes: 15 + timeout-minutes: 30
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/e2e-atoms.yml(1 hunks)
🔇 Additional comments (1)
.github/workflows/e2e-atoms.yml (1)
16-17: Incorrect — env var names already match; no change required.The workflow exports ATOMS_E2E_APPLE_ID and ATOMS_E2E_APPLE_CONNECT_APP_SPECIFIC_PASSCODE and the tests read process.env.ATOMS_E2E_APPLE_ID / ATOMS_E2E_APPLE_CONNECT_APP_SPECIFIC_PASSCODE, so no aliasing/renaming is necessary. See .github/workflows/e2e-atoms.yml (env lines ~16–17), packages/platform/examples/base/tests/connect-atoms/apple-connect.e2e.ts (lines ~4–5), and turbo.json (entries ~279–280).
Likely an incorrect or invalid review comment.
3 participants
What does this PR do?
APPLE_IDandAPPLE_CONNECT_APP_SPECIFIC_PASSCODEto examples base .envVideo Demo:
Screen.Recording.2025-08-29.at.2.35.41.AM.mov
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
This can be tested in examples app