From 7c36d9a596559685ab43e903939623477010b027 Mon Sep 17 00:00:00 2001
From: xiaomin qiu <amandaxmqiu@gmail.com>
Date: Sat, 2 Aug 2025 21:45:13 +0200
Subject: [PATCH 1/4] optimize test

---
 .github/workflows/releasetest_sep.yaml | 32 +++++++++++++++++---------
 tests/demo-todo-app.spec.ts            | 12 ++++++++--
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/.github/workflows/releasetest_sep.yaml b/.github/workflows/releasetest_sep.yaml
index d902be41..73c8eb8e 100644
--- a/.github/workflows/releasetest_sep.yaml
+++ b/.github/workflows/releasetest_sep.yaml
@@ -137,16 +137,6 @@ jobs:
           name: playwright-metrics
           path: dl
 
-      - name: Prepare metrics file
-        run: |
-          if [ -f dl/playwright-metrics-pr.json ]; then
-            cp dl/playwright-metrics-pr.json dl/playwright-metrics.json
-            echo "✅ Copied playwright-metrics-pr.json to playwright-metrics.json"
-          else
-            echo "⚠️  Warning: playwright-metrics-pr.json not found"
-            ls -la dl/
-          fi
-
       - name: Download PR HTML report
         uses: actions/download-artifact@v4
         with:
@@ -165,6 +155,26 @@ jobs:
           name: lint-summary
           path: dl
 
+      # CRITICAL: Ensure PR metrics are used for visualization
+      - name: Prepare artifact files
+        run: |
+          echo "📁 Downloaded artifacts:"
+          ls -la dl/
+          
+          # Ensure the PR metrics file is properly named
+          if [ -f dl/playwright-metrics-pr.json ]; then
+            cp dl/playwright-metrics-pr.json dl/playwright-metrics.json
+            echo "✅ Using PR metrics for visualization"
+          else
+            echo "❌ PR metrics file not found!"
+          fi
+          
+          # Ensure PR summary is properly named
+          if [ -f dl/playwright-summary-pr.json ] && [ ! -f dl/playwright-summary.json ]; then
+            cp dl/playwright-summary-pr.json dl/playwright-summary.json
+            echo "✅ Using PR summary"
+          fi
+
       # Build dashboard, post comment, deploy Pages
       - id: review
         name: Dashboard / PR comment / Pages
@@ -174,4 +184,4 @@ jobs:
           mode:               dashboard-only
           custom-artifacts-path: dl
           enable-github-pages: 'true'
