From 46c3814a4a81ed1b33e86cb8c4dafc1277fdab5d Mon Sep 17 00:00:00 2001
From: Nirmal Kumar R <mysticmode@yahoo.com>
Date: Fri, 5 Sep 2025 00:25:23 +0200
Subject: [PATCH] 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>
---
 tests/e2e/repo-settings.test.e2e.ts | 40 +++++++++++++++++------------
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/tests/e2e/repo-settings.test.e2e.ts b/tests/e2e/repo-settings.test.e2e.ts
index 3d260866fb..547b015921 100644
--- a/tests/e2e/repo-settings.test.e2e.ts
+++ b/tests/e2e/repo-settings.test.e2e.ts
@@ -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');
-  });
 });