tests(e2e): Prepare for visual regression testing

- fix some configuration issues
- mask or replace dynamic content from screenshots
- explain the behaviour in the e2e README

readme

playwright.config.ts

@@ -103,4 +103,5 @@ export default {
   outputDir: 'tests/e2e/test-artifacts/',
   /* Folder for explicit snapshots for visual testing */
   snapshotDir: 'tests/e2e/test-snapshots/',
+  snapshotPathTemplate: '{snapshotDir}/snapshots/{testFilePath}/{projectName}_{arg}{ext}',
 } satisfies PlaywrightTestConfig;

tests/e2e/README.md

@@ -155,20 +155,6 @@ For SQLite:
 make test-e2e-sqlite#example
 ```
 
-### Visual testing
-
-> **Warning**
-> This is not currently used by most Forgejo contributors.
-> Your help to improve the situation and allow for visual testing is appreciated.
-
-Although the main goal of e2e is assertion testing, we have added a framework for visual regression testing. If you are working on front-end features, please use the following:
- - Check out `main`, `make clean frontend`, and run e2e tests with `VISUAL_TEST=1` to generate outputs. This will initially fail, as no screenshots exist. You can run the e2e tests again to assert that it passes.
- - Check out your branch, `make clean frontend`, and run e2e tests with `VISUAL_TEST=1`. You should be able to assert that your front-end changes don't break any other tests unintentionally.
-
-`VISUAL_TEST=1` will create screenshots in tests/e2e/test-snapshots. The test will fail the first time this is enabled (until we get visual test image persistence figured out), because it will be testing against an empty screenshot folder.
-
-`ACCEPT_VISUAL=1` will overwrite the snapshot images with new images.
-
 
 ## Tips and tricks
 
@@ -216,6 +202,41 @@ you can alternatively use:
 await page.waitForURL('**/target.html');
 ~~~
 
+### Visual testing
+
+Due to size and frequent updates, we do not host screenshots in the Forgejo repository.
+However, it is good practice to ensure that your test is capable of generating relevant and stable screenshots.
+Forgejo is regularly tested against visual regressions in a dedicated repository which contains the screenshots:
+https://code.forgejo.org/forgejo/visual-browser-testing/
+
+For tests that consume only the `page`,
+screenshots are automatically created at the end of each test.
+
+If your test visits different relevant screens or pages during the test,
+or creates a custom `page` from context
+(e.g. for tests that require a signed-in user)
+calling `await save_visual(page);` explicitly in relevant positions is encouraged.
+
+Please confirm locally that your screenshots are stable by performing several runs of your test.
+When screenshots are available and reproducible,
+check in your test without the screenshots.
+
+When your screenshots differ between runs,
+for example because dynamic elements (e.g. timestamps, commit hashes etc)
+change between runs,
+mask these elements in the `save_visual` function in `utils_e2e.ts`.
+
+#### Working with screenshots
+
+The following environment variables control visual testing:
+
+`VISUAL_TEST=1` will create screenshots in tests/e2e/test-snapshots.
+  The test will fail the first time,
+  because the screenshots are not included with Forgejo.
+  Subsequent runs will comopare against your local copy of the screenshots.
+
+`ACCEPT_VISUAL=1` will overwrite the snapshot images with new images.
+
 ### Only sign in if necessary
 
 Signing in takes time and is actually executed step-by-step.

tests/e2e/example.test.e2e.ts

@@ -31,7 +31,7 @@ test('Register Form', async ({page}, workerInfo) => {
   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!');
 
-  save_visual(page);
+  await save_visual(page);
 });
 
 // eslint-disable-next-line playwright/no-skipped-test

tests/e2e/release.test.e2e.ts

@@ -41,7 +41,7 @@ test('External Release Attachments', async ({browser, isMobile}, workerInfo) =>
   await page.fill('input[name=attachment-new-name-2]', 'Test');
   await page.fill('input[name=attachment-new-exturl-2]', 'https://forgejo.org/');
   await page.click('.remove-rel-attach');
-  save_visual(page);
+  await save_visual(page);
   await page.click('.button.small.primary');
 
   // Validate release page and click edit
@@ -53,7 +53,7 @@ test('External Release Attachments', async ({browser, isMobile}, workerInfo) =>
   await expect(page.locator('.download[open] li:nth-of-type(2) a')).toHaveAttribute('href', '/user2/repo2/archive/2.0.tar.gz');
   await expect(page.locator('.download[open] li:nth-of-type(3)')).toContainText('Test');
   await expect(page.locator('.download[open] li:nth-of-type(3) a')).toHaveAttribute('href', 'https://forgejo.org/');
-  save_visual(page);
+  await save_visual(page);
   await page.locator('.octicon-pencil').first().click();
 
   // Validate edit page and edit the release
@@ -68,7 +68,7 @@ test('External Release Attachments', async ({browser, isMobile}, workerInfo) =>
   await expect(page.locator('.attachment_edit:visible')).toHaveCount(4);
   await page.locator('.attachment_edit:visible').nth(2).fill('Test3');
   await page.locator('.attachment_edit:visible').nth(3).fill('https://gitea.com/');
-  save_visual(page);
+  await save_visual(page);
   await page.click('.button.small.primary');
 
   // Validate release page and click edit
@@ -78,7 +78,7 @@ test('External Release Attachments', async ({browser, isMobile}, workerInfo) =>
   await expect(page.locator('.download[open] li:nth-of-type(3) a')).toHaveAttribute('href', 'https://gitea.io/');
   await expect(page.locator('.download[open] li:nth-of-type(4)')).toContainText('Test3');
   await expect(page.locator('.download[open] li:nth-of-type(4) a')).toHaveAttribute('href', 'https://gitea.com/');
-  save_visual(page);
+  await save_visual(page);
   await page.locator('.octicon-pencil').first().click();
 
   // Delete release

tests/e2e/utils_e2e.ts

@@ -4,6 +4,15 @@ export const test = baseTest.extend({
   context: async ({browser}, use) => {
     return use(await test_context(browser));
   },
+  // 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}],
 });
 
 async function test_context(browser: Browser, options?: BrowserContextOptions) {
@@ -66,14 +75,28 @@ export async function save_visual(page: Page) {
   // Optionally include visual testing
   if (process.env.VISUAL_TEST) {
     await page.waitForLoadState('domcontentloaded');
-    // Mock page/version string
+    // Mock/replace dynamic content which can have different size (and thus cannot simply be masked below)
-    await page.locator('footer div.ui.left').evaluate((node) => node.innerHTML = 'MOCK');
+    await page.locator('footer .left-links').evaluate((node) => node.innerHTML = 'MOCK');
+    // replace timestamps in repos to mask them later down
+    await page.locator('.flex-item-body > relative-time').filter({hasText: /now|minute/}).evaluateAll((nodes) => {
+      for (const node of nodes) node.outerHTML = 'relative time in repo';
+    });
+    await page.locator('relative-time').evaluateAll((nodes) => {
+      for (const node of nodes) node.outerHTML = 'time element';
+    });
+    // used for instance for security keys
+    await page.locator('absolute-date').evaluateAll((nodes) => {
+      for (const node of nodes) node.outerHTML = 'time element';
+    });
     await expect(page).toHaveScreenshot({
       fullPage: true,
       timeout: 20000,
       mask: [
-        page.locator('.secondary-nav span>img.ui.avatar'),
+        page.locator('.ui.avatar'),
-        page.locator('.ui.dropdown.jump.item span>img.ui.avatar'),
+        page.locator('.sha'),
+        page.locator('#repo_migrating'),
+        // update order of recently created repos is not fully deterministic
+        page.locator('.flex-item-main').filter({hasText: 'relative time in repo'}),
       ],
     });
   }