-          enable-visual-comparison: 'true'
+          enable-visual-comparison: 'true'
\ No newline at end of file
diff --git a/tests/demo-todo-app.spec.ts b/tests/demo-todo-app.spec.ts
index 4724ce95..36bc9b16 100644
--- a/tests/demo-todo-app.spec.ts
+++ b/tests/demo-todo-app.spec.ts
@@ -20,9 +20,17 @@ test.describe('New Todo', () => {
     await newTodo.fill(TODO_ITEMS[0]);
     await newTodo.press('Enter');
 
-    // Make sure the list only has one todo item.
+    // Add visual difference
+    await page.evaluate(() => {
+      document.body.style.borderTop = '12px solid hotpink';
+    });
+
+    // Take screenshot before failure
+    await page.screenshot({ path: 'test-results/before-failure.png' });
+
+    // DELIBERATE FAILURE: This will fail and should show in the metrics
     await expect(page.getByTestId('todo-title')).toHaveText([
-      TODO_ITEMS[0]
+      'WRONG EXPECTED TEXT - This will fail and generate screenshots!'
     ]);
 
     // Create 2nd todo.

From bc6348e2ed6aa1370b9c510cc3efbedbe4bf7e1e Mon Sep 17 00:00:00 2001
From: xiaomin qiu <amandaxmqiu@gmail.com>
Date: Sat, 2 Aug 2025 22:10:21 +0200
Subject: [PATCH 2/4] optimize test

---
 scripts/visual-regression.js | 369 ++++++++++++++++++++++-------------
 1 file changed, 238 insertions(+), 131 deletions(-)

diff --git a/scripts/visual-regression.js b/scripts/visual-regression.js
index 78ad9676..16a30c84 100644
--- a/scripts/visual-regression.js
+++ b/scripts/visual-regression.js
@@ -2,93 +2,164 @@
 /**
  * visual-regression.js
  * Compares screenshots from PR and main branches
- * Works with actual Playwright screenshot artifacts
+ * FIXED: Improved screenshot matching logic
  */
 
-const fs           = require('fs');
-const path         = require('path');
+const fs = require('fs');
+const path = require('path');
 const { execSync } = require('child_process');
 
 const ART = 'artifacts';
 
 /* ────────────────────────────────────────────────────────── *
- *  Collect screenshots inside a Playwright HTML report
+ *  Read Playwright report metadata for better matching
  * ────────────────────────────────────────────────────────── */
-// ★ PATCH: read report.json to map screenshots to the exact testId + title
-function readReportIndex(reportPath) {
-  const index = {};                                             // { <sha>.png : "testId#attachmentName" }
-  const indexFile = path.join(reportPath, 'report.json');
-  try {
-    const json = JSON.parse(fs.readFileSync(indexFile, 'utf8'));
-    json.tests?.forEach(t => {
-      const id = t.testId || `${t.file}::${t.title}`;
-      (t.attachments || []).forEach(a => {
-        if (a.contentType?.startsWith('image/') && a.path) {
-          index[path.basename(a.path)] = `${id}#${a.name || a.title || 'screenshot'}`;
-        }
-      });
-    });
-  } catch (_) { /* silently ignore */ }
-  return index;
+function extractScreenshotMetadata(reportPath) {
+  const metadata = {};
+  
+  // Try to read the report.json file that Playwright generates
+  const reportJsonPath = path.join(reportPath, 'report.json');
+  if (fs.existsSync(reportJsonPath)) {
+    try {
+      const report = JSON.parse(fs.readFileSync(reportJsonPath, 'utf8'));
+      
+      // Map screenshots to test names
+      if (report.suites) {
+        report.suites.forEach(suite => {
+          suite.tests?.forEach(test => {
+            test.results?.forEach(result => {
+              result.attachments?.forEach(attachment => {
+                if (attachment.name === 'screenshot' && attachment.path) {
+                  const filename = path.basename(attachment.path);
+                  metadata[filename] = {
+                    testId: test.testId || test.title,
+                    title: test.title,
+                    status: result.status,
+                    retry: result.retry || 0
+                  };
+                }
+              });
+            });
+          });
+        });
+      }
+    } catch (e) {
+      console.warn('Could not parse report.json:', e.message);
+    }
+  }
+  
+  return metadata;
 }
 
+/* ────────────────────────────────────────────────────────── *
+ *  Find and categorize screenshots
+ * ────────────────────────────────────────────────────────── */
 function findScreenshots(reportPath) {
   const screenshots = [];
-  const shotIndex   = readReportIndex(reportPath);
-
+  const metadata = extractScreenshotMetadata(reportPath);
+  
   if (!fs.existsSync(reportPath)) return screenshots;
+  
+  // Look in all possible subdirectories
+  const searchDirs = ['', 'data', 'trace', 'test-results'];
+  
+  searchDirs.forEach(subDir => {
+    const dir = path.join(reportPath, subDir);
+    if (!fs.existsSync(dir)) return;
+    
+    fs.readdirSync(dir).forEach(file => {
+      if (file.match(/\.(png|jpe?g)$/i)) {
+        const meta = metadata[file] || {};
+        
+        // Extract test information from filename
+        let testInfo = extractTestInfo(file);
+        
+        screenshots.push({
+          filename: file,
+          path: path.join(dir, file),
+          testName: meta.title || testInfo.testName,
+          testId: meta.testId || testInfo.testId,
+          isFailure: file.includes('-actual') || meta.status === 'failed',
+          isExpected: file.includes('-expected'),
+          isDiff: file.includes('-diff'),
+          retry: meta.retry || testInfo.retry
+        });
+      }
+    });
+  });
+  
+  return screenshots;
+}
 
-  const dirs = ['data', 'trace', ''];                        // report sub-dirs
-  for (const sub of dirs) {
-    const dir = path.join(reportPath, sub);
-    if (!fs.existsSync(dir)) continue;
-
-    for (const f of fs.readdirSync(dir)) {
-      if (!f.match(/\.(png|jpe?g)$/i)) continue;
-      screenshots.push({
-        filename: f,
-        path    : path.join(dir, f),
-        testName: shotIndex[f] || extractTestName(f),
-        isTrace : sub === 'trace'
-      });
-    }
+/* ────────────────────────────────────────────────────────── *
+ *  Smart test info extraction from filename
+ * ────────────────────────────────────────────────────────── */
+function extractTestInfo(filename) {
+  // Remove common prefixes and suffixes
+  let cleaned = filename
+    .replace(/^[a-f0-9]{40}-?/, '') // Remove SHA
+    .replace(/-(actual|expected|diff|previous)/, '') // Remove comparison suffixes
+    .replace(/-(chromium|firefox|webkit|darwin|linux|win32)/, '') // Remove browser/platform
+    .replace(/-attempt\d+/, '') // Remove attempt number
+    .replace(/\.(png|jpe?g)$/i, ''); // Remove extension
+  
+  // Extract retry number if present
+  const retryMatch = cleaned.match(/-retry(\d+)/);
+  const retry = retryMatch ? parseInt(retryMatch[1]) : 0;
+  cleaned = cleaned.replace(/-retry\d+/, '');
+  
+  // Try to extract test hierarchy
+  const parts = cleaned.split('-');
+  let testName = cleaned;
+  let testId = cleaned;
+  
+  // Common patterns: suite-name-test-name
+  if (parts.length >= 3) {
+    testName = parts.slice(-2).join(' ').replace(/_/g, ' ');
+    testId = cleaned;
+  } else {
+    testName = cleaned.replace(/[-_]/g, ' ');
+    testId = cleaned;
   }
-  return screenshots;
+  
+  return { testName, testId, retry };
 }
 
 /* ────────────────────────────────────────────────────────── *
- *  CHANGE #1 – smarter test-name extraction
+ *  Create a unique key for matching screenshots
  * ────────────────────────────────────────────────────────── */
-function extractTestName(filename) {
-  return filename
-    // strip 40-char SHA-1 prefix (+ optional dash)
-    .replace(/^[a-f0-9]{40}-?/, '')
-    // strip PW screenshot suffixes
-    .replace(/-(actual|expected|diff|chromium|firefox|webkit|darwin|linux|win32)\.(png|jpe?g)$/i, '')
-    // drop any remaining extension
-    .replace(/\.(png|jpe?g)$/i, '')
-    // normalise
-    .replace(/-/g, ' ')
-    .replace(/^\d+/, '')
-    .trim() || filename;
+function createMatchKey(screenshot) {
+  // For failure screenshots, we want to match actual screenshots only
+  if (screenshot.isExpected || screenshot.isDiff) {
+    return null;
+  }
+  
+  // Create a normalized key based on test information
+  const testKey = (screenshot.testId || screenshot.testName || '')
+    .toLowerCase()
+    .replace(/[^a-z0-9]+/g, '-')
+    .replace(/^-|-$/g, '');
+  
+  // Include retry number to match correct attempts
+  return `${testKey}-retry${screenshot.retry || 0}`;
 }
 
 /* ────────────────────────────────────────────────────────── *
- *  Image comparison helper (unchanged)
+ *  Image comparison helper
  * ────────────────────────────────────────────────────────── */
 async function compareImages(img1Path, img2Path, diffPath) {
   try {
     if (!fs.existsSync(img1Path) || !fs.existsSync(img2Path)) return null;
 
-    /* quick binary equality */
-    if (fs.statSync(img1Path).size === fs.statSync(img2Path).size &&
-        fs.readFileSync(img1Path).equals(fs.readFileSync(img2Path))) {
+    // Quick binary check
+    const buf1 = fs.readFileSync(img1Path);
+    const buf2 = fs.readFileSync(img2Path);
+    
+    if (buf1.length === buf2.length && buf1.equals(buf2)) {
       return { hasDiff: false, diffPercent: 0, diffImage: null, identical: true };
     }
 
-    /* try ImageMagick */
-    try { execSync('which compare', { stdio: 'ignore' }); } catch { /* not present */ }
-
+    // Use ImageMagick for detailed comparison
     fs.mkdirSync(path.dirname(diffPath), { recursive: true });
 
     try {
@@ -98,21 +169,33 @@ async function compareImages(img1Path, img2Path, diffPath) {
       ).trim();
 
       const pixels = parseInt(diffOutput) || 0;
-      const [w, h] = execSync(`identify -format "%w %h" "${img1Path}"`, { encoding: 'utf8' })
-                     .trim().split(' ').map(Number);
-      const percent = w * h ? (pixels / (w * h)) * 100 : 0;
+      
+      // Get image dimensions
+      const dimensions = execSync(
+        `identify -format "%w %h" "${img1Path}"`,
+        { encoding: 'utf8' }
+      ).trim().split(' ').map(Number);
+      
+      const totalPixels = dimensions[0] * dimensions[1];
+      const percent = totalPixels > 0 ? (pixels / totalPixels) * 100 : 0;
 
       return {
-        hasDiff     : pixels > 0,
-        diffPercent : Math.round(percent * 100) / 100,
-        diffImage   : diffPath,
-        pixelDiff   : pixels,
-        totalPixels : w * h,
-        dimensions  : { width: w, height: h }
+        hasDiff: pixels > 0,
+        diffPercent: Math.round(percent * 100) / 100,
+        diffImage: diffPath,
+        pixelDiff: pixels,
+        totalPixels: totalPixels,
+        dimensions: { width: dimensions[0], height: dimensions[1] }
+      };
+    } catch (compareErr) {
+      // ImageMagick returns non-zero exit code when images differ
+      const pixels = parseInt(compareErr.stdout || compareErr.stderr || '1') || 1;
+      return { 
+        hasDiff: true, 
+        diffPercent: 50, // Assume significant difference
+        diffImage: diffPath, 
+        pixelDiff: pixels 
       };
-    } catch (cmpErr) {
-      const pixels = parseInt(cmpErr.stdout || cmpErr.stderr || '') || 1;
-      return { hasDiff: true, diffPercent: 50, diffImage: diffPath, pixelDiff: pixels };
     }
   } catch (err) {
     console.error('Error comparing images:', err.message);
@@ -121,109 +204,134 @@ async function compareImages(img1Path, img2Path, diffPath) {
 }
 
 /* ────────────────────────────────────────────────────────── *
- *  Generate full visual-regression report
+ *  Main visual regression analysis
  * ────────────────────────────────────────────────────────── */
 async function generateVisualReport() {
-  const prShots   = findScreenshots(path.join(ART, 'pr-report'));
+  console.log('🔍 Starting visual regression analysis...');
+  
+  const prShots = findScreenshots(path.join(ART, 'pr-report'));
   const mainShots = findScreenshots(path.join(ART, 'main-report'));
 
-  console.log(`Found ${prShots.length} PR screenshots`);
-  console.log(`Found ${mainShots.length} main screenshots`);
+  console.log(`📸 Found ${prShots.length} PR screenshots`);
+  console.log(`📸 Found ${mainShots.length} main screenshots`);
 
-  const diffDir = path.join(ART, 'visual-diffs');
-  fs.mkdirSync(diffDir, { recursive: true });
+  // Filter out helper images (expected, diff)
+  const prActual = prShots.filter(s => !s.isExpected && !s.isDiff);
+  const mainActual = mainShots.filter(s => !s.isExpected && !s.isDiff);
 
-  const stripHash = name => name.replace(/^[a-f0-9]{40}-?/, '');
+  console.log(`📸 Comparing ${prActual.length} PR vs ${mainActual.length} main screenshots`);
 
-  const comparisons = [];
+  // Create match maps
+  const prMap = new Map();
+  const mainMap = new Map();
 
-  /* compare PR vs main */
-  for (const pr of prShots) {
-    if (/-diff\.|-expected\./.test(pr.filename)) continue;    // skip helper files
+  prActual.forEach(shot => {
+    const key = createMatchKey(shot);
+    if (key) {
+      prMap.set(key, shot);
+      console.log(`PR: ${key} -> ${shot.filename}`);
+    }
+  });
+
+  mainActual.forEach(shot => {
+    const key = createMatchKey(shot);
+    if (key) {
+      mainMap.set(key, shot);
+      console.log(`Main: ${key} -> ${shot.filename}`);
+    }
+  });
 
-    /* CHANGE #2 – match on testName OR hash-stripped filename */
-    const main = mainShots.find(m =>
-      m.testName === pr.testName || stripHash(m.filename) === stripHash(pr.filename)
-    );
+  const diffDir = path.join(ART, 'visual-diffs');
+  fs.mkdirSync(diffDir, { recursive: true });
 
-    if (main) {
-      const diffPath   = path.join(diffDir, `diff-${pr.filename}`);
-      const cmp        = await compareImages(main.path, pr.path, diffPath) || {};
-      const pct        = cmp.diffPercent || 0;
+  const comparisons = [];
 
-      comparisons.push({
-        testName : pr.testName,
-        filename : pr.filename,
-        prImage  : pr.path,
-        mainImage: main.path,
-        ...cmp,
-        status   : pct === 0 ? 'identical'
-                  : pct < 0.1 ? 'negligible'
-                  : pct < 1   ? 'minor'
-                  : 'major'
-      });
+  // Compare matched screenshots
+  for (const [key, prShot] of prMap) {
+    const mainShot = mainMap.get(key);
+    
+    if (mainShot) {
+      console.log(`🔄 Comparing ${key}`);
+      const diffPath = path.join(diffDir, `diff-${key}.png`);
+      const result = await compareImages(mainShot.path, prShot.path, diffPath);
+      
+      if (result) {
+        const diffPercent = result.diffPercent || 0;
+        comparisons.push({
+          testName: prShot.testName || key,
+          filename: prShot.filename,
+          matchKey: key,
+          prImage: prShot.path,
+          mainImage: mainShot.path,
+          ...result,
+          status: diffPercent === 0 ? 'identical' :
+                  diffPercent < 0.1 ? 'negligible' :
+                  diffPercent < 1 ? 'minor' : 'major'
+        });
+      }
     } else {
+      // New screenshot in PR
       comparisons.push({
-        testName : pr.testName,
-        filename : pr.filename,
-        prImage  : pr.path,
+        testName: prShot.testName || key,
+        filename: prShot.filename,
+        matchKey: key,
+        prImage: prShot.path,
         mainImage: null,
-        hasDiff  : true,
+        hasDiff: true,
         diffPercent: 100,
-        status   : 'new'
+        status: 'new'
       });
     }
   }
 
-  /* detect removed screenshots (unchanged) */
-  for (const main of mainShots) {
-    if (/-diff\.|-expected\./.test(main.filename)) continue;
-    if (!prShots.find(p => stripHash(p.filename) === stripHash(main.filename))) {
+  // Find removed screenshots
+  for (const [key, mainShot] of mainMap) {
+    if (!prMap.has(key)) {
       comparisons.push({
-        testName : main.testName,
-        filename : main.filename,
-        prImage  : null,
-        mainImage: main.path,
-        hasDiff  : true,
+        testName: mainShot.testName || key,
+        filename: mainShot.filename,
+        matchKey: key,
+        prImage: null,
+        mainImage: mainShot.path,
+        hasDiff: true,
         diffPercent: 100,
-        status   : 'removed'
+        status: 'removed'
       });
     }
   }
 
-  /* summarise (unchanged) */
+  // Sort by difference percentage
   comparisons.sort((a, b) => b.diffPercent - a.diffPercent);
 
+  // Generate summary
   const summary = {
-    timestamp        : new Date().toISOString(),
-    totalScreenshots : prShots.length,
-    totalComparisons : comparisons.length,
-    identical        : comparisons.filter(c => c.status === 'identical').length,
-    negligible       : comparisons.filter(c => c.status === 'negligible').length,
-    minor            : comparisons.filter(c => c.status === 'minor').length,
-    major            : comparisons.filter(c => c.status === 'major').length,
-    new              : comparisons.filter(c => c.status === 'new').length,
-    removed          : comparisons.filter(c => c.status === 'removed').length,
-    comparisons
+    timestamp: new Date().toISOString(),
+    totalScreenshots: prActual.length,
+    totalComparisons: comparisons.length,
+    identical: comparisons.filter(c => c.status === 'identical').length,
+    negligible: comparisons.filter(c => c.status === 'negligible').length,
+    minor: comparisons.filter(c => c.status === 'minor').length,
+    major: comparisons.filter(c => c.status === 'major').length,
+    new: comparisons.filter(c => c.status === 'new').length,
+    removed: comparisons.filter(c => c.status === 'removed').length,
+    comparisons: comparisons
   };
 
+  // Save results
   fs.writeFileSync(
     path.join(ART, 'visual-regression-report.json'),
     JSON.stringify(summary, null, 2)
   );
 
-  /* the HTML / MD generation code is unchanged … */
+  // Generate HTML report
   fs.writeFileSync(
     path.join(ART, 'visual-regression.html'),
     generateHTMLReport(summary)
   );
-  fs.writeFileSync(
-    path.join(ART, 'visual-regression-summary.md'),
-    generateMarkdownSummary(summary)
-  );
 
   console.log('✅ Visual regression report generated');
   console.log(`📊 Summary: ${summary.identical} identical, ${summary.minor} minor, ${summary.major} major changes`);
+  
   return summary;
 }
 
@@ -610,7 +718,6 @@ function generateMarkdownSummary(report) {
   
   return md;
 }
-
 // Run the visual regression analysis
 if (require.main === module) {
   generateVisualReport().catch(console.error);

From 2cecd989225e88dec0cffff5e9b8929777ad12aa Mon Sep 17 00:00:00 2001
From: xiaomin qiu <amandaxmqiu@gmail.com>
Date: Sun, 3 Aug 2025 01:24:43 +0200
Subject: [PATCH 3/4] optimize test

---
 .github/workflows/releasetest_sep.yaml |   6 +-
 scripts/visual-regression.js           | 365 +++++++++++++------------
 2 files changed, 196 insertions(+), 175 deletions(-)

diff --git a/.github/workflows/releasetest_sep.yaml b/.github/workflows/releasetest_sep.yaml
index 73c8eb8e..935cc1d1 100644
--- a/.github/workflows/releasetest_sep.yaml
+++ b/.github/workflows/releasetest_sep.yaml
@@ -22,7 +22,7 @@ jobs:
 
       # Run Playwright with PR-vs-Main comparison
       - name: GUI Test – Playwright only
-        uses: DigitalProductInnovationAndDevelopment/Code-Reviews-of-GUI-Tests@v1.2.2
+        uses: DigitalProductInnovationAndDevelopment/Code-Reviews-of-GUI-Tests@v1.3.0
         with:
           github-token:            ${{ secrets.GITHUB_TOKEN }}
           mode:                    test-only
@@ -79,7 +79,7 @@ jobs:
       - run: npm install
 
       - name: GUI Test – Lint only
-        uses: DigitalProductInnovationAndDevelopment/Code-Reviews-of-GUI-Tests@v1.2.2
+        uses: DigitalProductInnovationAndDevelopment/Code-Reviews-of-GUI-Tests@v1.3.0
         with:
           github-token: ${{ secrets.GITHUB_TOKEN }}
           mode:        lint-only
@@ -178,7 +178,7 @@ jobs:
       # Build dashboard, post comment, deploy Pages
       - id: review
         name: Dashboard / PR comment / Pages
-        uses: DigitalProductInnovationAndDevelopment/Code-Reviews-of-GUI-Tests@v1.2.2
+        uses: DigitalProductInnovationAndDevelopment/Code-Reviews-of-GUI-Tests@v1.3.0
         with:
           github-token:       ${{ secrets.GITHUB_TOKEN }}
           mode:               dashboard-only
diff --git a/scripts/visual-regression.js b/scripts/visual-regression.js
index 16a30c84..67d36c47 100644
--- a/scripts/visual-regression.js
+++ b/scripts/visual-regression.js
@@ -2,7 +2,7 @@
 /**
  * visual-regression.js
  * Compares screenshots from PR and main branches
- * FIXED: Improved screenshot matching logic
+ * FIXED: Better screenshot discovery without report.json
  */
 
 const fs = require('fs');
@@ -12,189 +12,198 @@ const { execSync } = require('child_process');
 const ART = 'artifacts';
 
 /* ────────────────────────────────────────────────────────── *
- *  Read Playwright report metadata for better matching
+ *  Find all screenshots in Playwright HTML report
  * ────────────────────────────────────────────────────────── */
-function extractScreenshotMetadata(reportPath) {
-  const metadata = {};
+function findScreenshots(reportPath) {
+  const screenshots = [];
   
-  // Try to read the report.json file that Playwright generates
-  const reportJsonPath = path.join(reportPath, 'report.json');
-  if (fs.existsSync(reportJsonPath)) {
+  if (!fs.existsSync(reportPath)) {
+    console.log(`❌ Report path not found: ${reportPath}`);
+    return screenshots;
+  }
+
+  // Search in all possible locations where Playwright stores screenshots
+  const searchPaths = [
+    reportPath,
+    path.join(reportPath, 'data'),
+    path.join(reportPath, 'trace'),
+    path.join(reportPath, 'test-results')
+  ];
+
+  // Also check for index.html to find screenshot references
+  const indexPath = path.join(reportPath, 'index.html');
+  let screenshotRefs = [];
+  
+  if (fs.existsSync(indexPath)) {
     try {
-      const report = JSON.parse(fs.readFileSync(reportJsonPath, 'utf8'));
-      
-      // Map screenshots to test names
-      if (report.suites) {
-        report.suites.forEach(suite => {
-          suite.tests?.forEach(test => {
-            test.results?.forEach(result => {
-              result.attachments?.forEach(attachment => {
-                if (attachment.name === 'screenshot' && attachment.path) {
-                  const filename = path.basename(attachment.path);
-                  metadata[filename] = {
-                    testId: test.testId || test.title,
-                    title: test.title,
-                    status: result.status,
-                    retry: result.retry || 0
-                  };
-                }
-              });
-            });
-          });
-        });
-      }
+      const html = fs.readFileSync(indexPath, 'utf8');
+      // Extract screenshot references from HTML
+      const matches = html.match(/data\/[a-f0-9\-]+\.(png|jpe?g)/gi) || [];
+      screenshotRefs = matches.map(m => path.basename(m));
+      console.log(`Found ${screenshotRefs.length} screenshot references in index.html`);
     } catch (e) {
-      console.warn('Could not parse report.json:', e.message);
+      console.warn('Could not parse index.html:', e.message);
     }
   }
-  
-  return metadata;
-}
 
-/* ────────────────────────────────────────────────────────── *
- *  Find and categorize screenshots
- * ────────────────────────────────────────────────────────── */
-function findScreenshots(reportPath) {
-  const screenshots = [];
-  const metadata = extractScreenshotMetadata(reportPath);
-  
-  if (!fs.existsSync(reportPath)) return screenshots;
-  
-  // Look in all possible subdirectories
-  const searchDirs = ['', 'data', 'trace', 'test-results'];
-  
-  searchDirs.forEach(subDir => {
-    const dir = path.join(reportPath, subDir);
-    if (!fs.existsSync(dir)) return;
+  searchPaths.forEach(searchPath => {
+    if (!fs.existsSync(searchPath)) return;
     
-    fs.readdirSync(dir).forEach(file => {
-      if (file.match(/\.(png|jpe?g)$/i)) {
-        const meta = metadata[file] || {};
-        
-        // Extract test information from filename
-        let testInfo = extractTestInfo(file);
-        
-        screenshots.push({
-          filename: file,
-          path: path.join(dir, file),
-          testName: meta.title || testInfo.testName,
-          testId: meta.testId || testInfo.testId,
-          isFailure: file.includes('-actual') || meta.status === 'failed',
-          isExpected: file.includes('-expected'),
-          isDiff: file.includes('-diff'),
-          retry: meta.retry || testInfo.retry
-        });
-      }
-    });
+    const stats = fs.statSync(searchPath);
+    if (!stats.isDirectory()) return;
+    
+    try {
+      const files = fs.readdirSync(searchPath);
+      
+      files.forEach(file => {
+        // Match screenshot files
+        if (file.match(/\.(png|jpe?g)$/i)) {
+          const filePath = path.join(searchPath, file);
+          
+          // Skip diff and expected files for now
+          const isActual = !file.includes('-diff') && !file.includes('-expected');
+          const isDiff = file.includes('-diff');
+          const isExpected = file.includes('-expected');
+          
+          // Extract test information from filename
+          const testInfo = extractTestInfoFromFilename(file);
+          
+          screenshots.push({
+            filename: file,
+            path: filePath,
+            testName: testInfo.testName,
+            testId: testInfo.testId,
+            isActual,
+            isDiff,
+            isExpected,
+            browser: testInfo.browser,
+            isFailure: file.includes('-actual') || searchPath.includes('test-results')
+          });
+        }
+      });
+    } catch (e) {
+      console.warn(`Error reading directory ${searchPath}:`, e.message);
+    }
   });
+
+  console.log(`📸 Found ${screenshots.length} total files in ${reportPath}`);
+  console.log(`   - Actual: ${screenshots.filter(s => s.isActual).length}`);
+  console.log(`   - Expected: ${screenshots.filter(s => s.isExpected).length}`);
+  console.log(`   - Diff: ${screenshots.filter(s => s.isDiff).length}`);
   
   return screenshots;
 }
 
 /* ────────────────────────────────────────────────────────── *
- *  Smart test info extraction from filename
+ *  Extract test information from screenshot filename
  * ────────────────────────────────────────────────────────── */
-function extractTestInfo(filename) {
-  // Remove common prefixes and suffixes
-  let cleaned = filename
-    .replace(/^[a-f0-9]{40}-?/, '') // Remove SHA
-    .replace(/-(actual|expected|diff|previous)/, '') // Remove comparison suffixes
-    .replace(/-(chromium|firefox|webkit|darwin|linux|win32)/, '') // Remove browser/platform
-    .replace(/-attempt\d+/, '') // Remove attempt number
-    .replace(/\.(png|jpe?g)$/i, ''); // Remove extension
+function extractTestInfoFromFilename(filename) {
+  // Remove extension
+  let name = filename.replace(/\.(png|jpe?g)$/i, '');
   
-  // Extract retry number if present
-  const retryMatch = cleaned.match(/-retry(\d+)/);
-  const retry = retryMatch ? parseInt(retryMatch[1]) : 0;
-  cleaned = cleaned.replace(/-retry\d+/, '');
+  // Extract browser if present
+  let browser = 'chromium';
+  const browserMatch = name.match(/-(chromium|firefox|webkit)/);
+  if (browserMatch) {
+    browser = browserMatch[1];
+    name = name.replace(/-(chromium|firefox|webkit)/, '');
+  }
   
-  // Try to extract test hierarchy
-  const parts = cleaned.split('-');
-  let testName = cleaned;
-  let testId = cleaned;
+  // Remove SHA prefix if present (40 hex chars)
+  name = name.replace(/^[a-f0-9]{40}-?/, '');
   
-  // Common patterns: suite-name-test-name
-  if (parts.length >= 3) {
-    testName = parts.slice(-2).join(' ').replace(/_/g, ' ');
-    testId = cleaned;
-  } else {
-    testName = cleaned.replace(/[-_]/g, ' ');
-    testId = cleaned;
-  }
+  // Remove suffixes
+  name = name.replace(/-(actual|expected|diff|previous)$/, '');
+  name = name.replace(/-(darwin|linux|win32)$/, '');
+  name = name.replace(/-attempt\d+$/, '');
+  
+  // Create a normalized test name
+  const testName = name
+    .replace(/[_-]+/g, ' ')
+    .replace(/\s+/g, ' ')
+    .trim();
   
-  return { testName, testId, retry };
+  // Create a normalized test ID for matching
+  const testId = testName
+    .toLowerCase()
+    .replace(/[^a-z0-9]+/g, '-')
+    .replace(/^-|-$/g, '');
+  
+  return { testName, testId, browser };
 }
 
 /* ────────────────────────────────────────────────────────── *
- *  Create a unique key for matching screenshots
+ *  Create normalized key for matching screenshots
  * ────────────────────────────────────────────────────────── */
 function createMatchKey(screenshot) {
-  // For failure screenshots, we want to match actual screenshots only
-  if (screenshot.isExpected || screenshot.isDiff) {
-    return null;
-  }
+  // Only match actual screenshots (not diff or expected)
+  if (!screenshot.isActual) return null;
   
-  // Create a normalized key based on test information
-  const testKey = (screenshot.testId || screenshot.testName || '')
-    .toLowerCase()
-    .replace(/[^a-z0-9]+/g, '-')
-    .replace(/^-|-$/g, '');
-  
-  // Include retry number to match correct attempts
-  return `${testKey}-retry${screenshot.retry || 0}`;
+  // Create a key that should match between PR and main
+  return `${screenshot.testId}-${screenshot.browser}`;
 }
 
 /* ────────────────────────────────────────────────────────── *
- *  Image comparison helper
+ *  Image comparison
  * ────────────────────────────────────────────────────────── */
 async function compareImages(img1Path, img2Path, diffPath) {
   try {
-    if (!fs.existsSync(img1Path) || !fs.existsSync(img2Path)) return null;
+    if (!fs.existsSync(img1Path) || !fs.existsSync(img2Path)) {
+      console.log(`❌ Missing image for comparison: ${!fs.existsSync(img1Path) ? img1Path : img2Path}`);
+      return null;
+    }
 
-    // Quick binary check
+    // Quick binary comparison
     const buf1 = fs.readFileSync(img1Path);
     const buf2 = fs.readFileSync(img2Path);
     
     if (buf1.length === buf2.length && buf1.equals(buf2)) {
-      return { hasDiff: false, diffPercent: 0, diffImage: null, identical: true };
+      return { hasDiff: false, diffPercent: 0, identical: true };
+    }
+
+    // Try ImageMagick
+    try {
+      execSync('which compare', { stdio: 'ignore' });
+    } catch {
+      console.warn('⚠️  ImageMagick not found, using binary comparison only');
+      return { hasDiff: true, diffPercent: 50, method: 'binary' };
     }
 
-    // Use ImageMagick for detailed comparison
     fs.mkdirSync(path.dirname(diffPath), { recursive: true });
 
     try {
-      const diffOutput = execSync(
+      // Use ImageMagick compare
+      const result = execSync(
         `compare -metric AE -fuzz 5% "${img1Path}" "${img2Path}" "${diffPath}" 2>&1`,
-        { encoding: 'utf8' }
-      ).trim();
+        { encoding: 'utf8', stdio: 'pipe' }
+      );
 
-      const pixels = parseInt(diffOutput) || 0;
-      
-      // Get image dimensions
-      const dimensions = execSync(
-        `identify -format "%w %h" "${img1Path}"`,
-        { encoding: 'utf8' }
-      ).trim().split(' ').map(Number);
+      const pixels = parseInt(result) || 0;
       
-      const totalPixels = dimensions[0] * dimensions[1];
-      const percent = totalPixels > 0 ? (pixels / totalPixels) * 100 : 0;
+      // Get dimensions
+      const identify = execSync(`identify -format "%w %h" "${img1Path}"`, { encoding: 'utf8' });
+      const [width, height] = identify.trim().split(' ').map(Number);
+      const totalPixels = width * height;
+      const diffPercent = totalPixels > 0 ? (pixels / totalPixels) * 100 : 0;
 
       return {
         hasDiff: pixels > 0,
-        diffPercent: Math.round(percent * 100) / 100,
+        diffPercent: Math.round(diffPercent * 100) / 100,
         diffImage: diffPath,
         pixelDiff: pixels,
-        totalPixels: totalPixels,
-        dimensions: { width: dimensions[0], height: dimensions[1] }
+        totalPixels,
+        dimensions: { width, height },
+        method: 'imagemagick'
       };
-    } catch (compareErr) {
-      // ImageMagick returns non-zero exit code when images differ
-      const pixels = parseInt(compareErr.stdout || compareErr.stderr || '1') || 1;
-      return { 
-        hasDiff: true, 
+    } catch (err) {
+      // ImageMagick returns non-zero when images differ
+      const pixels = parseInt(err.stdout || err.stderr || '1') || 1;
+      return {
+        hasDiff: true,
         diffPercent: 50, // Assume significant difference
-        diffImage: diffPath, 
-        pixelDiff: pixels 
+        diffImage: diffPath,
+        pixelDiff: pixels,
+        method: 'imagemagick-error'
       };
     }
   } catch (err) {
@@ -209,27 +218,30 @@ async function compareImages(img1Path, img2Path, diffPath) {
 async function generateVisualReport() {
   console.log('🔍 Starting visual regression analysis...');
   
+  // Find all screenshots
   const prShots = findScreenshots(path.join(ART, 'pr-report'));
   const mainShots = findScreenshots(path.join(ART, 'main-report'));
 
-  console.log(`📸 Found ${prShots.length} PR screenshots`);
-  console.log(`📸 Found ${mainShots.length} main screenshots`);
+  // Filter to only actual screenshots (not diff/expected)
+  const prActual = prShots.filter(s => s.isActual);
+  const mainActual = mainShots.filter(s => s.isActual);
 
-  // Filter out helper images (expected, diff)
-  const prActual = prShots.filter(s => !s.isExpected && !s.isDiff);
-  const mainActual = mainShots.filter(s => !s.isExpected && !s.isDiff);
+  console.log(`\n📊 Actual screenshots to compare:`);
+  console.log(`   PR: ${prActual.length}`);
+  console.log(`   Main: ${mainActual.length}`);
 
-  console.log(`📸 Comparing ${prActual.length} PR vs ${mainActual.length} main screenshots`);
-
-  // Create match maps
+  // Create maps for matching
   const prMap = new Map();
   const mainMap = new Map();
-
+  
+  // Debug: Show what we're mapping
+  console.log('\n🔑 Creating match keys:');
+  
   prActual.forEach(shot => {
     const key = createMatchKey(shot);
     if (key) {
       prMap.set(key, shot);
-      console.log(`PR: ${key} -> ${shot.filename}`);
+      console.log(`   PR: "${key}" <- ${shot.filename}`);
     }
   });
 
@@ -237,30 +249,34 @@ async function generateVisualReport() {
     const key = createMatchKey(shot);
     if (key) {
       mainMap.set(key, shot);
-      console.log(`Main: ${key} -> ${shot.filename}`);
+      console.log(`   Main: "${key}" <- ${shot.filename}`);
     }
   });
 
+  // Prepare diff directory
   const diffDir = path.join(ART, 'visual-diffs');
   fs.mkdirSync(diffDir, { recursive: true });
 
   const comparisons = [];
+  const processedKeys = new Set();
 
-  // Compare matched screenshots
+  // Compare PR vs Main
+  console.log('\n🔄 Comparing screenshots:');
   for (const [key, prShot] of prMap) {
+    processedKeys.add(key);
     const mainShot = mainMap.get(key);
     
     if (mainShot) {
-      console.log(`🔄 Comparing ${key}`);
+      console.log(`   Comparing: ${key}`);
       const diffPath = path.join(diffDir, `diff-${key}.png`);
       const result = await compareImages(mainShot.path, prShot.path, diffPath);
       
       if (result) {
         const diffPercent = result.diffPercent || 0;
         comparisons.push({
-          testName: prShot.testName || key,
+          testName: prShot.testName,
+          testId: key,
           filename: prShot.filename,
-          matchKey: key,
           prImage: prShot.path,
           mainImage: mainShot.path,
           ...result,
@@ -268,13 +284,14 @@ async function generateVisualReport() {
                   diffPercent < 0.1 ? 'negligible' :
                   diffPercent < 1 ? 'minor' : 'major'
         });
+        console.log(`     -> ${result.diffPercent}% difference (${comparisons[comparisons.length-1].status})`);
       }
     } else {
-      // New screenshot in PR
+      console.log(`   New in PR: ${key}`);
       comparisons.push({
-        testName: prShot.testName || key,
+        testName: prShot.testName,
+        testId: key,
         filename: prShot.filename,
-        matchKey: key,
         prImage: prShot.path,
         mainImage: null,
         hasDiff: true,
@@ -284,13 +301,14 @@ async function generateVisualReport() {
     }
   }
 
-  // Find removed screenshots
+  // Check for removed screenshots
   for (const [key, mainShot] of mainMap) {
-    if (!prMap.has(key)) {
+    if (!processedKeys.has(key)) {
+      console.log(`   Removed from PR: ${key}`);
       comparisons.push({
-        testName: mainShot.testName || key,
+        testName: mainShot.testName,
+        testId: key,
         filename: mainShot.filename,
-        matchKey: key,
         prImage: null,
         mainImage: mainShot.path,
         hasDiff: true,
@@ -303,7 +321,7 @@ async function generateVisualReport() {
   // Sort by difference percentage
   comparisons.sort((a, b) => b.diffPercent - a.diffPercent);
 
-  // Generate summary
+  // Create summary
   const summary = {
     timestamp: new Date().toISOString(),
     totalScreenshots: prActual.length,
@@ -314,9 +332,15 @@ async function generateVisualReport() {
     major: comparisons.filter(c => c.status === 'major').length,
     new: comparisons.filter(c => c.status === 'new').length,
     removed: comparisons.filter(c => c.status === 'removed').length,
-    comparisons: comparisons
+    comparisons
   };
 
+  console.log('\n📊 Final summary:');
+  console.log(`   Total comparisons: ${summary.totalComparisons}`);
+  console.log(`   Identical: ${summary.identical}`);
+  console.log(`   Major changes: ${summary.major}`);
+  console.log(`   Minor changes: ${summary.minor}`);
+
   // Save results
   fs.writeFileSync(
     path.join(ART, 'visual-regression-report.json'),
@@ -329,13 +353,18 @@ async function generateVisualReport() {
     generateHTMLReport(summary)
   );
 
-  console.log('✅ Visual regression report generated');
-  console.log(`📊 Summary: ${summary.identical} identical, ${summary.minor} minor, ${summary.major} major changes`);
+  // Generate markdown summary
+  fs.writeFileSync(
+    path.join(ART, 'visual-regression-summary.md'),
+    generateMarkdownSummary(summary)
+  );
+
+  console.log('\n✅ Visual regression report generated');
   
   return summary;
 }
 
-// Generate HTML report
+// Generate HTML report (keeping the same as original)
 function generateHTMLReport(report) {
   const getStatusColor = (status) => {
     switch (status) {
@@ -672,15 +701,6 @@ function generateHTMLReport(report) {
         }
       });
     }
-    
-    function openImageModal(src) {
-      // This function should be defined in the main dashboard
-      if (window.openImageModal) {
-        window.openImageModal(src);
-      } else {
-        window.open(src, '_blank');
-      }
-    }
   </script>
 </div>
   `;
@@ -699,9 +719,9 @@ function generateMarkdownSummary(report) {
   
   md += '## Summary\n\n';
   md += `- ✅ Identical: ${report.identical}\n`;
-  md += `- ✓ Negligible (<1%): ${report.negligible}\n`;
-  md += `- ⚠️ Minor (1-5%): ${report.minor}\n`;
-  md += `- ❌ Major (>5%): ${report.major}\n`;
+  md += `- ✓ Negligible (<0.1%): ${report.negligible}\n`;
+  md += `- ⚠️ Minor (0.1-1%): ${report.minor}\n`;
+  md += `- ❌ Major (>1%): ${report.major}\n`;
   md += `- 🆕 New: ${report.new}\n`;
   md += `- 🗑️ Removed: ${report.removed}\n\n`;
   
@@ -718,6 +738,7 @@ function generateMarkdownSummary(report) {
   
   return md;
 }
+
 // Run the visual regression analysis
 if (require.main === module) {
   generateVisualReport().catch(console.error);

From 00088c1e012dca7ba378030ff771e5a73f36eb35 Mon Sep 17 00:00:00 2001
From: xiaomin qiu <amandaxmqiu@gmail.com>
Date: Mon, 4 Aug 2025 15:03:58 +0200
Subject: [PATCH 4/4] optimize test

---
 playwright.config.js | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/playwright.config.js b/playwright.config.js
index a8a33dd9..dad6ef97 100644
--- a/playwright.config.js
+++ b/playwright.config.js
@@ -4,14 +4,25 @@ module.exports = defineConfig({
   testDir: './tests',
   use: {
     headless: true,
-    screenshot: 'on',
+    screenshot: {
+      mode: 'on',
+      fullPage: true
+    },
     trace: 'on-first-retry',
     video: 'off',
     ignoreHTTPSErrors: true,
   },
   reporter: [
     ['list'],
-    ['json', { outputFile: 'playwright-metrics.json' }],   // ← relative path
+    ['json', { outputFile: 'playwright-metrics.json' }],
     ['html', { outputFolder: 'playwright-report', open: 'never' }],
   ],
-});
+  use: {
+    testIdAttribute: 'data-testid',
+    contextOptions: {
+      recordVideo: {
+        dir: 'test-results'
+      }
+    }
+  }
+});
\ No newline at end of file