Merged
Conversation
Contributor
WalkthroughThe PR updates CircleCI configuration to replace mochawesome-based test execution with a Playwright-focused E2E pipeline, including environment variable injection into Changes
Sequence DiagramsequenceDiagram
actor CircleCI as CircleCI Job
participant x-e2e as x-e2e Repo
participant Playwright as Playwright Tests
participant Artifacts as /tmp Artifacts
participant CCAPI as CircleCI API
participant Slack as Slack
CircleCI->>x-e2e: Clone repository
CircleCI->>CircleCI: Inject env vars to .env
CircleCI->>Playwright: Install deps & run tests
Playwright->>Artifacts: Generate blob-report & test-results
rect rgb(200, 220, 240)
Note over CircleCI,Slack: Dynamic Report URL Resolution
CircleCI->>CCAPI: Query artifacts for HTML report
alt Report found
CCAPI-->>CircleCI: Return artifact URL
else Fallback
CircleCI->>CircleCI: Use fallback URL
end
end
rect rgb(220, 240, 200)
Note over CircleCI,Slack: Slack Notification
CircleCI->>Slack: Send notification with Job URL & Report URL
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
Deploying x with
|
| Latest commit: |
3699812
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://34617adc.x-e62.pages.dev |
| Branch Preview URL: | https://e2e.x-e62.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.circleci/config.yml (1)
45-66: Artifact handling looks solid, with minor note on permissions.The defensive checks (
if [ -d ... ]) before moving artifacts are good. However,chmod 777is overly permissive. Since these are temporary CI artifacts in/tmp, considerchmod 755orchmod 644(depending on whether others need write access):- chmod 777 -R /tmp/blob-report + chmod 755 -R /tmp/blob-reportThis reduces exposure if the artifact directory is later shared or inspected by other users.
📜 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). (4)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
- GitHub Check: Cloudflare Pages: pillarx-debug
🔇 Additional comments (5)
.circleci/config.yml (5)
16-20: Install jq step is appropriate.This step is necessary for the JSON parsing used in Slack reporting. Consider pinning package versions for reproducibility in future updates.
68-79: HTML report artifact handling looks good.Consistent with the blob-report and test-results artifact handling. Same minor note: consider
chmod 755instead of the implicit default for consistency with the earlier artifacts (see prior comment).
109-125: Slack message structure is clear and informative.The passed and failed test messages include relevant context (project, branch, job link, report link). The newline escaping on line 128 is appropriate for JSON formatting.
135-138: Branch filter is sensible.Restricting E2E tests to the
stagingbranch prevents them from running on every PR branch, which reduces CI load. Ensure this aligns with your workflow expectations.
28-28: High-risk: Credential exposure and unsafe environment variable substitution.Two issues here:
Credential leakage (line 28): The git clone exposes
$GITHUB_TOKENin the command, which will appear in CI logs. While CircleCI attempts to mask known secrets, relying on log masking is fragile. Use Git credential helpers or SSH keys instead.Sed injection vulnerability (lines 31–39): The sed commands lack escaping for special characters. If environment variables contain
/,&,$, or\, sed will fail or cause unintended substitution. For example, an email liketest&user@example.comwill break the sed command.Use
envsubst(POSIX standard) or safely escape variables:- touch .env - sed -i "s|test_mail_email_value|$TESTMAIL_EMAIL|g" .env - sed -i "s|test_mail_username_value|$TESTMAIL_USERNAME|g" .env - sed -i "s|test_mail_apikey_value|$TESTMAIL_API_KEY|g" .env - sed -i "s|staging_url_value|$STAGING_URL|g" .env - sed -i "s|sushi_url_value|$SUSHI_URL|g" .env - sed -i "s|ci_value|$CI|g" .env - sed -i "s|headed_value|$HEADED|g" .env - sed -i "s|debug_value|$DEBUG|g" .env + touch .env + envsubst < .env.template > .env(Assumes x-e2e repo has a
.env.templatewith placeholders like${TESTMAIL_EMAIL}, etc.)Also applies to: 31-39
⛔ Skipped due to learnings
Learnt from: chetanbothra Repo: pillarwallet/x PR: 333 File: .circleci/config.yml:24-27 Timestamp: 2025-06-16T04:17:05.080Z Learning: In this project, tokens are stored securely in CircleCI project settings and GitHub automatically masks sensitive information in logs by printing **, so using HTTPS URLs with embedded tokens in CircleCI workflows is acceptable and secure.
| command: | | ||
| cd ~ | ||
| git clone -b main --single-branch https://$GITHUB_TOKEN@github.com/pillarwallet/x-e2e.git | ||
| git clone -b pillarx-playwright --single-branch https://$GITHUB_TOKEN@github.com/pillarwallet/x-e2e.git |
Contributor
There was a problem hiding this comment.
Critical: Test failures are suppressed by || true; risk of silent failures.
Line 43 uses || true, which suppresses the test exit code. This means the CI job reports success even when Playwright tests fail. Tests should exit with a non-zero status on failure so the pipeline can be marked as failed.
Remove || true so test failures properly fail the job:
- npm run test:all || true
+ npm run test:allIf you need to continue even after failures (to collect artifacts), use a conditional instead:
- npm run test:all || true
+ npm run test:all; TEST_EXIT=$?
+ exit $TEST_EXITAlso applies to: 43-43
🤖 Prompt for AI Agents
In .circleci/config.yml around line 28 (and also apply to line 43), the pipeline
is suppressing test failures with "|| true" which masks Playwright exit codes;
remove the "|| true" so the job exits non-zero on test failures, and if you
still need to collect artifacts after failures wrap the test command in a
conditional step or run tests in a separate step that always runs (using
CircleCI "when: always" or save-artifacts on failure) instead of silencing the
exit code.
Comment on lines
+81
to
+129
| # Generate report URL dynamically (fixes 404) | ||
| - run: | ||
| name: Parse Playwright results and send Slack notification | ||
| name: Parse Playwright results and send Slack | ||
| when: always | ||
| command: | | ||
| RESULTS_FILE="/tmp/test-results/.last-run.json" | ||
|
|
||
| if [ ! -f "$RESULTS_FILE" ]; then | ||
| echo ".last-run.json not found — cannot generate Slack report" | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Fetch real signed artifact URL from CircleCI API | ||
| REPORT_URL=$(curl -s -H "Circle-Token: $CIRCLECI_TOKEN" \ | ||
| "https://circleci.com/api/v2/project/github/${CIRCLE_PROJECT_USERNAME}/${CIRCLE_PROJECT_REPONAME}/${CIRCLE_BUILD_NUM}/artifacts" \ | ||
| | jq -r '.items[] | select(.path | endswith("playwright-report/index.html")) | .url') | ||
|
|
||
|
|
||
| if [ -z "$REPORT_URL" ]; then | ||
| REPORT_URL="No report found" | ||
| fi | ||
|
|
||
| STATUS=$(jq -r '.status' $RESULTS_FILE) | ||
| FAILED_TESTS=$(jq -r '.failedTests | join(", ")' $RESULTS_FILE) | ||
|
|
||
|
|
||
| JOB_URL="https://app.circleci.com/pipelines/github/${CIRCLE_PROJECT_USERNAME}/${CIRCLE_PROJECT_REPONAME}/${CIRCLE_BUILD_NUM}" | ||
|
|
||
| if [ "$STATUS" == "passed" ]; then | ||
| SLACK_MESSAGE=":white_check_mark: All Playwright UI tests passed | ||
| *Project:* ${CIRCLE_PROJECT_REPONAME} | ||
| *Triggered by:* ${CIRCLE_USERNAME} | ||
| *Branch:* ${CIRCLE_BRANCH} | ||
| *Job:* <${JOB_URL}|Open Job> | ||
| *Commit:* <https://github.com/${CIRCLE_PROJECT_USERNAME}/${CIRCLE_PROJECT_REPONAME}/commit/${CIRCLE_SHA1}|${CIRCLE_SHA1}> | ||
| *Report:* <https://output.circle-artifacts.com/output/job/${CIRCLE_WORKFLOW_JOB_ID}/artifacts/${CIRCLE_NODE_INDEX}/tmp/playwright-report/index.html|View HTML Report>" | ||
| *Report:* <${REPORT_URL}|View HTML Report>" | ||
| else | ||
| SLACK_MESSAGE=":x: *Playwright UI Tests Failed :x:* | ||
| SLACK_MESSAGE=":x: *Playwright UI Tests Failed* | ||
| *Project:* ${CIRCLE_PROJECT_REPONAME} | ||
| *Triggered by:* ${CIRCLE_USERNAME} | ||
| *Branch:* ${CIRCLE_BRANCH} | ||
| *Commit:* <https://github.com/${CIRCLE_PROJECT_USERNAME}/${CIRCLE_PROJECT_REPONAME}/commit/${CIRCLE_SHA1}|${CIRCLE_SHA1}> | ||
| *Job:* <${JOB_URL}|Open Job> | ||
| *Failed Tests:* ${FAILED_TESTS} | ||
| *Report:* <https://output.circle-artifacts.com/output/job/${CIRCLE_WORKFLOW_JOB_ID}/artifacts/${CIRCLE_NODE_INDEX}/tmp/playwright-report/index.html|View HTML Report>" | ||
| *Commit:* <https://github.com/${CIRCLE_PROJECT_USERNAME}/${CIRCLE_PROJECT_REPONAME}/commit/${CIRCLE_SHA1}|${CIRCLE_SHA1}> | ||
| *Report:* <${REPORT_URL}|View HTML Report>" | ||
| fi | ||
|
|
||
| curl -X POST -H 'Content-type: application/json' \ | ||
| --data "{\"text\":\"${SLACK_MESSAGE}\"}" $E2E_SLACK_WEBHOOK_URL | ||
|
|
||
| curl -X POST -H 'Content-type: application/json' \ | ||
| --data "{\"text\":\"${SLACK_MESSAGE//$'\n'/\\n}\"}" \ | ||
| $E2E_SLACK_WEBHOOK_URL |
Contributor
There was a problem hiding this comment.
Missing error handling for CircleCI API calls and JSON parsing.
The Slack reporting step assumes several things that may fail silently:
-
Line 94:
$CIRCLECI_TOKENis set and valid. If missing or expired, the API call returns an error or 401, and jq silently returns empty, settingREPORT_URLto "No report found" without distinguishing between a retrieval error and a legitimate missing report. -
Line 103–104:
.last-run.jsoncontainsstatusandfailedTestsfields with the expected structure. If the schema changes or the file is corrupted, jq parsing fails silently. -
Line 106:
CIRCLE_BUILD_NUMexists. If missing, the job URL will be malformed.
Add validation and logging:
+ # Validate required env vars
+ for var in CIRCLECI_TOKEN CIRCLE_PROJECT_USERNAME CIRCLE_PROJECT_REPONAME CIRCLE_BUILD_NUM CIRCLE_SHA1 E2E_SLACK_WEBHOOK_URL; do
+ if [ -z "${!var}" ]; then
+ echo "ERROR: $var is not set"
+ exit 1
+ fi
+ done
+
RESULTS_FILE="/tmp/test-results/.last-run.json"
if [ ! -f "$RESULTS_FILE" ]; then
echo ".last-run.json not found — cannot generate Slack report"
exit 0
fi
# Fetch real signed artifact URL from CircleCI API
+ REPORT_RESPONSE=$(curl -s -w "\n%{http_code}" -H "Circle-Token: $CIRCLECI_TOKEN" \
+ "https://circleci.com/api/v2/project/github/${CIRCLE_PROJECT_USERNAME}/${CIRCLE_PROJECT_REPONAME}/${CIRCLE_BUILD_NUM}/artifacts")
+ HTTP_CODE=$(echo "$REPORT_RESPONSE" | tail -n1)
+ REPORT_JSON=$(echo "$REPORT_RESPONSE" | head -n-1)
+
+ if [ "$HTTP_CODE" != "200" ]; then
+ echo "ERROR: CircleCI API returned HTTP $HTTP_CODE"
+ REPORT_URL="Unable to retrieve report URL (API error)"
+ else
- REPORT_URL=$(curl -s -H "Circle-Token: $CIRCLECI_TOKEN" \
- "https://circleci.com/api/v2/project/github/${CIRCLE_PROJECT_USERNAME}/${CIRCLE_PROJECT_REPONAME}/${CIRCLE_BUILD_NUM}/artifacts" \
- | jq -r '.items[] | select(.path | endswith("playwright-report/index.html")) | .url')
+ REPORT_URL=$(echo "$REPORT_JSON" | jq -r '.items[] | select(.path | endswith("playwright-report/index.html")) | .url')
+ fi
if [ -z "$REPORT_URL" ]; then
REPORT_URL="No report found"
fi
+
+ # Validate JSON structure
+ if ! jq -e '.status' "$RESULTS_FILE" > /dev/null 2>&1; then
+ echo "ERROR: .last-run.json missing 'status' field"
+ exit 1
+ fi📝 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.
Suggested change
| # Generate report URL dynamically (fixes 404) | |
| - run: | |
| name: Parse Playwright results and send Slack notification | |
| name: Parse Playwright results and send Slack | |
| when: always | |
| command: | | |
| RESULTS_FILE="/tmp/test-results/.last-run.json" | |
| if [ ! -f "$RESULTS_FILE" ]; then | |
| echo ".last-run.json not found — cannot generate Slack report" | |
| exit 0 | |
| fi | |
| # Fetch real signed artifact URL from CircleCI API | |
| REPORT_URL=$(curl -s -H "Circle-Token: $CIRCLECI_TOKEN" \ | |
| "https://circleci.com/api/v2/project/github/${CIRCLE_PROJECT_USERNAME}/${CIRCLE_PROJECT_REPONAME}/${CIRCLE_BUILD_NUM}/artifacts" \ | |
| | jq -r '.items[] | select(.path | endswith("playwright-report/index.html")) | .url') | |
| if [ -z "$REPORT_URL" ]; then | |
| REPORT_URL="No report found" | |
| fi | |
| STATUS=$(jq -r '.status' $RESULTS_FILE) | |
| FAILED_TESTS=$(jq -r '.failedTests | join(", ")' $RESULTS_FILE) | |
| JOB_URL="https://app.circleci.com/pipelines/github/${CIRCLE_PROJECT_USERNAME}/${CIRCLE_PROJECT_REPONAME}/${CIRCLE_BUILD_NUM}" | |
| if [ "$STATUS" == "passed" ]; then | |
| SLACK_MESSAGE=":white_check_mark: All Playwright UI tests passed | |
| *Project:* ${CIRCLE_PROJECT_REPONAME} | |
| *Triggered by:* ${CIRCLE_USERNAME} | |
| *Branch:* ${CIRCLE_BRANCH} | |
| *Job:* <${JOB_URL}|Open Job> | |
| *Commit:* <https://github.com/${CIRCLE_PROJECT_USERNAME}/${CIRCLE_PROJECT_REPONAME}/commit/${CIRCLE_SHA1}|${CIRCLE_SHA1}> | |
| *Report:* <https://output.circle-artifacts.com/output/job/${CIRCLE_WORKFLOW_JOB_ID}/artifacts/${CIRCLE_NODE_INDEX}/tmp/playwright-report/index.html|View HTML Report>" | |
| *Report:* <${REPORT_URL}|View HTML Report>" | |
| else | |
| SLACK_MESSAGE=":x: *Playwright UI Tests Failed :x:* | |
| SLACK_MESSAGE=":x: *Playwright UI Tests Failed* | |
| *Project:* ${CIRCLE_PROJECT_REPONAME} | |
| *Triggered by:* ${CIRCLE_USERNAME} | |
| *Branch:* ${CIRCLE_BRANCH} | |
| *Commit:* <https://github.com/${CIRCLE_PROJECT_USERNAME}/${CIRCLE_PROJECT_REPONAME}/commit/${CIRCLE_SHA1}|${CIRCLE_SHA1}> | |
| *Job:* <${JOB_URL}|Open Job> | |
| *Failed Tests:* ${FAILED_TESTS} | |
| *Report:* <https://output.circle-artifacts.com/output/job/${CIRCLE_WORKFLOW_JOB_ID}/artifacts/${CIRCLE_NODE_INDEX}/tmp/playwright-report/index.html|View HTML Report>" | |
| *Commit:* <https://github.com/${CIRCLE_PROJECT_USERNAME}/${CIRCLE_PROJECT_REPONAME}/commit/${CIRCLE_SHA1}|${CIRCLE_SHA1}> | |
| *Report:* <${REPORT_URL}|View HTML Report>" | |
| fi | |
| curl -X POST -H 'Content-type: application/json' \ | |
| --data "{\"text\":\"${SLACK_MESSAGE}\"}" $E2E_SLACK_WEBHOOK_URL | |
| curl -X POST -H 'Content-type: application/json' \ | |
| --data "{\"text\":\"${SLACK_MESSAGE//$'\n'/\\n}\"}" \ | |
| $E2E_SLACK_WEBHOOK_URL | |
| # Generate report URL dynamically (fixes 404) | |
| - run: | |
| name: Parse Playwright results and send Slack | |
| when: always | |
| command: | | |
| # Validate required env vars | |
| for var in CIRCLECI_TOKEN CIRCLE_PROJECT_USERNAME CIRCLE_PROJECT_REPONAME CIRCLE_BUILD_NUM CIRCLE_SHA1 E2E_SLACK_WEBHOOK_URL; do | |
| if [ -z "${!var}" ]; then | |
| echo "ERROR: $var is not set" | |
| exit 1 | |
| fi | |
| done | |
| RESULTS_FILE="/tmp/test-results/.last-run.json" | |
| if [ ! -f "$RESULTS_FILE" ]; then | |
| echo ".last-run.json not found — cannot generate Slack report" | |
| exit 0 | |
| fi | |
| # Fetch real signed artifact URL from CircleCI API | |
| REPORT_RESPONSE=$(curl -s -w "\n%{http_code}" -H "Circle-Token: $CIRCLECI_TOKEN" \ | |
| "https://circleci.com/api/v2/project/github/${CIRCLE_PROJECT_USERNAME}/${CIRCLE_PROJECT_REPONAME}/${CIRCLE_BUILD_NUM}/artifacts") | |
| HTTP_CODE=$(echo "$REPORT_RESPONSE" | tail -n1) | |
| REPORT_JSON=$(echo "$REPORT_RESPONSE" | head -n-1) | |
| if [ "$HTTP_CODE" != "200" ]; then | |
| echo "ERROR: CircleCI API returned HTTP $HTTP_CODE" | |
| REPORT_URL="Unable to retrieve report URL (API error)" | |
| else | |
| REPORT_URL=$(echo "$REPORT_JSON" | jq -r '.items[] | select(.path | endswith("playwright-report/index.html")) | .url') | |
| fi | |
| if [ -z "$REPORT_URL" ]; then | |
| REPORT_URL="No report found" | |
| fi | |
| # Validate JSON structure | |
| if ! jq -e '.status' "$RESULTS_FILE" > /dev/null 2>&1; then | |
| echo "ERROR: .last-run.json missing 'status' field" | |
| exit 1 | |
| fi | |
| STATUS=$(jq -r '.status' $RESULTS_FILE) | |
| FAILED_TESTS=$(jq -r '.failedTests | join(", ")' $RESULTS_FILE) | |
| JOB_URL="https://app.circleci.com/pipelines/github/${CIRCLE_PROJECT_USERNAME}/${CIRCLE_PROJECT_REPONAME}/${CIRCLE_BUILD_NUM}" | |
| if [ "$STATUS" == "passed" ]; then | |
| SLACK_MESSAGE=":white_check_mark: All Playwright UI tests passed | |
| *Project:* ${CIRCLE_PROJECT_REPONAME} | |
| *Triggered by:* ${CIRCLE_USERNAME} | |
| *Branch:* ${CIRCLE_BRANCH} | |
| *Job:* <${JOB_URL}|Open Job> | |
| *Commit:* <https://github.com/${CIRCLE_PROJECT_USERNAME}/${CIRCLE_PROJECT_REPONAME}/commit/${CIRCLE_SHA1}|${CIRCLE_SHA1}> | |
| *Report:* <${REPORT_URL}|View HTML Report>" | |
| else | |
| SLACK_MESSAGE=":x: *Playwright UI Tests Failed* | |
| *Project:* ${CIRCLE_PROJECT_REPONAME} | |
| *Triggered by:* ${CIRCLE_USERNAME} | |
| *Branch:* ${CIRCLE_BRANCH} | |
| *Job:* <${JOB_URL}|Open Job> | |
| *Failed Tests:* ${FAILED_TESTS} | |
| *Commit:* <https://github.com/${CIRCLE_PROJECT_USERNAME}/${CIRCLE_PROJECT_REPONAME}/commit/${CIRCLE_SHA1}|${CIRCLE_SHA1}> | |
| *Report:* <${REPORT_URL}|View HTML Report>" | |
| fi | |
| curl -X POST -H 'Content-type: application/json' \ | |
| --data "{\"text\":\"${SLACK_MESSAGE//$'\n'/\\n}\"}" \ | |
| $E2E_SLACK_WEBHOOK_URL |
Deploying pillarx-debug with
|
| Latest commit: |
3699812
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://09da84fd.pillarx-debug.pages.dev |
| Branch Preview URL: | https://e2e.pillarx-debug.pages.dev |
boonet
approved these changes
Nov 25, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.