test(e2e): Fix and refactor repo-settings flaky test (#9036)

Refactor:
---
Hooks should be placed before the tests such that it is obvious that you read what is set in hooks before reading the tests. Here, test.afterEach is placed at the top of test describe.

Uncomment the Mobile Safari browser, it has to work and is working.

Fixes:
---
On the afterEach hook, we were deleting the rules and waiting for the page load, with await page.waitForLoadState('domcontentloaded') which behaves unreliably or being flaky on multiple runs.

In order to fix this, we can simply go for the selector count expectation to be 0 that is not present. In this case, the delete button should not be present after the rule is successfully deleted.

Also, avoid using page.getByText instead use page.locator with selectors, because getByText will lead to problems when you have the text present elsewhere in the same page.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9036
Reviewed-by: Antonin Delpeuch <wetneb@noreply.codeberg.org>
Reviewed-by: Beowulf <beowulf@beocode.eu>
Reviewed-by: Otto <otto@codeberg.org>
Co-authored-by: Nirmal Kumar R <mysticmode@yahoo.com>
Co-committed-by: Nirmal Kumar R <mysticmode@yahoo.com>
This commit is contained in:
Nirmal Kumar R 2025-09-05 00:25:23 +02:00 committed by Otto
commit 46c3814a4a

View file

@ -12,8 +12,7 @@ import {validate_form} from './shared/forms.ts';
test.use({user: 'user2'});
test('repo webhook settings', async ({page}, workerInfo) => {
test.skip(workerInfo.project.name === 'Mobile Safari', 'Cannot get it to work - as usual');
test('repo webhook settings', async ({page}) => {
const response = await page.goto('/user2/repo1/settings/hooks/forgejo/new');
expect(response?.status()).toBe(200);
@ -32,8 +31,20 @@ test('repo webhook settings', async ({page}, workerInfo) => {
});
test.describe('repo branch protection settings', () => {
test('form', async ({page}, {project}) => {
test.skip(project.name === 'Mobile Safari', 'Cannot get it to work - as usual');
test.afterEach(async ({page}) => {
// delete the rule for the next test
await page.goto('/user2/repo1/settings/branches/');
await page.waitForLoadState('domcontentloaded');
const deleteButton = page.locator('.delete-button').first();
test.skip(await deleteButton.isHidden(), 'Nothing to delete at this time');
await deleteButton.click();
await page.locator('#delete-protected-branch .actions .ok').click();
// Here page.waitForLoadState('domcontentloaded') does not work reliably.
// Instead, wait for the delete button to disappear.
await expect(deleteButton).toHaveCount(0);
});
test('form', async ({page}) => {
const response = await page.goto('/user2/repo1/settings/branches/edit');
expect(response?.status()).toBe(200);
@ -43,22 +54,17 @@ test.describe('repo branch protection settings', () => {
await expect(page.locator('h4')).toContainText('new');
await page.locator('input[name="rule_name"]').fill('testrule');
await save_visual(page);
await page.getByText('Save rule').click();
await page.locator('button:text("Save rule")').click();
// verify header is in edit mode
await page.waitForLoadState('domcontentloaded');
await save_visual(page);
await page.getByText('Edit').click();
await expect(page.locator('h4')).toContainText('Protection rules for branch');
// find the edit button and click it
const editButton = page.locator('a[href="/user2/repo1/settings/branches/edit?rule_name=testrule"]');
await editButton.click();
await page.waitForLoadState();
await expect(page.locator('.repo-setting-content .header')).toContainText('Protection rules for branch', {ignoreCase: true, useInnerText: true});
await save_visual(page);
});
test.afterEach(async ({page}) => {
// delete the rule for the next test
await page.goto('/user2/repo1/settings/branches/');
await page.waitForLoadState('domcontentloaded');
test.skip(await page.getByText('Delete rule').isHidden(), 'Nothing to delete at this time');
await page.getByText('Delete rule').click();
await page.getByText('Yes').click();
await page.waitForLoadState('load');
});
});