Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 40 additions & 4 deletions .github/workflows/playwright-visual-regression.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ on:
branches:
- main

concurrency:
group: visual-regression-${{ github.head_ref }}
cancel-in-progress: true
Comment on lines +8 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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' || true

Repository: 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:


🏁 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 || true

Repository: 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.


jobs:
visual-regression:
name: Visual regression
Expand All @@ -17,19 +21,51 @@ jobs:
- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version: 18
node-version: 22
cache: 'npm'
cache-dependency-path: package-lock.json

- name: Install dependencies
run: npm install
run: npm ci

- name: Install Playwright browsers
run: npx playwright install --with-deps
run: npx playwright install --with-deps chromium

- name: Run visual regression tests
run: npm run test:visual

- 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
Comment on lines +37 to +53

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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 || true

Repository: 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
});
Comment on lines +55 to +67

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 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"
fi

Repository: 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


- name: Upload visual regression artifacts on failure
if: failure()
uses: actions/upload-artifact@v4
Expand All @@ -38,4 +74,4 @@ jobs:
path: |
playwright-report
test-results
e2e/screenshots
e2e/screenshots
82 changes: 82 additions & 0 deletions e2e/pages.visual.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import { test, expect } from '@playwright/test';

test.describe('Page-level visual regression', () => {
test.beforeEach(async ({ page }) => {
await page.setViewportSize({ width: 1440, height: 900 });
});

test('homepage dashboard renders correctly', async ({ page }) => {
await page.goto('/');
await page.waitForSelector('.app-shell');
await page.waitForSelector('.hero');
await expect(page).toHaveScreenshot('homepage.png', {
fullPage: true,
maxDiffPixelRatio: 0.01,
animations: 'disabled',
});
});

test('campaign detail panel renders with selected campaign', async ({ page }) => {
await page.goto('/');
await page.waitForSelector('.app-shell');
const firstCard = page.locator('[class*="campaign"]').first();
if (await firstCard.isVisible()) {
await firstCard.click();
await page.waitForTimeout(500);
}
Comment on lines +22 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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-L45
  • e2e/pages.visual.spec.ts#L57-L60
  • e2e/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.

await expect(page).toHaveScreenshot('campaign-detail.png', {
fullPage: true,
maxDiffPixelRatio: 0.01,
animations: 'disabled',
});
});

test('pledge form renders within campaign detail', async ({ page }) => {
await page.goto('/');
await page.waitForSelector('.app-shell');
const firstCard = page.locator('[class*="campaign"]').first();
if (await firstCard.isVisible()) {
await firstCard.click();
await page.waitForTimeout(500);
}
const pledgeSection = page.locator('input[type="number"]').first();
if (await pledgeSection.isVisible()) {
await pledgeSection.fill('50');
}
await expect(page).toHaveScreenshot('pledge-form.png', {
fullPage: true,
maxDiffPixelRatio: 0.01,
animations: 'disabled',
});
});

test('creator analytics dashboard renders', async ({ page }) => {
await page.goto('/');
await page.waitForSelector('.app-shell');
await page.waitForTimeout(1000);
const analyticsSection = page.locator('section.animate-fade-in').first();
if (await analyticsSection.isVisible()) {
await analyticsSection.scrollIntoViewIfNeeded();
}
await expect(page).toHaveScreenshot('creator-analytics.png', {
fullPage: true,
maxDiffPixelRatio: 0.01,
animations: 'disabled',
});
});

test('dark mode homepage renders correctly', async ({ page }) => {
await page.goto('/');
await page.waitForSelector('.app-shell');
const toggleButton = page.getByRole('button', { name: /dark mode|light mode/i });
if (await toggleButton.isVisible()) {
await toggleButton.click();
await page.waitForTimeout(300);
}
await expect(page).toHaveScreenshot('homepage-dark.png', {
fullPage: true,
maxDiffPixelRatio: 0.01,
animations: 'disabled',
});
});
});
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
"format:check": "npx prettier --check \"**/*.{ts,tsx,json}\"",
"deploy:contract": "./scripts/deploy.sh",
"gen:bindings": "cd backend && npx ts-node-dev --respawn --transpile-only ../scripts/gen-bindings.ts",
"gen:bindings:mainnet": "NETWORK=mainnet npm run gen:bindings"
"gen:bindings:mainnet": "NETWORK=mainnet npm run gen:bindings",
"test:visual:update": "npx playwright test --config=playwright.visual.config.ts --update-snapshots"
},
"license": "MIT",
"dependencies": {
Expand All @@ -34,4 +35,4 @@
"playwright": "^1.59.1",
"prettier": "^3.0.0"
}
}
}