feat: add visual regression tests for key application pages - #767
feat: add visual regression tests for key application pages#767priscannanna85-lgtm wants to merge 1 commit into
Conversation
…ge form, creator dashboard
|
@priscannanna85-lgtm is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR adds Playwright visual tests for five page states, a snapshot update command, and GitHub Actions support for concurrency control, Chromium-only execution, screenshot artifacts, and pull-request failure comments. ChangesVisual regression coverage
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant GitHubActions
participant Playwright
participant Application
participant GitHubScript
PullRequest->>GitHubActions: Start visual regression workflow
GitHubActions->>Playwright: Run Chromium visual tests
Playwright->>Application: Navigate and capture page states
Application-->>Playwright: Return rendered pages
Playwright-->>GitHubActions: Return test results and screenshots
GitHubActions->>GitHubScript: Generate failure summary
GitHubScript->>PullRequest: Post visual-diff comment
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/playwright-visual-regression.yml:
- Around line 37-53: Update the “Generate visual diff report” step to initialize
diff_found as false before scanning test-results/**/*.png, then set it to true
inside the existing file check when at least one PNG diff artifact is found.
Keep the report generation behavior unchanged while ensuring downstream PR
comment logic receives false for non-screenshot failures.
- Around line 55-67: Add a job-level permissions block for the workflow
containing only repository contents read access and issues write access, so the
existing actions/github-script step can create PR comments without inheriting
broader token permissions. Keep the current visual-diff and comment behavior
unchanged.
- Around line 8-10: Update the workflow concurrency group to use
github.event.pull_request.number for pull request runs, with github.ref as the
fallback for non-pull-request events, while preserving cancel-in-progress
behavior.
In `@e2e/pages.visual.spec.ts`:
- Around line 22-26: Require the named visual states in e2e/pages.visual.spec.ts
at lines 22-26, 37-45, 57-60, and 71-75 instead of allowing isVisible() branches
to continue when targets are absent: require the campaign card, assert the
campaign detail panel after clicking, require the detail and pledge input and
assert the entered value, require the creator analytics section, and require the
theme toggle and assert the dark-theme state after clicking. Replace
timeout-only or optional checks with assertions that fail before screenshots
when the expected state is not rendered.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3cf2156d-1a3e-4ca4-974e-a652f3db3082
📒 Files selected for processing (3)
.github/workflows/playwright-visual-regression.ymle2e/pages.visual.spec.tspackage.json
| concurrency: | ||
| group: visual-regression-${{ github.head_ref }} | ||
| cancel-in-progress: true |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow files =="
git ls-files .github/workflows || true
echo
echo "== target workflow excerpt =="
if [ -f .github/workflows/playwright-visual-regression.yml ]; then
nl -ba .github/workflows/playwright-visual-regression.yml | sed -n '1,80p'
else
echo "target workflow not found"
fi
echo
echo "== all uses of head_ref in workflows =="
rg -n "head_ref|pull_request.number|ref_name|github.ref" .github/workflows || true
echo
echo "== diff stat/name status =="
git diff --stat -- .github/workflows/playwright-visual-regression.yml || true
git diff -- .github/workflows/playwright-visual-regression.yml | sed -n '1,120p' || trueRepository: ritik4ever/stellar-goal-vault
Length of output: 861
🌐 Web query:
GitHub Actions context github.head_ref pull_request.number fork same branch concurrent runs concurrency group
💡 Result:
To effectively manage concurrency and cancel redundant in-progress workflow runs in GitHub Actions, you should define a concurrency group at the workflow level [1]. Using a group key that is too specific (like using only github.head_ref or including unique identifiers like github.run_id) can lead to unintended behavior, such as collisions across different pull requests or failure to cancel redundant runs [2][3][4]. Recommended Strategy: The most robust concurrency group key combines the workflow name and the reference identifier [5][1][4]. This ensures that each workflow instance is scoped to its specific branch or pull request and prevents different workflows (e.g., CI and Lint) from cancelling each other [2][6][4]. A commonly recommended configuration is: concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true Why this configuration works: 1. github.workflow: Ensures that the concurrency group is unique to the specific workflow file, preventing separate workflows from interfering with one another [2][6][5]. 2. github.ref: On pull requests, this provides a unique reference (e.g., refs/pull/123/merge) that distinguishes one PR from another, even if they share the same source branch name (common when using forks) [3][4]. This avoids the risk of different users' PRs cancelling each other's runs [6][3]. 3. cancel-in-progress: Setting this to true causes the currently running workflow in the same group to be cancelled immediately when a new commit is pushed, ensuring that resources are focused on the latest commit [1][4]. Alternative Considerations: - If you use github.head_ref, be aware that it is only defined for pull_request events and can lead to unintended cancellations across different PRs that share the same branch name (e.g., both named 'patch-1' in different forks) [7][3]. If you must use github.head_ref, you should provide a fallback, such as ${{ github.head_ref || github.ref }} [7][4]. - For critical workflows (like deployments or releases), you may prefer to set cancel-in-progress to false or use conditional logic to avoid cancelling runs on the main branch, ensuring every deployment completes fully [1][4]. Top Results: [7], [3], [1], [4]
Citations:
- 1: https://starsling.dev/best-practices/github-actions/cancel-superseded-runs
- 2: GitHub Actions: Examples for canceling in-progress jobs / runs with concurrency groups lead to confusing behavior github/docs#11243
- 3: ci: harden GHA workflows with least-privilege permissions, concurrency, and action updates jmeridth/jmeridth.github.io#72
- 4: https://dev.to/ace2932/cancel-duplicate-github-actions-runs-with-one-block-and-the-one-place-you-shouldnt-2m3k
- 5: https://stackoverflow.com/questions/66335225/how-to-cancel-previous-runs-in-the-pr-when-you-push-new-commitsupdate-the-curre
- 6: https://github.com/mmlb/NixOS-nixpkgs/blob/master/.github/workflows/README.md
- 7: https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/control-workflow-concurrency
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target workflow excerpt =="
if [ -f .github/workflows/playwright-visual-regression.yml ]; then
awk '{printf "%5d %s\n", NR, $0}' .github/workflows/playwright-visual-regression.yml | sed -n '1,120p'
else
echo "target workflow not found"
fi
echo
echo "== triggers in target workflow =="
python3 - <<'PY'
from pathlib import Path
p=Path(".github/workflows/playwright-visual-regression.yml")
for i,line in enumerate(p.read_text().splitlines(),1):
if any(s in line for s in ["pull_request", "github.head_ref", "concurrency:", "workflow_dispatch", "push:", "schedule:", "issues:", "pull_request_target"]):
print(f"{i}: {line}")
PY
echo
echo "== all head_ref/use in workflows =='
rg -n -C 2 "github\.head_ref|pull_request\.number|concurrency:" .github/workflows || trueRepository: ritik4ever/stellar-goal-vault
Length of output: 3233
Use a unique pull request identifier for the concurrency group.
github.head_ref only contains the source branch name. Concurrent pull requests with the same branch name, including forked pull requests, share this group and can cancel each other.
Use github.event.pull_request.number for pull requests, with github.ref as a fallback for non-pull-request events.
🤖 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/playwright-visual-regression.yml around lines 8 - 10,
Update the workflow concurrency group to use github.event.pull_request.number
for pull request runs, with github.ref as the fallback for non-pull-request
events, while preserving cancel-in-progress behavior.
| - name: Generate visual diff report | ||
| if: failure() | ||
| id: visual-diff | ||
| run: | | ||
| echo 'diff_found=true' >> "$GITHUB_OUTPUT" | ||
| echo '### Visual Regression Results' > /tmp/visual-report.md | ||
| echo '' >> /tmp/visual-report.md | ||
| echo '| Test | Status |' >> /tmp/visual-report.md | ||
| echo '|------|--------|' >> /tmp/visual-report.md | ||
| for f in test-results/**/*.png; do | ||
| if [ -f "$f" ]; then | ||
| name=$(basename "$f" .png) | ||
| echo "| $name | :x: Diff detected |" >> /tmp/visual-report.md | ||
| fi | ||
| done | ||
| echo '' >> /tmp/visual-report.md | ||
| echo 'Review the uploaded artifacts for full screenshot diffs.' >> /tmp/visual-report.md |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the workflow and related Playwright config/artifact artifacts handling.
if [ -f .github/workflows/playwright-visual-regression.yml ]; then
echo "== workflow excerpt =="
cat -n .github/workflows/playwright-visual-regression.yml | sed -n '1,140p'
else
echo "workflow file not found"
fi
echo "== search for visual-diff and playwrites artifact patterns =="
rg -n "visual diff|visual-diff|diff_found|test-results|upload-artifact|playwright" .github/workflows playwright*.config.* 2>/dev/null || trueRepository: ritik4ever/stellar-goal-vault
Length of output: 5892
Set diff_found only when a screenshot diff artifact exists.
The workflow sets diff_found=true before scanning test-results/**/*.png, while the PR comment step uses only that output. Set diff_found=false by default and set it to true only after finding generated diff images to avoid posting a misleading summary for non-screenshot test failures.
🤖 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/playwright-visual-regression.yml around lines 37 - 53,
Update the “Generate visual diff report” step to initialize diff_found as false
before scanning test-results/**/*.png, then set it to true inside the existing
file check when at least one PNG diff artifact is found. Keep the report
generation behavior unchanged while ensuring downstream PR comment logic
receives false for non-screenshot failures.
| - name: Comment PR with visual diff summary | ||
| if: failure() && steps.visual-diff.outputs.diff_found == 'true' | ||
| uses: actions/github-script@v7 | ||
| with: | ||
| script: | | ||
| const fs = require('fs'); | ||
| const body = fs.readFileSync('/tmp/visual-report.md', 'utf8'); | ||
| github.rest.issues.createComment({ | ||
| issue_number: context.issue.number, | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| body: body | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow files =="
git ls-files .github/workflows || true
file=".github/workflows/playwright-visual-regression.yml"
if [ -f "$file" ]; then
echo "== permissions / comments around lines 1-120 =="
sed -n '1,120p' "$file" | cat -n
echo
echo "== relevant references =="
rg -n "permissions:|GITHUB_TOKEN|contents:|issues:|actions/github-script|createComment|pull-requests" "$file"
fiRepository: ritik4ever/stellar-goal-vault
Length of output: 3806
Declare least-privilege workflow permissions.
This job installs dependencies, runs test code, calls actions/github-script, and creates PR comments with GITHUB_TOKEN. Add a permissions block that grants only repository read access and issue-comment write access to avoid relying on repository-wide defaults.
Proposed fix
+permissions:
+ contents: read
+ issues: write
+
concurrency:🤖 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/playwright-visual-regression.yml around lines 55 - 67, Add
a job-level permissions block for the workflow containing only repository
contents read access and issues write access, so the existing
actions/github-script step can create PR comments without inheriting broader
token permissions. Keep the current visual-diff and comment behavior unchanged.
Source: Linters/SAST tools
| const firstCard = page.locator('[class*="campaign"]').first(); | ||
| if (await firstCard.isVisible()) { | ||
| await firstCard.click(); | ||
| await page.waitForTimeout(500); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require the named visual state before taking each screenshot.
Each isVisible() branch permits the test to continue when the required target is absent. The screenshot can then capture the homepage and still update or pass its baseline. waitForTimeout() does not prove that the target state rendered.
e2e/pages.visual.spec.ts#L22-L26: Require a campaign card and assert that the selected campaign detail panel is visible after the click.e2e/pages.visual.spec.ts#L37-L45: Require the campaign detail and pledge input, then assert the entered pledge value.e2e/pages.visual.spec.ts#L57-L60: Require the creator analytics section before the screenshot.e2e/pages.visual.spec.ts#L71-L75: Require the theme toggle and assert the dark-theme state after the click.
📍 Affects 1 file
e2e/pages.visual.spec.ts#L22-L26(this comment)e2e/pages.visual.spec.ts#L37-L45e2e/pages.visual.spec.ts#L57-L60e2e/pages.visual.spec.ts#L71-L75
🤖 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 `@e2e/pages.visual.spec.ts` around lines 22 - 26, Require the named visual
states in e2e/pages.visual.spec.ts at lines 22-26, 37-45, 57-60, and 71-75
instead of allowing isVisible() branches to continue when targets are absent:
require the campaign card, assert the campaign detail panel after clicking,
require the detail and pledge input and assert the entered value, require the
creator analytics section, and require the theme toggle and assert the
dark-theme state after clicking. Replace timeout-only or optional checks with
assertions that fail before screenshots when the expected state is not rendered.
1 participant
Category: testing
Project area: e2e/, .github/workflows/, package.json
Implementation:
test:visual:updateto refresh approved screenshot baselines.Acceptance Criteria:
npm run test:visual.npm run test:visual:update.Summary by CodeRabbit