Conversation
WalkthroughThe CircleCI e2e workflows were updated to run Playwright-based tests, inject environment variables into x-e2e’s .env, adjust test commands and reporters, revise artifact collection paths, and change Slack result parsing and notifications to consume Playwright JSON output. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CI as CircleCI Job (e2e-tests / e2e-playwright)
participant Repo as x-e2e Repo
participant PW as Playwright Runner
participant FS as Artifact FS (/tmp)
participant Slack as Slack Webhook
CI->>Repo: Checkout x-e2e
CI->>Repo: Write .env from injected environment variables
CI->>PW: npm run test:metamask -- --reporter=list,html,json
PW-->>CI: Exit code + outputs (JSON, traces, screenshots, HTML report)
CI->>FS: Move test-results and playwright-report to /tmp
CI->>CI: Store CircleCI artifacts (/tmp/test-results, /tmp/playwright-report)
CI->>CI: Read /tmp/test-results/.last-run.json (status, failedTests)
alt status == success
CI->>Slack: Send "All Playwright UI tests passed" + HTML report link
else status == failure
CI->>Slack: Send "Playwright UI Tests Failed" + failed tests + report link
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
Deploying x with
|
| Latest commit: |
3a0cd7a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://78b8c4b2.x-e62.pages.dev |
| Branch Preview URL: | https://playright-html.x-e62.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.circleci/config.yml (1)
132-149: Sed replacements can corrupt values containing&, backslashes, or newlines; also fails if.envis missing.Environment values like URLs or tokens commonly include
&. In sed replacements,&expands to the matched text, breaking the output. Also, this assumes.envexists.Suggested safer pattern within this step:
- # Inject environment variables into .env file + # Inject environment variables into .env file (safe escaping; ensure .env exists) + escape_sed() { printf '%s' "$1" | sed -e 's/[\/&]/\\&/g'; } + [ -f .env ] || { [ -f .env.example ] && cp .env.example .env || touch .env; } sed -i "s|base_url_value|$BASE_URL|g" .env - sed -i "s|dry_run_value|$DRY_RUN|g" .env + sed -i "s|dry_run_value|$(escape_sed "$DRY_RUN")|g" .env sed -i "s|master_email_value|$MASTER_EMAIL|g" .env sed -i "s|master_wallet_value|$MASTER_WALLET|g" .env - sed -i "s|testmail_api_key_value|$TESTMAIL_API_KEY|g" .env + sed -i "s|testmail_api_key_value|$(escape_sed "$TESTMAIL_API_KEY")|g" .env sed -i "s|testmail_namespace_value|$TESTMAIL_NAMESPACE|g" .env sed -i "s|recovery_recipient_address_value|$RECOVERY_RECIPIENT_ADDRESS|g" .env sed -i "s|minimum_amount_value|$MINIMUM_AMOUNT|g" .env sed -i "s|headless_value|$HEADLESS|g" .env sed -i "s|timeout_value|$TIMEOUT|g" .env sed -i "s|debug_value|$DEBUG|g" .env sed -i "s|ci_value|$CI|g" .env sed -i "s|headed_value|$HEADED|g" .env - sed -i "s|sushi_url_value|$SUSHI_URL|g" .env - sed -i "s|staging_url_value|$STAGING_URL|g" .env + sed -i "s|sushi_url_value|$(escape_sed "$SUSHI_URL")|g" .env + sed -i "s|staging_url_value|$(escape_sed "$STAGING_URL")|g" .env sed -i "s|testmail_username_value|$TESTMAIL_USERNAME|g" .env sed -i "s|testmail_email_value|$TESTMAIL_EMAIL|g" .env
🧹 Nitpick comments (2)
.circleci/config.yml (2)
156-174: Ensure artifact dirs always exist to prevent downstream failures.
store_artifactsfails if the path is missing. Create placeholders so the artifact steps don’t error when directories aren’t produced.Apply:
echo "Contents of project root before move:" ls -lah /root/x-e2e/ || true - + # Ensure artifact directories exist to avoid store_artifacts failures + mkdir -p /tmp/test-results /tmp/playwright-report + # Move test results (JSON, traces, screenshots) if [ -d "/root/x-e2e/test-results" ]; then mv /root/x-e2e/test-results /tmp/test-results chmod 777 -R /tmp/test-results fi
125-131: Cloning with token in URL—confirm masking and consider pinning a commit/branch.CircleCI usually masks
$GITHUB_TOKEN, but URLs can still leak in some contexts. Also, cloning a mutable branch (pillarx-playwright) makes runs non‑reproducible.
- Prefer
git config url."https://oauth2:${GITHUB_TOKEN}@github.com/".insteadOf "https://github.com/"and clone the normal URL.- Pin to a commit or tag for deterministic CI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.circleci/config.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: unit-tests
🔇 Additional comments (2)
.circleci/config.yml (2)
153-154: Verify Playwright reporter outputs and availability of.last-run.json.
--reporter=list,html,jsonprints JSON to stdout; combining withlistcan corrupt JSON if you later redirect it. Your Slack step relies on/tmp/test-results/.last-run.jsonand fields.statusand.failedTests— these are version/feature-dependent.Consider either:
- Remove
listand redirect JSON reporter to a file (ensures valid JSON to parse), or- Keep current reporters but confirm that
.last-run.jsonis generated with the expected schema in your Playwright version. If not, adjust parsing or write a small post-run script to summarize failed tests.
177-182: Artifacts paths are correct; paired with the move step they’ll publish as expected.Confirm CircleCI doesn’t fail the job when the directories are empty but present (the mkdir fix above addresses this).
3 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
Tests
Chores