Skip to content

Fix frontend test failures across all browsers #7416

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JohnMcLear merged 7 commits into from
Apr 1, 2026
Merged

Fix frontend test failures across all browsers #7416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d16e3ae
Fix frontend test failures across all browsers
JohnMcLear Mar 31, 2026
a01e3df
Run frontend tests on pull requests
JohnMcLear Mar 31, 2026
60ddbef
Fix sticky chat test: use click() instead of check()/uncheck()
JohnMcLear Mar 31, 2026
de73edf
Fix sticky chat handler and reduce parallel workers
JohnMcLear Mar 31, 2026
3c2a1a1
Clean up workflows: remove Sauce Labs, load test push-only
JohnMcLear Mar 31, 2026
d1b7d23
Fix sticky chat test: click label instead of checkbox
JohnMcLear Mar 31, 2026
19ae682
Fix home button to preserve subpath installations
JohnMcLear Apr 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 4 additions & 57 deletions .github/workflows/frontend-admin-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
# Leave the powered by Sauce Labs bit in as this means we get additional concurrency
name: "Frontend admin tests"

on:
push:
paths-ignore:
- 'doc/**'
pull_request:
paths-ignore:
- 'doc/**'

permissions:
contents: read # to fetch code (actions/checkout)
contents: read

jobs:
withplugins:
Expand All @@ -22,12 +24,6 @@ jobs:
node: [20, 22, 24]

steps:
-
name: Generate Sauce Labs strings
id: sauce_strings
run: |
printf %s\\n '::set-output name=name::${{ github.workflow }} - ${{ github.job }} - Node ${{ matrix.node }}'
printf %s\\n '::set-output name=tunnel_id::${{ github.run_id }}-${{ github.run_number }}-${{ github.job }}-node${{ matrix.node }}'
-
name: Checkout repository
uses: actions/checkout@v6
Expand All @@ -54,31 +50,9 @@ jobs:
path: |
~/.cache/ms-playwright
key: ${{ runner.os }}-playwright-${{ env.PLAYWRIGHT_VERSION }}
#-
# name: Install etherpad plugins
# # We intentionally install an old ep_align version to test upgrades to
# # the minor version number. The --legacy-peer-deps flag is required to
# # work around a bug in npm v7: https://github.com/npm/cli/issues/2199
# run: pnpm install --workspace-root ep_align@0.2.27
# Etherpad core dependencies must be installed after installing the
# plugin's dependencies, otherwise npm will try to hoist common
# dependencies by removing them from src/node_modules and installing them
# in the top-level node_modules. As of v6.14.10, npm's hoist logic appears
# to be buggy, because it sometimes removes dependencies from
# src/node_modules but fails to add them to the top-level node_modules.
# Even if npm correctly hoists the dependencies, the hoisting seems to
# confuse tools such as `npm outdated`, `npm update`, and some ESLint
# rules.
-
name: Install all dependencies and symlink for ep_etherpad-lite
run: gnpm i --runtimeVersion="${{ matrix.node }}"
#-
# name: Install etherpad plugins
# run: rm -Rf node_modules/ep_align/static/tests/*
-
name: export GIT_HASH to env
id: environment
run: echo "::set-output name=sha_short::$(git rev-parse --short ${{ github.sha }})"
-
name: Create settings.json
run: cp settings.json.template settings.json
Expand All @@ -96,33 +70,6 @@ jobs:
working-directory: admin
run: |
gnpm run build --runtimeVersion="${{ matrix.node }}"
# name: Run the frontend admin tests
# shell: bash
# env:
# SAUCE_USERNAME: ${{ secrets.SAUCE_USERNAME }}
# SAUCE_ACCESS_KEY: ${{ secrets.SAUCE_ACCESS_KEY }}
# SAUCE_NAME: ${{ steps.sauce_strings.outputs.name }}
# TRAVIS_JOB_NUMBER: ${{ steps.sauce_strings.outputs.tunnel_id }}
# GIT_HASH: ${{ steps.environment.outputs.sha_short }}
# run: |
# src/tests/frontend/travis/adminrunner.sh
#-
# uses: saucelabs/sauce-connect-action@v2.3.6
# with:
# username: ${{ secrets.SAUCE_USERNAME }}
# accessKey: ${{ secrets.SAUCE_ACCESS_KEY }}
# tunnelIdentifier: ${{ steps.sauce_strings.outputs.tunnel_id }}
#-
# name: Run the frontend admin tests
# shell: bash
# env:
# SAUCE_USERNAME: ${{ secrets.SAUCE_USERNAME }}
# SAUCE_ACCESS_KEY: ${{ secrets.SAUCE_ACCESS_KEY }}
# SAUCE_NAME: ${{ steps.sauce_strings.outputs.name }}
# TRAVIS_JOB_NUMBER: ${{ steps.sauce_strings.outputs.tunnel_id }}
# GIT_HASH: ${{ steps.environment.outputs.sha_short }}
# run: |
# src/tests/frontend/travis/adminrunner.sh
- name: Run the frontend admin tests
shell: bash
run: |
Expand Down
45 changes: 8 additions & 37 deletions .github/workflows/frontend-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
# Leave the powered by Sauce Labs bit in as this means we get additional concurrency
name: "Frontend tests powered by Sauce Labs"
name: "Frontend tests"

