[ZEPPELIN-6422] Stabilize flaky Playwright E2E tests#5262
Conversation
|
|
||
| async enterNewName(name: string): Promise<void> { | ||
| await this.renameInput.fill(name); | ||
| await this.fillAndVerifyInput(this.renameInput, name); |
There was a problem hiding this comment.
pressSequentially reproduced the same race. Angular's FormControl initializes after any key/value input, so every input method hits the same reset window. Routed this and the other modal-input specs through BasePage.fillAndVerifyInput instead.
| private getNoteByName(noteName: string): Locator { | ||
| return this.page.locator('nz-tree-node').filter({ hasText: noteName }).first(); | ||
| noteLinkByName(noteName: string): Locator { | ||
| return this.nodeListContainer.getByRole('link', { name: noteName, exact: true }); |
There was a problem hiding this comment.
Master's substring hasText + .first() non-deterministically picked one of several TestNotebook_* nodes coexisting in the shared workspace. getByRole('link', { name, exact: true }) is exact-match and the Playwright-recommended locator.
| export const STORAGE_STATE = path.join('playwright', '.auth', 'user.json'); | ||
|
|
||
| setup('authenticate', async ({ page }) => { | ||
| fs.mkdirSync(path.dirname(STORAGE_STATE), { recursive: true }); | ||
|
|
||
| const isShiroEnabled = await LoginTestUtil.isShiroEnabled(); | ||
| if (!isShiroEnabled) { | ||
| // Auth variant disabled — write an empty storage state so dependent projects load, | ||
| // then exit. This keeps the setup-project pattern uniform across CI matrix variants. | ||
| await page.context().storageState({ path: STORAGE_STATE }); | ||
| return; | ||
| } | ||
|
|
||
| await page.goto('/'); | ||
| await waitForZeppelinReady(page); | ||
|
|
||
| await performLoginIfRequired(page); | ||
|
|
||
| // Verify we are authenticated. Don't rely on performLoginIfRequired's return value — | ||
| // it returns false both for "no work to do" and "login attempt failed". | ||
| await expect(page.locator('zeppelin-login')).toBeHidden({ timeout: 30000 }); | ||
| await expect(page.getByRole('heading', { name: 'Welcome to Zeppelin!' })).toBeVisible({ timeout: 30000 }); | ||
|
|
||
| await page.context().storageState({ path: STORAGE_STATE }); | ||
| }); |
There was a problem hiding this comment.
This setup project is the core change in the PR. The 29 specs that previously called login in their own beforeEach raced on the shared session cookie under parallel workers. Consolidating to a single login + storageState removes that race.
|
Could you rebase onto the master branch first? There have been many dependency updates, so I think the rebase needs to happen before anything else. |
1c2aaba to
97ac984
Compare
| const createNotebookViaRest = async ( | ||
| page: Page, | ||
| notebookName: string | ||
| ): Promise<{ noteId: string; paragraphId: string }> => { | ||
| const defaultInterpreterGroup = await getDefaultInterpreterGroup(page); | ||
| const payload: Record<string, unknown> = { | ||
| notePath: notebookName, | ||
| addingEmptyParagraph: true | ||
| }; | ||
|
|
||
| if (defaultInterpreterGroup) { | ||
| payload.defaultInterpreterGroup = defaultInterpreterGroup; | ||
| } | ||
|
|
||
| await notebookLink.click({ timeout: 15000 }); | ||
| await page.waitForURL(NOTEBOOK_PATTERNS.URL_REGEX, { timeout: 20000 }); | ||
| const createResponse = await page.request.post('/api/notebook', { | ||
| data: payload, | ||
| failOnStatusCode: false | ||
| }); | ||
| if (!createResponse.ok()) { | ||
| throw new Error(`Create notebook REST request failed: ${createResponse.status()} ${await createResponse.text()}`); | ||
| } | ||
|
|
||
| const noteId = await extractNoteIdFromUrl(page); | ||
| const createJson = (await createResponse.json()) as ZeppelinJsonResponse<string>; | ||
| const noteId = createJson.body; | ||
| if (!noteId) { | ||
| throw new Error('Failed to extract notebook ID after home page navigation'); | ||
| throw new Error(`Create notebook REST response did not include note id: ${JSON.stringify(createJson)}`); | ||
| } | ||
|
|
||
| return noteId; | ||
| }; | ||
|
|
||
| const extractFirstParagraphId = async (page: Page): Promise<string> => { | ||
| await page.locator('zeppelin-notebook-paragraph').first().waitFor({ state: 'visible', timeout: 20000 }); | ||
| let noteJson!: ZeppelinJsonResponse<NoteSummary>; | ||
| await expect(async () => { | ||
| const response = await page.request.get(`/api/notebook/${noteId}`, { failOnStatusCode: false }); | ||
| if (!response.ok()) { | ||
| throw new Error(`Fetch notebook REST request failed: ${response.status()} ${await response.text()}`); | ||
| } | ||
| noteJson = (await response.json()) as ZeppelinJsonResponse<NoteSummary>; | ||
| }).toPass({ timeout: 7500, intervals: [500, 1000, 1500, 2000, 2500] }); | ||
|
|
||
| const paragraphContainer = page.locator('zeppelin-notebook-paragraph').first(); | ||
| const dropdownTrigger = paragraphContainer.locator('a[nz-dropdown]'); | ||
| await dropdownTrigger.click(); | ||
| const paragraphId = noteJson.body?.paragraphs?.[0]?.id; | ||
| if (!paragraphId || !paragraphId.startsWith('paragraph_')) { | ||
| throw new Error(`Create notebook REST response did not include paragraph id: ${JSON.stringify(noteJson.body)}`); | ||
| } | ||
|
|
||
| const paragraphLink = page.locator('li.paragraph-id a').first(); | ||
| await paragraphLink.waitFor({ state: 'attached', timeout: 15000 }); | ||
| return { noteId, paragraphId }; | ||
| }; |
There was a problem hiding this comment.
UI create-note is already covered by note-create-modal.spec.ts. Reusing it for setup would re-run the same Ant-modal + ngModel race in every spec's beforeEach. REST is deterministic and keeps that coverage in one spec.
Rebased onto the latest master.
95eff2f to
f4e05ca
Compare
3769420 to
c8b6026
Compare
…ncomplete on master
d632501 to
5bb91f2
Compare
What is this PR for?
Three races caused flaky
frontend / run-playwright-e2e-tests:setupproject +storageState.locator.fillon Ant modal inputs landed before Angular bound the form-control -> newBasePage.fillAndVerifyInput()retries viaexpect.toPassuntil the input value sticks.Inline comments on the diff for the non-obvious bits.
What type of PR is it?
Bug Fix
Todos
What is the Jira issue?
ZEPPELIN-6422
How should this be tested?
Screenshots (if appropriate)
Questions: