mirror of
https://codeberg.org/forgejo/forgejo
synced 2025-10-19 00:40:51 +02:00
Had another random failure in `webauthn.test.e2e.ts`: ``` Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── TimeoutError: locator.click: Timeout 3000ms exceeded. Call log: - waiting for getByText('Sign out') - waiting for" http://localhost:3003/user/settings/security" navigation to finish... - navigated to "http://localhost:3003/user/settings/security" - locator resolved to <a href="" tabindex="-1" role="menuitem" id="_aria_auto_id_10" data-url="/user/logout" class="item link-action">…</a> - attempting click action 2 × waiting for element to be visible, enabled and stable - element is not visible - retrying click action - waiting 20ms 2 × waiting for element to be visible, enabled and stable - element is not visible - retrying click action - waiting 100ms 6 × waiting for element to be visible, enabled and stable - element is not visible - retrying click action - waiting 500ms 41 | // Logout. 42 | await page.locator('div[aria-label="Profile and settings…"]').click(); > 43 | await page.getByText('Sign out').click(); | ^ 44 | await expect(async () => { 45 | await page.waitForURL(`${workerInfo.project.use.baseURL}/`); 46 | }).toPass(); at /workspace/forgejo/forgejo/tests/e2e/webauthn.test.e2e.ts:43:36 ``` While attempting to click `Sign out`, playwright waited for page navigation to `http://localhost:3003/user/settings/security` to complete, and then the `Sign out` button never became visible. This suggests to me that the test: - Clicked `Add security key` - There was a race between the browser, which began reloading `/user/settings/security`... - And the test clicked on `Profile and settings…` immediately *before* the new page loaded, since that was visible and available on the old page - Therefore `Sign out` never appeared on the new page to be clicked This PR addresses the race by ensuring that after the security key is added, the page with the security key added is visible (specifically the Remove button). This should prevent the click on "Profile and settings" and "Sign out" from potentially occurring on different pages (as would happen if the reload occurred between the two clicks). I have not been able to reproduce this exact failure locally, but I have tricked my e2e testing situation into reproducing other errors in this test by introducing a synthetic 100ms wait on every web request in the gitea server. After adding this fix, the test does not fail in that scenario. (🤷 Probably good, but no guarantee that we're not going to see another issue.) ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [x] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] I do not want this change to show in the release notes. - [ ] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9688 Reviewed-by: Michael Kriese <michael.kriese@gmx.de> Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net> Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
70 lines
2.7 KiB
TypeScript
70 lines
2.7 KiB
TypeScript
// Copyright 2024 The Forgejo Authors. All rights reserved.
|
|
// SPDX-License-Identifier: MIT
|
|
|
|
// @watch start
|
|
// templates/user/auth/**
|
|
// templates/user/settings/**
|
|
// web_src/js/features/user-**
|
|
// @watch end
|
|
|
|
import {expect} from '@playwright/test';
|
|
import {test, create_temp_user, login_user} from './utils_e2e.ts';
|
|
import {screenshot} from './shared/screenshots.ts';
|
|
|
|
test('WebAuthn register & login flow', async ({browser, request}, workerInfo) => {
|
|
test.skip(workerInfo.project.name !== 'chromium', 'Uses Chrome protocol');
|
|
const {context, username} = await create_temp_user(browser, workerInfo, request);
|
|
const page = await context.newPage();
|
|
|
|
// Register a security key.
|
|
let response = await page.goto('/user/settings/security');
|
|
expect(response?.status()).toBe(200);
|
|
|
|
// https://github.com/microsoft/playwright/issues/7276#issuecomment-1516768428
|
|
const cdpSession = await page.context().newCDPSession(page);
|
|
await cdpSession.send('WebAuthn.enable');
|
|
await cdpSession.send('WebAuthn.addVirtualAuthenticator', {
|
|
options: {
|
|
protocol: 'ctap2',
|
|
ctap2Version: 'ctap2_1',
|
|
hasUserVerification: true,
|
|
transport: 'usb',
|
|
automaticPresenceSimulation: true,
|
|
isUserVerified: true,
|
|
},
|
|
});
|
|
|
|
await page.locator('input#nickname').fill('Testing Security Key');
|
|
await screenshot(page, page.locator('.user-setting-content'));
|
|
await page.getByText('Add security key').click();
|
|
await expect(page.getByRole('button', {name: 'Remove'})).toBeVisible(); // "Remove" button is visible, indicating that the security key was added
|
|
|
|
// Logout.
|
|
await page.locator('div[aria-label="Profile and settings…"]').click();
|
|
await page.getByText('Sign out').click();
|
|
await expect(async () => {
|
|
await page.waitForURL(`${workerInfo.project.use.baseURL}/`);
|
|
}).toPass();
|
|
|
|
// Login.
|
|
response = await page.goto('/user/login');
|
|
expect(response?.status()).toBe(200);
|
|
|
|
await page.getByLabel('Username or email address').fill(username);
|
|
await page.getByLabel('Password').fill('password');
|
|
await page.getByRole('button', {name: 'Sign in'}).click();
|
|
await page.waitForURL(`${workerInfo.project.use.baseURL}/user/webauthn`);
|
|
await page.waitForURL(`${workerInfo.project.use.baseURL}/`);
|
|
|
|
// Cleanup.
|
|
response = await page.goto('/user/settings/security');
|
|
expect(response?.status()).toBe(200);
|
|
await page.getByRole('button', {name: 'Remove'}).click();
|
|
await screenshot(page, page.locator('.ui.g-modal-confirm.delete.modal'), 50);
|
|
await page.getByRole('button', {name: 'Yes'}).click();
|
|
await expect(page.getByRole('button', {name: 'Remove'})).toBeHidden();
|
|
await page.waitForLoadState();
|
|
|
|
// verify the user can login without a key
|
|
await login_user(browser, workerInfo, username);
|
|
});
|