on:
push:
paths-ignore:
- 'doc/**'
pull_request:
paths-ignore:
- 'doc/**'

permissions:
contents: read # to fetch code (actions/checkout)
contents: read

jobs:
playwright-chrome:
Expand All @@ -16,12 +18,6 @@ jobs:
name: Playwright Chrome
runs-on: ubuntu-latest
steps:
-
name: Generate Sauce Labs strings
id: sauce_strings
run: |
printf %s\\n '::set-output name=name::${{ github.workflow }} - ${{ github.job }}'
printf %s\\n '::set-output name=tunnel_id::${{ github.run_id }}-${{ github.run_number }}-${{ github.job }}'
-
name: Checkout repository
uses: actions/checkout@v6
Expand All @@ -45,10 +41,6 @@ jobs:
-
name: Install all dependencies and symlink for ep_etherpad-lite
run: gnpm install --frozen-lockfile
-
name: export GIT_HASH to env
id: environment
run: echo "::set-output name=sha_short::$(git rev-parse --short ${{ github.sha }})"
-
name: Create settings.json
run: cp ./src/tests/settings.json settings.json
Expand All @@ -72,7 +64,7 @@ jobs:
- uses: actions/upload-artifact@v7
if: always()
with:
name: playwright-report-${{ matrix.node }}-chrome
name: playwright-report-chrome
path: src/playwright-report/
retention-days: 30
playwright-firefox:
Expand All @@ -81,11 +73,6 @@ jobs:
name: Playwright Firefox
runs-on: ubuntu-latest
steps:
- name: Generate Sauce Labs strings
id: sauce_strings
run: |
printf %s\\n '::set-output name=name::${{ github.workflow }} - ${{ github.job }}'
printf %s\\n '::set-output name=tunnel_id::${{ github.run_id }}-${{ github.run_number }}-${{ github.job }}'
- name: Checkout repository
uses: actions/checkout@v6
- uses: actions/cache@v5
Expand All @@ -107,9 +94,6 @@ jobs:
version: 0.0.12
- name: Install all dependencies and symlink for ep_etherpad-lite
run: gnpm install --frozen-lockfile
- name: export GIT_HASH to env
id: environment
run: echo "::set-output name=sha_short::$(git rev-parse --short ${{ github.sha }})"
- name: Create settings.json
run: cp ./src/tests/settings.json settings.json
- name: Run the frontend tests
Expand All @@ -132,7 +116,7 @@ jobs:
- uses: actions/upload-artifact@v7
if: always()
with:
name: playwright-report-${{ matrix.node }}-firefox
name: playwright-report-firefox
path: src/playwright-report/
retention-days: 30
playwright-webkit:
Expand All @@ -141,12 +125,6 @@ jobs:
env:
PNPM_HOME: ~/.pnpm-store
steps:
-
name: Generate Sauce Labs strings
id: sauce_strings
run: |
printf %s\\n '::set-output name=name::${{ github.workflow }} - ${{ github.job }}'
printf %s\\n '::set-output name=tunnel_id::${{ github.run_id }}-${{ github.run_number }}-${{ github.job }}'
-
name: Checkout repository
uses: actions/checkout@v6
Expand All @@ -169,10 +147,6 @@ jobs:
-
name: Install all dependencies and symlink for ep_etherpad-lite
run: gnpm install --frozen-lockfile
-
name: export GIT_HASH to env
id: environment
run: echo "::set-output name=sha_short::$(git rev-parse --short ${{ github.sha }})"
-
name: Create settings.json
run: cp ./src/tests/settings.json settings.json
Expand All @@ -196,9 +170,6 @@ jobs:
- uses: actions/upload-artifact@v7
if: always()
with:
name: playwright-report-${{ matrix.node }}-webkit
name: playwright-report-webkit
path: src/playwright-report/
retention-days: 30



4 changes: 0 additions & 4 deletions .github/workflows/load-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
name: "Loadtest"

# any branch is useful for testing before a PR is submitted
on:
push:
paths-ignore:
- "doc/**"
pull_request:
paths-ignore:
- "doc/**"

permissions:
contents: read
Expand Down
2 changes: 1 addition & 1 deletion src/playwright.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export default defineConfig({
expect: { timeout: defaultExpectTimeout },
timeout: defaultTestTimeout,
retries: 0,
workers: 5,
workers: 2,
/* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */
use: {
/* Base URL to use in actions like `await page.goto('/')`. */
Expand Down
2 changes: 1 addition & 1 deletion src/static/js/pad_editbar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ exports.padeditbar = new class {
this.registerDropdownCommand('import_export');
this.registerDropdownCommand('embed');
this.registerCommand('home', ()=>{
window.location.href = window.location.href + "/../.."
window.location.href = new URL('../..', window.location.href).href
})

this.registerCommand('settings', () => {
Expand Down
3 changes: 3 additions & 0 deletions src/tests/frontend-new/helper/padHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,19 +111,22 @@ export const appendQueryParams = async (page: Page, queryParameters: MapArrayTyp
});
await page.goto(page.url()+"?"+ searchParams.toString());
await page.waitForSelector('iframe[name="ace_outer"]');
await page.waitForSelector('#editorcontainer.initialized');

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Mar 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. padhelper added waits misindented 📘 Rule violation ⚙ Maintainability

The newly added waitForSelector('#editorcontainer.initialized') lines are not indented with 2
spaces as required. This introduces formatting that violates the indentation standard.
Agent Prompt 
## Issue description
Newly added lines in `padHelper.ts` do not follow 2-space indentation.

## Issue Context
Compliance requires spaces-only and 2-space indentation across the codebase.

## Fix Focus Areas
- src/tests/frontend-new/helper/padHelper.ts[114-114]
- src/tests/frontend-new/helper/padHelper.ts[122-122]
- src/tests/frontend-new/helper/padHelper.ts[129-129]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}

export const goToNewPad = async (page: Page) => {
// create a new pad before each test run
const padId = "FRONTEND_TESTS"+randomUUID();
await page.goto('http://localhost:9001/p/'+padId);
await page.waitForSelector('iframe[name="ace_outer"]');
await page.waitForSelector('#editorcontainer.initialized');
return padId;
}

export const goToPad = async (page: Page, padId: string) => {
await page.goto('http://localhost:9001/p/'+padId);
await page.waitForSelector('iframe[name="ace_outer"]');
await page.waitForSelector('#editorcontainer.initialized');
}


Expand Down
8 changes: 4 additions & 4 deletions src/tests/frontend-new/helper/settingsHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ export const enableStickyChatviaSettings = async (page: Page) => {
const stickyChat = page.locator('#options-stickychat')
const checked = await stickyChat.isChecked()
if(checked) return
await stickyChat.check({force: true})
await page.waitForSelector('#options-stickychat:checked')
await page.locator('label[for="options-stickychat"]').click()
await page.waitForFunction(() => document.querySelector('#chatbox')?.classList.contains('stickyChat'))
}

export const disableStickyChat = async (page: Page) => {
const stickyChat = page.locator('#options-stickychat')
const checked = await stickyChat.isChecked()
if(!checked) return
await stickyChat.uncheck({force: true})
await page.waitForSelector('#options-stickychat:not(:checked)')
await page.locator('label[for="options-stickychat"]').click()
await page.waitForFunction(() => !document.querySelector('#chatbox')?.classList.contains('stickyChat'))
}
3 changes: 2 additions & 1 deletion src/tests/frontend-new/specs/chat.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import {
import {disableStickyChat, enableStickyChatviaSettings, hideSettings, showSettings} from "../helper/settingsHelper";


test.beforeEach(async ({ page })=>{
test.beforeEach(async ({ page, context })=>{
await context.clearCookies();
Comment on lines +17 to +18

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Mar 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

3. chat.spec beforeeach misindented 📘 Rule violation ⚙ Maintainability

The newly added context.clearCookies() line (and surrounding modified setup) is not indented with
2 spaces as required. This violates the formatting/indentation compliance rule.
Agent Prompt 
## Issue description
New lines in `chat.spec.ts` do not follow 2-space indentation.

## Issue Context
Compliance requires spaces-only and 2-space indentation.

## Fix Focus Areas
- src/tests/frontend-new/specs/chat.spec.ts[17-18]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

await goToNewPad(page);
})

Expand Down
1 change: 1 addition & 0 deletions src/tests/frontend-new/specs/editbar.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ test('should go to home on pad', async ({page}) => {
expect(attribute).toBe('pad.toolbar.home.title');

await homeButton.click();
await page.waitForURL((url) => !url.pathname.includes('/p/'));

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Mar 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

4. editbar.spec waitforurl misindented 📘 Rule violation ⚙ Maintainability

The newly added page.waitForURL(...) line is not indented with 2 spaces as required. This violates
the code formatting standard for indentation.
Agent Prompt 
## Issue description
A newly added line in `editbar.spec.ts` does not follow 2-space indentation.

## Issue Context
Compliance requires spaces-only and 2-space indentation.

## Fix Focus Areas
- src/tests/frontend-new/specs/editbar.spec.ts[15-15]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

const url = page.url();
expect(url).not.toContain('/p/');
})
Loading