From 40551de313c33a10b6ea9d67bf506a7700c48950 Mon Sep 17 00:00:00 2001
From: Otto Richter <git@otto.splvs.net>
Date: Tue, 12 Nov 2024 21:07:09 +0100
Subject: [PATCH] tests(e2e): Refactor various tests

Goals:

- speedup
- less flakiness
- best practices and more use
- documentation

config:
- sync ports in Makefile and playwright config
  (otherwise, some tests fail locally because they assert the full URL including the (wrong) port)
- even more generous timeouts
- limit workers to one again (because I finally understand how
  Playwright works)
- allow nested functions to group them together with the related test

all:

- deprecate waitForLoadState('networkidle')
  - it is discouraged as per https://playwright.dev/docs/api/class-page#page-wait-for-load-state
  - I could not find a usage that seems to require it actually (see
    added documentation in README)
  - adding an exception should be made explicitly
  - it does not do what you might expect anyway in most cases
- only log in when necessary

webauthn:

- verify that login is possible after disabling key
- otherwise, the cleanup was not necessary after the previous refactor to create a fresh user each

issue-sidebar / WIP toggle:

- split into smaller chunks
- restore original state first
- add missed assertion to fix race condition (not waiting
  before state was reached)
- explicitly toggle the state to detect mismatch earlier

issue-sidebar / labels:

- restore original state first
- better waiting for background request
---
 Makefile                                  |   1 -
 eslint.config.mjs                         |   3 +-
 playwright.config.ts                      |  14 +-
 tests/e2e/README.md                       |  88 ++++++++++
 tests/e2e/actions.test.e2e.ts             |   5 -
 tests/e2e/dashboard-ci-status.test.e2e.ts |   5 +-
 tests/e2e/issue-comment.test.e2e.ts       |   2 +-
 tests/e2e/issue-sidebar.test.e2e.ts       | 186 +++++++++++++---------
 tests/e2e/markup.test.e2e.ts              |   1 -
 tests/e2e/profile_actions.test.e2e.ts     |   1 -
 tests/e2e/repo-code.test.e2e.ts           |  11 +-
 tests/e2e/repo-commitgraph.test.e2e.ts    |  12 +-
 tests/e2e/repo-wiki.test.e2e.ts           |   1 -
 tests/e2e/utils_e2e.ts                    |   4 +-
 tests/e2e/webauthn.test.e2e.ts            |  13 +-
 15 files changed, 230 insertions(+), 117 deletions(-)

diff --git a/Makefile b/Makefile
index ae7ed16846..670cb9452a 100644
--- a/Makefile
+++ b/Makefile
@@ -716,7 +716,6 @@ test-e2e-pgsql\#%: playwright e2e.pgsql.test generate-ini-pgsql
 
 .PHONY: test-e2e-debugserver
 test-e2e-debugserver: e2e.sqlite.test generate-ini-sqlite
-	sed -i s/3003/3000/g tests/sqlite.ini
 	GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/sqlite.ini ./e2e.sqlite.test -test.run TestDebugserver -test.timeout 24h
 
 .PHONY: bench-sqlite
