tests(e2e): Various fixes to visual testing (#6569)

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6569
Reviewed-by: 0ko <0ko@noreply.codeberg.org>

13 files changed, 34 insertions, 20 deletions

diff --git a/tests/e2e/actions.test.e2e.ts b/tests/e2e/actions.test.e2e.ts
index 6236fe70d3..4e93b89ee0 100644
--- a/tests/e2e/actions.test.e2e.ts
+++ b/tests/e2e/actions.test.e2e.ts
@@ -71,4 +71,5 @@ test('workflow dispatch box not available for unauthenticated users', async ({pa
   await page.goto('/user2/test_workflows/actions?workflow=test-dispatch.yml&actor=0&status=0');
 
   await expect(page.locator('body')).not.toContainText(workflow_trigger_notification_text);
+  await save_visual(page);
 });
diff --git a/tests/e2e/clipboard-copy.test.e2e.ts b/tests/e2e/clipboard-copy.test.e2e.ts
index 70a3425868..2ae0e0dfff 100644
--- a/tests/e2e/clipboard-copy.test.e2e.ts
+++ b/tests/e2e/clipboard-copy.test.e2e.ts
@@ -8,7 +8,7 @@
 // @watch end
 
 import {expect} from '@playwright/test';
-import {test} from './utils_e2e.ts';
+import {save_visual, test} from './utils_e2e.ts';
 
 test('copy src file path to clipboard', async ({page}, workerInfo) => {
   test.skip(['Mobile Safari', 'webkit'].includes(workerInfo.project.name), 'Apple clipboard API addon - starting at just $499!');
@@ -19,6 +19,7 @@ test('copy src file path to clipboard', async ({page}, workerInfo) => {
   await page.click('[data-clipboard-text]');
   const clipboardText = await page.evaluate(() => navigator.clipboard.readText());
   expect(clipboardText).toContain('README.md');
+  await save_visual(page);
 });
 
 test('copy diff file path to clipboard', async ({page}, workerInfo) => {
@@ -30,4 +31,6 @@ test('copy diff file path to clipboard', async ({page}, workerInfo) => {
   await page.click('[data-clipboard-text]');
   const clipboardText = await page.evaluate(() => navigator.clipboard.readText());
   expect(clipboardText).toContain('README.md');
+  await expect(page.getByText('Copied')).toBeVisible();
+  await save_visual(page);
 });
diff --git a/tests/e2e/dashboard-ci-status.test.e2e.ts b/tests/e2e/dashboard-ci-status.test.e2e.ts
index 800fc951e6..d35fe299ff 100644
--- a/tests/e2e/dashboard-ci-status.test.e2e.ts
+++ b/tests/e2e/dashboard-ci-status.test.e2e.ts
@@ -3,7 +3,7 @@
 // @watch end
 
 import {expect} from '@playwright/test';
-import {save_visual, test} from './utils_e2e.ts';
+import {test} from './utils_e2e.ts';
 
 test.use({user: 'user2'});
 
@@ -23,5 +23,6 @@ test('Correct link and tooltip', async ({page}, testInfo) => {
   const repoStatus = page.locator('.dashboard-repos .repo-owner-name-list > li:nth-child(1) > a:nth-child(2)');
   await expect(repoStatus).toHaveAttribute('href', '/user2/test_workflows/actions', {timeout: 10000});
   await expect(repoStatus).toHaveAttribute('data-tooltip-content', /^(Error|Failure)$/);
-  await save_visual(page);
+  // ToDo: Ensure stable screenshot of dashboard. Known to be flaky: https://code.forgejo.org/forgejo/visual-browser-testing/commit/206d4cfb7a4af6d8d7043026cdd4d63708798b2a
+  // await save_visual(page);
 });
diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go
index 88b254b6f9..8e4a9653cd 100644
--- a/tests/e2e/e2e_test.go
+++ b/tests/e2e/e2e_test.go
@@ -82,6 +82,7 @@ func TestE2e(t *testing.T) {
 
 	runArgs := []string{"npx", "playwright", "test"}
 
+	_, testVisual := os.LookupEnv("VISUAL_TEST")
 	// To update snapshot outputs
 	if _, set := os.LookupEnv("ACCEPT_VISUAL"); set {
 		runArgs = append(runArgs, "--update-snapshots")
@@ -105,6 +106,10 @@ func TestE2e(t *testing.T) {
 			onForgejoRun(t, func(*testing.T, *url.URL) {
 				defer DeclareGitRepos(t)()
 				thisTest := runArgs
+				// when all tests are run, use unique artifacts directories per test to preserve artifacts from other tests
+				if testVisual {
+					thisTest = append(thisTest, "--output=tests/e2e/test-artifacts/"+testname)
+				}
 				thisTest = append(thisTest, path)
 				cmd := exec.Command(runArgs[0], thisTest...)
 				cmd.Env = os.Environ()
@@ -114,7 +119,7 @@ func TestE2e(t *testing.T) {
 				cmd.Stderr = os.Stderr
 
 				err := cmd.Run()
-				if err != nil {
+				if err != nil && !testVisual {
 					log.Fatal("Playwright Failed: %s", err)
 				}
 			})
diff --git a/tests/e2e/example.test.e2e.ts b/tests/e2e/example.test.e2e.ts
index b2a679a82d..97c5b8684b 100644
--- a/tests/e2e/example.test.e2e.ts
+++ b/tests/e2e/example.test.e2e.ts
@@ -5,7 +5,7 @@
 // @watch end
 
 import {expect} from '@playwright/test';
-import {test} from './utils_e2e.ts';
+import {save_visual, test} from './utils_e2e.ts';
 
 test('Load Homepage', async ({page}) => {
   const response = await page.goto('/');
@@ -26,6 +26,7 @@ test('Register Form', async ({page}, workerInfo) => {
   expect(page.url()).toBe(`${workerInfo.project.use.baseURL}/`);
   await expect(page.locator('.secondary-nav span>img.ui.avatar')).toBeVisible();
   await expect(page.locator('.ui.positive.message.flash-success')).toHaveText('Account was successfully created. Welcome!');
+  await save_visual(page);
 });
 
 // eslint-disable-next-line playwright/no-skipped-test
diff --git a/tests/e2e/explore.test.e2e.ts b/tests/e2e/explore.test.e2e.ts
index 44c9b21f58..1bb5af3cc6 100644
--- a/tests/e2e/explore.test.e2e.ts
+++ b/tests/e2e/explore.test.e2e.ts
@@ -7,7 +7,7 @@
 // @watch end
 
 import {expect} from '@playwright/test';
-import {test} from './utils_e2e.ts';
+import {save_visual, test} from './utils_e2e.ts';
 
 test('Explore view taborder', async ({page}) => {
   await page.goto('/explore/repos');
@@ -42,4 +42,5 @@ test('Explore view taborder', async ({page}) => {
     }
   }
   expect(res).toBe(exp);
+  await save_visual(page);
 });
diff --git a/tests/e2e/markup.test.e2e.ts b/tests/e2e/markup.test.e2e.ts
index 2726942d57..398a0a6300 100644
--- a/tests/e2e/markup.test.e2e.ts
+++ b/tests/e2e/markup.test.e2e.ts
@@ -3,7 +3,7 @@
 // @watch end
 
 import {expect} from '@playwright/test';
-import {test} from './utils_e2e.ts';
+import {save_visual, test} from './utils_e2e.ts';
 
 test('markup with #xyz-mode-only', async ({page}) => {
   const response = await page.goto('/user2/repo1/issues/1');
@@ -13,4 +13,5 @@ test('markup with #xyz-mode-only', async ({page}) => {
   await expect(comment).toBeVisible();
   await expect(comment.locator('[src$="#gh-light-mode-only"]')).toBeVisible();
   await expect(comment.locator('[src$="#gh-dark-mode-only"]')).toBeHidden();
+  await save_visual(page);
 });
diff --git a/tests/e2e/repo-code.test.e2e.ts b/tests/e2e/repo-code.test.e2e.ts
index 335fb5b7f5..11b710c956 100644
--- a/tests/e2e/repo-code.test.e2e.ts
+++ b/tests/e2e/repo-code.test.e2e.ts
@@ -49,6 +49,7 @@ test('Line Range Selection', async ({page}) => {
   // out-of-bounds end line
   await page.goto(`${filePath}#L1-L100`);
   await assertSelectedLines(page, ['1', '2', '3']);
+  await save_visual(page);
 });
 
 test('Readable diff', async ({page}, workerInfo) => {
@@ -75,6 +76,7 @@ test('Readable diff', async ({page}, workerInfo) => {
       await expect(page.getByText(thisDiff.added, {exact: true})).toHaveCSS('background-color', 'rgb(134, 239, 172)');
     }
   }
+  await save_visual(page);
 });
 
 test.describe('As authenticated user', () => {
diff --git a/tests/e2e/repo-commitgraph.test.e2e.ts b/tests/e2e/repo-commitgraph.test.e2e.ts
index 5f0cad117a..39c5661900 100644
--- a/tests/e2e/repo-commitgraph.test.e2e.ts
+++ b/tests/e2e/repo-commitgraph.test.e2e.ts
@@ -5,7 +5,7 @@
 // @watch end
 
 import {expect} from '@playwright/test';
-import {test} from './utils_e2e.ts';
+import {save_visual, test} from './utils_e2e.ts';
 
 test('Commit graph overflow', async ({page}) => {
   await page.goto('/user2/diff-test/graph');
@@ -28,4 +28,5 @@ test('Switch branch', async ({page}) => {
   await expect(page.locator('#loading-indicator')).toBeHidden();
   await expect(page.locator('#rel-container')).toBeVisible();
   await expect(page.locator('#rev-container')).toBeVisible();
+  await save_visual(page);
 });
diff --git a/tests/e2e/repo-migrate.test.e2e.ts b/tests/e2e/repo-migrate.test.e2e.ts
index 428c2cb171..5e67f89ed1 100644
--- a/tests/e2e/repo-migrate.test.e2e.ts
+++ b/tests/e2e/repo-migrate.test.e2e.ts
@@ -21,7 +21,6 @@ test('Migration Progress Page', async ({page, browser}, workerInfo) => {
   await form.locator('button.primary').click({timeout: 5000});
   await expect(page).toHaveURL('user2/invalidrepo');
   await save_visual(page);
-  // page screenshot of unauthenticatedPage is checked automatically after the test
 
   const ctx = await test_context(browser);
   const unauthenticatedPage = await ctx.newPage();
@@ -37,4 +36,6 @@ test('Migration Progress Page', async ({page, browser}, workerInfo) => {
   await save_visual(page);
   await deleteModal.getByRole('button', {name: 'Delete repository'}).click();
   await expect(page).toHaveURL('/');
+  // checked last to preserve the order of screenshots from first run
+  await save_visual(unauthenticatedPage);
 });
diff --git a/tests/e2e/repo-wiki.test.e2e.ts b/tests/e2e/repo-wiki.test.e2e.ts
index f32fe3fc91..4ce66da8bc 100644
--- a/tests/e2e/repo-wiki.test.e2e.ts
+++ b/tests/e2e/repo-wiki.test.e2e.ts
@@ -4,7 +4,7 @@
 // @watch end
 
 import {expect} from '@playwright/test';
-import {test} from './utils_e2e.ts';
+import {save_visual, test} from './utils_e2e.ts';
 
 for (const searchTerm of ['space', 'consectetur']) {
   for (const width of [null, 2560, 4000]) {
@@ -23,6 +23,7 @@ for (const searchTerm of ['space', 'consectetur']) {
       await page.getByPlaceholder('Search wiki').dispatchEvent('keyup');
       // timeout is necessary because HTMX search could be slow
       await expect(page.locator('#wiki-search a[href]')).toBeInViewport({ratio: 1});
+      await save_visual(page);
     });
   }
 }
@@ -36,4 +37,5 @@ test(`Search results show titles (and not file names)`, async ({page}, workerInf
   // so we manually "type" the last letter
   await page.getByPlaceholder('Search wiki').dispatchEvent('keyup');
   await expect(page.locator('#wiki-search a[href] b')).toHaveText('Page With Spaced Name');
+  await save_visual(page);
 });
diff --git a/tests/e2e/right-settings-button.test.e2e.ts b/tests/e2e/right-settings-button.test.e2e.ts
index e1c40fdd4d..3bea329ba0 100644
--- a/tests/e2e/right-settings-button.test.e2e.ts
+++ b/tests/e2e/right-settings-button.test.e2e.ts
@@ -5,7 +5,7 @@
 // @watch end
 
 import {expect} from '@playwright/test';
-import {test} from './utils_e2e.ts';
+import {save_visual, test} from './utils_e2e.ts';
 
 test.describe('desktop viewport as user 2', () => {
   test.use({user: 'user2', viewport: {width: 1920, height: 300}});
@@ -54,6 +54,7 @@ test.describe('desktop viewport, unauthenticated', () => {
     await expect(page.locator('.overflow-menu-items>#settings-btn')).toHaveCount(0);
 
     await expect(page.locator('.overflow-menu-button')).toHaveCount(0);
+    await save_visual(page);
   });
 });
 
@@ -78,6 +79,7 @@ test.describe('small viewport', () => {
 
     const items = shownItems.concat(overflowItems);
     expect(Array.from(new Set(items))).toHaveLength(items.length);
+    await save_visual(page);
   });
 
   test('Settings button in overflow menu of org header', async ({page}) => {
@@ -121,5 +123,6 @@ test.describe('small viewport, unauthenticated', () => {
 
     const items = shownItems.concat(overflowItems);
     expect(Array.from(new Set(items))).toHaveLength(items.length);
+    await save_visual(page);
   });
 });
diff --git a/tests/e2e/utils_e2e.ts b/tests/e2e/utils_e2e.ts
index 80412e437d..ff921a2cf3 100644
--- a/tests/e2e/utils_e2e.ts
+++ b/tests/e2e/utils_e2e.ts
@@ -26,15 +26,6 @@ export const test = baseTest.extend<TestOptions>({
   },
   user: null,
   authScope: 'shared',
-  // see https://playwright.dev/docs/test-fixtures#adding-global-beforeeachaftereach-hooks
-  forEachTest: [async ({page}, use) => {
-    await use();
-    // some tests create a new page which is not yet available here
-    // only operate on tests that make the URL available
-    if (page.url() !== 'about:blank') {
-      await save_visual(page);
-    }
-  }, {auto: true}],
 });
 
 export async function test_context(browser: Browser, options?: BrowserContextOptions) {
@@ -128,6 +119,7 @@ export async function save_visual(page: Page) {
         // update order of recently created repos is not fully deterministic
         page.locator('.flex-item-main').filter({hasText: 'relative time in repo'}),
         page.locator('#activity-feed'),
+        page.locator('#user-heatmap'),
         // dynamic IDs in fixed-size inputs
         page.locator('input[value*="dyn-id-"]'),
       ],