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
This commit is contained in:
Otto Richter 2024-11-12 21:07:09 +01:00
parent a39f726643
commit 40551de313
15 changed files with 230 additions and 117 deletions

View file

@ -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

View file

@ -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,

View file

@ -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;

View file

@ -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();
~~~

View file

@ -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);
});

View file

@ -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');
});

View file

@ -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');

View file

@ -11,17 +11,24 @@ test.beforeAll(async ({browser}, workerInfo) => {
await login_user(browser, workerInfo, 'user2');
});
// belongs to test: Pull: Toggle WIP
/* 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 click_toggle_wip({page}) {
await page.locator('.toggle-wip>a').click();
await page.waitForLoadState('networkidle');
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();
}
}
async function check_wip({page}, is) {
const elemTitle = '#issue-title-display';
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) {
@ -33,70 +40,108 @@ async function check_wip({page}, is) {
}
}
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');
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
// 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);
// 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
// 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 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();
await page.locator('.select-label').click();
await page.waitForLoadState('networkidle');
}).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();

View file

@ -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();

View file

@ -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

View file

@ -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);

View file

@ -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();

View file

@ -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

View file

@ -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({

View file

@ -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 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);
});