diff --git a/eslint.config.mjs b/eslint.config.mjs
index c52fdd4ddd..73a4e0bcfa 100644
--- a/eslint.config.mjs
+++ b/eslint.config.mjs
@@ -1125,7 +1125,8 @@ export default tseslint.config(
       ...playwright.configs['flat/recommended'].rules,
       'playwright/no-conditional-in-test': [0],
       'playwright/no-conditional-expect': [0],
-      'playwright/no-networkidle': [0],
+      // allow grouping helper functions with tests
+      'unicorn/consistent-function-scoping': [0],
 
       'playwright/no-skipped-test': [
         2,
diff --git a/playwright.config.ts b/playwright.config.ts
index 625dc3bc69..0994c55045 100644
--- a/playwright.config.ts
+++ b/playwright.config.ts
@@ -1,6 +1,6 @@
 import {devices, type PlaywrightTestConfig} from '@playwright/test';
 
-const BASE_URL = process.env.GITEA_URL?.replace?.(/\/$/g, '') || 'http://localhost:3000';
+const BASE_URL = process.env.GITEA_URL?.replace?.(/\/$/g, '') || 'http://localhost:3003';
 
 /**
  * @see https://playwright.dev/docs/test-configuration
@@ -12,7 +12,7 @@ export default {
 
   // you can adjust this value locally to match your machine's power,
   // or pass `--workers x` to playwright
-  workers: process.env.CI ? 1 : 2,
+  workers: 1,
 
   /* Maximum time one test can run for. */
   timeout: 30 * 1000,
@@ -22,7 +22,7 @@ export default {
      * Maximum time expect() should wait for the condition to be met.
      * For example in `await expect(locator).toHaveText();`
      */
-    timeout: 2000,
+    timeout: 3000,
   },
 
   /* Fail the build on CI if you accidentally left test.only in the source code. */
@@ -30,6 +30,8 @@ export default {
 
   /* Retry on CI only */
   retries: process.env.CI ? 1 : 0,
+  /* fail fast */
+  maxFailures: process.env.CI ? 1 : 0,
 
   /* Reporter to use. See https://playwright.dev/docs/test-reporters */
   reporter: process.env.CI ? 'list' : [['list'], ['html', {outputFolder: 'tests/e2e/reports/', open: 'never'}]],
@@ -41,7 +43,7 @@ export default {
     locale: 'en-US',
 
     /* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */
-    actionTimeout: 2000,
+    actionTimeout: 3000,
 
     /* Maximum time allowed for navigation, such as `page.goto()`. */
     navigationTimeout: 10 * 1000,
@@ -95,8 +97,8 @@ export default {
     },
   ],
 
-  /* Folder for test artifacts such as screenshots, videos, traces, etc. */
+  /* Folder for test artifacts created during test execution such as screenshots, traces, etc. */
   outputDir: 'tests/e2e/test-artifacts/',
-  /* Folder for test artifacts such as screenshots, videos, traces, etc. */
+  /* Folder for explicit snapshots for visual testing */
   snapshotDir: 'tests/e2e/test-snapshots/',
 } satisfies PlaywrightTestConfig;
diff --git a/tests/e2e/README.md b/tests/e2e/README.md
index 8fae664564..81dc0bf832 100644
--- a/tests/e2e/README.md
+++ b/tests/e2e/README.md
@@ -175,6 +175,70 @@ ACCEPT_VISUAL=1 will overwrite the snapshot images with new images.
 If you know noteworthy tests that can act as an inspiration for new tests,
 please add some details here.
 
+### Understanding and waiting for page loads
+
+[Waiting for a load state](https://playwright.dev/docs/api/class-frame#frame-wait-for-load-state)
+sound like a convenient way to ensure the page was loaded,
+but it only works once and consecutive calls to it
+(e.g. after clicking a button which should reload a page)
+return immediately without waiting for *another* load event.
+
+If you match something which is on both the old and the new page,
+you might succeed before the page was reloaded,
+although the code using a `waitForLoadState` might intuitively suggest
+the page was changed before.
+
+Interacting with the page before the reload
+(e.g. by opening a dropdown)
+might then race and result in flaky tests,
+depending on the speed of the hardware running the test.
+
+A possible way to test that an interaction worked is by checking for a known change first.
+For example:
+
+- you submit a form and you want to check that the content persisted
+- checking for the content directly would succeed even without a page reload
+- check for a success message first (will wait until it appears), then verify the content
+
+Alternatively, if you know the backend request that will be made before the reload,
+you can explicitly wait for it:
+
+~~~js
+const submitted = page.waitForResponse('/my/backend/post/request');
+await page.locator('button').first().click(); // perform your interaction
+await submitted;
+~~~
+
+If the page redirects to another URL,
+you can alternatively use:
+
+~~~js
+await page.waitForURL('**/target.html');
+~~~
+
+### Only sign in if necessary
+
+Signing in takes time and is actually executed step-by-step.
+If your test does not rely on a user account, skip this step.
+
+~~~js
+test('For anyone', async ({page}) => {
+  await page.goto('/somepage');
+~~~
+
+If you need a user account, you can use something like:
+
+~~~js
+import {test, login_user, login} from './utils_e2e.ts';
+
+test.beforeAll(async ({browser}, workerInfo) => {
+  await login_user(browser, workerInfo, 'user2'); // or another user
+});
+
+test('For signed users only', async ({browser}, workerInfo) => {
+  const page = await login({browser}, workerInfo);
+~~~
+
 ### Run tests very selectively
 
 Browser testing can take some time.
@@ -264,3 +328,27 @@ and a set of files with a certain ending:
 
 The patterns are evaluated on a "first-match" basis.
 Under the hood, [gobwas/glob](https://github.com/gobwas/glob) is used.
+
+## Grouped retry for interactions
+
+Sometimes, it can be necessary to retry certain interactions together.
+Consider the following procedure:
+
+1. click to open a dropdown
+2. interact with content in the dropdown
+
+When for some reason the dropdown does not open,
+for example because of it taking time to initialize after page load,
+the click will succeed,
+but the depending interaction won't,
+although playwright repeatedly tries to find the content.
+
+You can [group statements using toPass]()https://playwright.dev/docs/test-assertions#expecttopass).
+This code retries the dropdown click until the second item is found.
+
+~~~js
+await expect(async () => {
+  await page.locator('.dropdown').click();
+  await page.locator('.dropdown .item').first().click();
+}).toPass();
+~~~
diff --git a/tests/e2e/actions.test.e2e.ts b/tests/e2e/actions.test.e2e.ts
index 5e3ae7e3ff..0aa1c747dc 100644
--- a/tests/e2e/actions.test.e2e.ts
+++ b/tests/e2e/actions.test.e2e.ts
@@ -44,7 +44,6 @@ test('workflow dispatch error: missing inputs', async ({browser}, workerInfo) =>
   const page = await context.newPage();
 
   await page.goto('/user2/test_workflows/actions?workflow=test-dispatch.yml&actor=0&status=0');
-  await page.waitForLoadState('networkidle');
 
   await page.locator('#workflow_dispatch_dropdown>button').click();
 
@@ -55,7 +54,6 @@ test('workflow dispatch error: missing inputs', async ({browser}, workerInfo) =>
   });
 
   await page.locator('#workflow-dispatch-submit').click();
-  await page.waitForLoadState('networkidle');
 
   await expect(page.getByText('Require value for input "String w/o. default".')).toBeVisible();
 });
@@ -68,13 +66,11 @@ test('workflow dispatch success', async ({browser}, workerInfo) => {
   const page = await context.newPage();
 
   await page.goto('/user2/test_workflows/actions?workflow=test-dispatch.yml&actor=0&status=0');
-  await page.waitForLoadState('networkidle');
 
   await page.locator('#workflow_dispatch_dropdown>button').click();
 
   await page.type('input[name="inputs[string2]"]', 'abc');
   await page.locator('#workflow-dispatch-submit').click();
-  await page.waitForLoadState('networkidle');
 
   await expect(page.getByText('Workflow run was successfully requested.')).toBeVisible();
 
@@ -83,7 +79,6 @@ test('workflow dispatch success', async ({browser}, workerInfo) => {
 
 test('workflow dispatch box not available for unauthenticated users', async ({page}) => {
   await page.goto('/user2/test_workflows/actions?workflow=test-dispatch.yml&actor=0&status=0');
-  await page.waitForLoadState('networkidle');
 
   await expect(page.locator('body')).not.toContainText(workflow_trigger_notification_text);
 });
diff --git a/tests/e2e/dashboard-ci-status.test.e2e.ts b/tests/e2e/dashboard-ci-status.test.e2e.ts
index a5bdc7ade9..531955209a 100644
--- a/tests/e2e/dashboard-ci-status.test.e2e.ts
+++ b/tests/e2e/dashboard-ci-status.test.e2e.ts
@@ -15,10 +15,9 @@ test('Correct link and tooltip', async ({browser}, workerInfo) => {
   const response = await page.goto('/?repo-search-query=test_workflows');
   expect(response?.status()).toBe(200);
 
-  await page.waitForLoadState('networkidle');
-
   const repoStatus = page.locator('.dashboard-repos .repo-owner-name-list > li:nth-child(1) > a:nth-child(2)');
-
+  // wait for network activity to cease (so status was loaded in frontend)
+  await page.waitForLoadState('networkidle'); // eslint-disable-line playwright/no-networkidle
   await expect(repoStatus).toHaveAttribute('href', '/user2/test_workflows/actions', {timeout: 10000});
   await expect(repoStatus).toHaveAttribute('data-tooltip-content', 'Failure');
 });
diff --git a/tests/e2e/issue-comment.test.e2e.ts b/tests/e2e/issue-comment.test.e2e.ts
index 7b8326b832..9a3a45f522 100644
--- a/tests/e2e/issue-comment.test.e2e.ts
+++ b/tests/e2e/issue-comment.test.e2e.ts
@@ -56,7 +56,7 @@ test('Always focus edit tab first on edit', async ({browser}, workerInfo) => {
   await page.locator('#issue-1 .comment-container a[data-tab-for=markdown-previewer]').click();
   await page.click('#issue-1 .comment-container .save');
 
-  await page.waitForLoadState('networkidle');
+  await page.waitForLoadState();
 
   // Edit again and assert that edit tab should be active (and not preview tab)
   await page.click('#issue-1 .comment-container .context-menu');
diff --git a/tests/e2e/issue-sidebar.test.e2e.ts b/tests/e2e/issue-sidebar.test.e2e.ts
index 31962bf3b4..422f3ef94e 100644
--- a/tests/e2e/issue-sidebar.test.e2e.ts
+++ b/tests/e2e/issue-sidebar.test.e2e.ts
@@ -11,92 +11,137 @@ test.beforeAll(async ({browser}, workerInfo) => {
   await login_user(browser, workerInfo, 'user2');
 });
 
-// belongs to test: Pull: Toggle WIP
-const prTitle = 'pull5';
-
-async function click_toggle_wip({page}) {
-  await page.locator('.toggle-wip>a').click();
-  await page.waitForLoadState('networkidle');
-}
-
-async function check_wip({page}, is) {
-  const elemTitle = '#issue-title-display';
-  const stateLabel = '.issue-state-label';
-  await expect(page.locator(elemTitle)).toContainText(prTitle);
-  await expect(page.locator(elemTitle)).toContainText('#5');
-  if (is) {
-    await expect(page.locator(elemTitle)).toContainText('WIP');
-    await expect(page.locator(stateLabel)).toContainText('Draft');
-  } else {
-    await expect(page.locator(elemTitle)).not.toContainText('WIP');
-    await expect(page.locator(stateLabel)).toContainText('Open');
+/* eslint-disable playwright/expect-expect */
+// some tests are reported to have no assertions,
+// which is not correct, because they use the global helper function
+test.describe('Pull: Toggle WIP', () => {
+  const prTitle = 'pull5';
+  async function toggle_wip_to({page}, should) {
+    await page.waitForLoadState('domcontentloaded');
+    if (should) {
+      await page.getByText('Still in progress?').click();
+    } else {
+      await page.getByText('Ready for review?').click();
+    }
   }
-}
 
-test('Pull: Toggle WIP', async ({browser}, workerInfo) => {
-  test.skip(workerInfo.project.name === 'Mobile Safari', 'Unable to get tests working on Safari Mobile, see https://codeberg.org/forgejo/forgejo/pulls/3445#issuecomment-1789636');
-  const page = await login({browser}, workerInfo);
-  const response = await page.goto('/user2/repo1/pulls/5');
-  expect(response?.status()).toBe(200); // Status OK
-  // initial state
-  await check_wip({page}, false);
-  // toggle to WIP
-  await click_toggle_wip({page});
-  await check_wip({page}, true);
-  // remove WIP
-  await click_toggle_wip({page});
-  await check_wip({page}, false);
+  async function check_wip({page}, is) {
+    const elemTitle = 'h1';
+    const stateLabel = '.issue-state-label';
+    await page.waitForLoadState('domcontentloaded');
+    await expect(page.locator(elemTitle)).toContainText(prTitle);
+    await expect(page.locator(elemTitle)).toContainText('#5');
+    if (is) {
+      await expect(page.locator(elemTitle)).toContainText('WIP');
+      await expect(page.locator(stateLabel)).toContainText('Draft');
+    } else {
+      await expect(page.locator(elemTitle)).not.toContainText('WIP');
+      await expect(page.locator(stateLabel)).toContainText('Open');
+    }
+  }
 
-  // manually edit title to another prefix
-  await page.locator('#issue-title-edit-show').click();
-  await page.locator('#issue-title-editor input').fill(`[WIP] ${prTitle}`);
-  await page.getByText('Save').click();
-  await page.waitForLoadState('networkidle');
-  await check_wip({page}, true);
-  // remove again
-  await click_toggle_wip({page});
-  await check_wip({page}, false);
-  // check maximum title length is handled gracefully
-  const maxLenStr = prTitle + 'a'.repeat(240);
-  await page.locator('#issue-title-edit-show').click();
-  await page.locator('#issue-title-editor input').fill(maxLenStr);
-  await page.getByText('Save').click();
-  await page.waitForLoadState('networkidle');
-  await click_toggle_wip({page});
-  await check_wip({page}, true);
-  await click_toggle_wip({page});
-  await check_wip({page}, false);
-  await expect(page.locator('h1')).toContainText(maxLenStr);
-  // restore original title
-  await page.locator('#issue-title-edit-show').click();
-  await page.locator('#issue-title-editor input').fill(prTitle);
-  await page.getByText('Save').click();
-  await check_wip({page}, false);
+  test.beforeEach(async ({browser}, workerInfo) => {
+    const page = await login({browser}, workerInfo);
+    const response = await page.goto('/user2/repo1/pulls/5');
+    expect(response?.status()).toBe(200); // Status OK
+    // ensure original title
+    await page.locator('#issue-title-edit-show').click();
+    await page.locator('#issue-title-editor input').fill(prTitle);
+    await page.getByText('Save').click();
+    await check_wip({page}, false);
+  });
+
+  test('simple toggle', async ({browser}, workerInfo) => {
+    test.skip(workerInfo.project.name === 'Mobile Safari', 'Unable to get tests working on Safari Mobile, see https://codeberg.org/forgejo/forgejo/pulls/3445#issuecomment-1789636');
+    const page = await login({browser}, workerInfo);
+    await page.goto('/user2/repo1/pulls/5');
+    // toggle to WIP
+    await toggle_wip_to({page}, true);
+    await check_wip({page}, true);
+    // remove WIP
+    await toggle_wip_to({page}, false);
+    await check_wip({page}, false);
+  });
+
+  test('manual edit', async ({browser}, workerInfo) => {
+    test.skip(workerInfo.project.name === 'Mobile Safari', 'Unable to get tests working on Safari Mobile, see https://codeberg.org/forgejo/forgejo/pulls/3445#issuecomment-1789636');
+    const page = await login({browser}, workerInfo);
+    await page.goto('/user2/repo1/pulls/5');
+    // manually edit title to another prefix
+    await page.locator('#issue-title-edit-show').click();
+    await page.locator('#issue-title-editor input').fill(`[WIP] ${prTitle}`);
+    await page.getByText('Save').click();
+    await check_wip({page}, true);
+    // remove again
+    await toggle_wip_to({page}, false);
+    await check_wip({page}, false);
+  });
+
+  test('maximum title length', async ({browser}, workerInfo) => {
+    test.skip(workerInfo.project.name === 'Mobile Safari', 'Unable to get tests working on Safari Mobile, see https://codeberg.org/forgejo/forgejo/pulls/3445#issuecomment-1789636');
+    const page = await login({browser}, workerInfo);
+    await page.goto('/user2/repo1/pulls/5');
+    // check maximum title length is handled gracefully
+    const maxLenStr = prTitle + 'a'.repeat(240);
+    await page.locator('#issue-title-edit-show').click();
+    await page.locator('#issue-title-editor input').fill(maxLenStr);
+    await page.getByText('Save').click();
+    await expect(page.locator('h1')).toContainText(maxLenStr);
+    await check_wip({page}, false);
+    await toggle_wip_to({page}, true);
+    await check_wip({page}, true);
+    await expect(page.locator('h1')).toContainText(maxLenStr);
+    await toggle_wip_to({page}, false);
+    await check_wip({page}, false);
+    await expect(page.locator('h1')).toContainText(maxLenStr);
+  });
 });
+/* eslint-enable playwright/expect-expect */
 
 test('Issue: Labels', async ({browser}, workerInfo) => {
   test.skip(workerInfo.project.name === 'Mobile Safari', 'Unable to get tests working on Safari Mobile, see https://codeberg.org/forgejo/forgejo/pulls/3445#issuecomment-1789636');
+
+  async function submitLabels({page}) {
+    const submitted = page.waitForResponse('/user2/repo1/issues/labels');
+    await page.locator('textarea').first().click(); // close via unrelated element
+    await submitted;
+    await page.waitForLoadState();
+  }
+
   const page = await login({browser}, workerInfo);
   // select label list in sidebar only
   const labelList = page.locator('.issue-content-right .labels-list a');
   const response = await page.goto('/user2/repo1/issues/1');
   expect(response?.status()).toBe(200);
-  // preconditions
-  await expect(labelList.filter({hasText: 'label1'})).toBeVisible();
+
+  // restore initial state
+  await page.locator('.select-label').click();
+  const responsePromise = page.waitForResponse('/user2/repo1/issues/labels');
+  await page.getByText('Clear labels').click();
+  await responsePromise;
+  await expect(labelList.filter({hasText: 'label1'})).toBeHidden();
   await expect(labelList.filter({hasText: 'label2'})).toBeHidden();
-  // add label2
+
+  // add both labels
   await page.locator('.select-label').click();
   // label search could be tested this way:
   // await page.locator('.select-label input').fill('label2');
   await page.locator('.select-label .item').filter({hasText: 'label2'}).click();
-  await page.locator('.select-label').click();
-  await page.waitForLoadState('networkidle');
+  await page.locator('.select-label .item').filter({hasText: 'label1'}).click();
+  await submitLabels({page});
   await expect(labelList.filter({hasText: 'label2'})).toBeVisible();
-  // test removing label again
-  await page.locator('.select-label').click();
-  await page.locator('.select-label .item').filter({hasText: 'label2'}).click();
-  await page.locator('.select-label').click();
-  await page.waitForLoadState('networkidle');
+  await expect(labelList.filter({hasText: 'label1'})).toBeVisible();
+
+  // test removing label2 again
+  // due to a race condition, the page could still be "reloading",
+  // closing the dropdown after it was clicked.
+  // Retry the interaction as a group
+  // also see https://playwright.dev/docs/test-assertions#expecttopass
+  await expect(async () => {
+    await page.locator('.select-label').click();
+    await page.locator('.select-label .item').filter({hasText: 'label2'}).click();
+  }).toPass();
+  await submitLabels({page});
   await expect(labelList.filter({hasText: 'label2'})).toBeHidden();
   await expect(labelList.filter({hasText: 'label1'})).toBeVisible();
 });
@@ -109,11 +154,6 @@ test('Issue: Assignees', async ({browser}, workerInfo) => {
 
   const response = await page.goto('/org3/repo3/issues/1');
   expect(response?.status()).toBe(200);
-  // preconditions
-  await expect(assigneesList.filter({hasText: 'user2'})).toBeVisible();
-  await expect(assigneesList.filter({hasText: 'user4'})).toBeHidden();
-  await expect(page.locator('.ui.assignees.list .item.no-select')).toBeHidden();
-
   // Clear all assignees
   await page.locator('.select-assignees-modify.dropdown').click();
   await page.locator('.select-assignees-modify.dropdown .no-select.item').click();
diff --git a/tests/e2e/markup.test.e2e.ts b/tests/e2e/markup.test.e2e.ts
index 2cf32669e4..2726942d57 100644
--- a/tests/e2e/markup.test.e2e.ts
+++ b/tests/e2e/markup.test.e2e.ts
@@ -8,7 +8,6 @@ import {test} from './utils_e2e.ts';
 test('markup with #xyz-mode-only', async ({page}) => {
   const response = await page.goto('/user2/repo1/issues/1');
   expect(response?.status()).toBe(200);
-  await page.waitForLoadState('networkidle');
 
   const comment = page.locator('.comment-body>.markup', {hasText: 'test markup light/dark-mode-only'});
   await expect(comment).toBeVisible();
diff --git a/tests/e2e/profile_actions.test.e2e.ts b/tests/e2e/profile_actions.test.e2e.ts
index efb4dd2f49..51a690aa60 100644
--- a/tests/e2e/profile_actions.test.e2e.ts
+++ b/tests/e2e/profile_actions.test.e2e.ts
@@ -13,7 +13,6 @@ test('Follow actions', async ({browser}, workerInfo) => {
   const page = await context.newPage();
 
   await page.goto('/user1');
-  await page.waitForLoadState('networkidle');
 
   // Check if following and then unfollowing works.
   // This checks that the event listeners of
diff --git a/tests/e2e/repo-code.test.e2e.ts b/tests/e2e/repo-code.test.e2e.ts
index 5207a6389c..b22670ab76 100644
--- a/tests/e2e/repo-code.test.e2e.ts
+++ b/tests/e2e/repo-code.test.e2e.ts
@@ -5,11 +5,7 @@
 // @watch end
 
 import {expect} from '@playwright/test';
-import {test, login_user, load_logged_in_context} from './utils_e2e.ts';
-
-test.beforeAll(async ({browser}, workerInfo) => {
-  await login_user(browser, workerInfo, 'user2');
-});
+import {test} from './utils_e2e.ts';
 
 async function assertSelectedLines(page, nums) {
   const pageAssertions = async () => {
@@ -33,10 +29,7 @@ async function assertSelectedLines(page, nums) {
   return pageAssertions();
 }
 
-test('Line Range Selection', async ({browser}, workerInfo) => {
-  const context = await load_logged_in_context(browser, workerInfo, 'user2');
-  const page = await context.newPage();
-
+test('Line Range Selection', async ({page}) => {
   const filePath = '/user2/repo1/src/branch/master/README.md?display=source';
 
   const response = await page.goto(filePath);
diff --git a/tests/e2e/repo-commitgraph.test.e2e.ts b/tests/e2e/repo-commitgraph.test.e2e.ts
index 2a6fb1e6d7..5f0cad117a 100644
--- a/tests/e2e/repo-commitgraph.test.e2e.ts
+++ b/tests/e2e/repo-commitgraph.test.e2e.ts
@@ -5,11 +5,7 @@
 // @watch end
 
 import {expect} from '@playwright/test';
-import {test, login_user, load_logged_in_context} from './utils_e2e.ts';
-
-test.beforeAll(async ({browser}, workerInfo) => {
-  await login_user(browser, workerInfo, 'user2');
-});
+import {test} from './utils_e2e.ts';
 
 test('Commit graph overflow', async ({page}) => {
   await page.goto('/user2/diff-test/graph');
@@ -18,9 +14,7 @@ test('Commit graph overflow', async ({page}) => {
   await expect(page.locator('.selection.search.dropdown')).toBeInViewport({ratio: 1});
 });
 
-test('Switch branch', async ({browser}, workerInfo) => {
-  const context = await load_logged_in_context(browser, workerInfo, 'user2');
-  const page = await context.newPage();
+test('Switch branch', async ({page}) => {
   const response = await page.goto('/user2/repo1/graph');
   expect(response?.status()).toBe(200);
 
@@ -29,7 +23,7 @@ test('Switch branch', async ({browser}, workerInfo) => {
   await input.pressSequentially('develop', {delay: 50});
   await input.press('Enter');
 
-  await page.waitForLoadState('networkidle');
+  await page.waitForLoadState();
 
   await expect(page.locator('#loading-indicator')).toBeHidden();
   await expect(page.locator('#rel-container')).toBeVisible();
diff --git a/tests/e2e/repo-wiki.test.e2e.ts b/tests/e2e/repo-wiki.test.e2e.ts
index 5b681c583c..f581ecdab6 100644
--- a/tests/e2e/repo-wiki.test.e2e.ts
+++ b/tests/e2e/repo-wiki.test.e2e.ts
@@ -9,7 +9,6 @@ import {test} from './utils_e2e.ts';
 test(`Search for long titles and test for no overflow`, async ({page}, workerInfo) => {
   test.skip(workerInfo.project.name === 'Mobile Safari', 'Fails as always, see https://codeberg.org/forgejo/forgejo/pulls/5326#issuecomment-2313275');
   await page.goto('/user2/repo1/wiki');
-  await page.waitForLoadState('networkidle');
   await page.getByPlaceholder('Search wiki').fill('spaces');
   await page.getByPlaceholder('Search wiki').click();
   // workaround: HTMX listens on keyup events, playwright's fill only triggers the input event
diff --git a/tests/e2e/utils_e2e.ts b/tests/e2e/utils_e2e.ts
index 050847c5ed..89dacce8a4 100644
--- a/tests/e2e/utils_e2e.ts
+++ b/tests/e2e/utils_e2e.ts
@@ -37,7 +37,7 @@ export async function login_user(browser: Browser, workerInfo: TestInfo, user: s
   await page.type('input[name=password]', LOGIN_PASSWORD);
   await page.click('form button.ui.primary.button:visible');
 
-  await page.waitForLoadState('networkidle');
+  await page.waitForLoadState();
 
   expect(page.url(), {message: `Failed to login user ${user}`}).toBe(`${workerInfo.project.use.baseURL}/`);
 
@@ -67,7 +67,7 @@ export async function login({browser}: {browser: Browser}, workerInfo: TestInfo)
 export async function save_visual(page: Page) {
   // Optionally include visual testing
   if (process.env.VISUAL_TEST) {
-    await page.waitForLoadState('networkidle');
+    await page.waitForLoadState('domcontentloaded');
     // Mock page/version string
     await page.locator('footer div.ui.left').evaluate((node) => node.innerHTML = 'MOCK');
     await expect(page).toHaveScreenshot({
diff --git a/tests/e2e/webauthn.test.e2e.ts b/tests/e2e/webauthn.test.e2e.ts
index 38e6b27821..c351b6a468 100644
--- a/tests/e2e/webauthn.test.e2e.ts
+++ b/tests/e2e/webauthn.test.e2e.ts
@@ -8,7 +8,7 @@
 // @watch end
 
 import {expect} from '@playwright/test';
-import {test, create_temp_user} from './utils_e2e.ts';
+import {test, create_temp_user, login_user} from './utils_e2e.ts';
 
 test('WebAuthn register & login flow', async ({browser, request}, workerInfo) => {
   test.skip(workerInfo.project.name !== 'chromium', 'Uses Chrome protocol');
@@ -38,8 +38,10 @@ test('WebAuthn register & login flow', async ({browser, request}, workerInfo) =>
   await page.getByText('Add security key').click();
 
   // Logout.
-  await page.locator('div[aria-label="Profile and settingsā€¦"]').click();
-  await page.getByText('Sign Out').click();
+  await expect(async () => {
+    await page.locator('div[aria-label="Profile and settingsā€¦"]').click();
+    await page.getByText('Sign Out').click();
+  }).toPass();
   await page.waitForURL(`${workerInfo.project.use.baseURL}/`);
 
   // Login.
@@ -57,5 +59,8 @@ test('WebAuthn register & login flow', async ({browser, request}, workerInfo) =>
   expect(response?.status()).toBe(200);
   await page.getByRole('button', {name: 'Remove'}).click();
   await page.getByRole('button', {name: 'Yes'}).click();
-  await page.waitForURL(`${workerInfo.project.use.baseURL}/user/settings/security`);
+  await page.waitForLoadState();
+
+  // verify the user can login without a key
+  await login_user(browser, workerInfo, username);
 });