From 905ca269871a7edbbb84a0c9511b5f2f5bcec2bf Mon Sep 17 00:00:00 2001 From: Marie Idleman Date: Mon, 13 Jan 2025 15:18:52 -0600 Subject: [PATCH] e2e-test: fix notebook-create flakes (#5976) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Summary Trying to fix flakes on `notebook-create.test.ts`. ### QA Notes 🟡 Running [10x on Linux](https://github.com/posit-dev/positron/actions/runs/12755915878) @:notebook --- test/e2e/pages/notebooks.ts | 97 +++++++++++-------- test/e2e/pages/quickaccess.ts | 93 ++++++++++-------- .../tests/notebook/notebook-create.test.ts | 4 +- 3 files changed, 110 insertions(+), 84 deletions(-) diff --git a/test/e2e/pages/notebooks.ts b/test/e2e/pages/notebooks.ts index d5a83a9bc1b..eea699d2427 100644 --- a/test/e2e/pages/notebooks.ts +++ b/test/e2e/pages/notebooks.ts @@ -7,7 +7,7 @@ import { Code } from '../infra/code'; import { QuickInput } from './quickInput'; import { QuickAccess } from './quickaccess'; import { basename } from 'path'; -import { expect } from '@playwright/test'; +import test, { expect } from '@playwright/test'; const KERNEL_LABEL = '.kernel-label'; @@ -37,24 +37,33 @@ export class Notebooks { constructor(private code: Code, private quickinput: QuickInput, private quickaccess: QuickAccess) { } async selectInterpreter(kernelGroup: string, desiredKernel: string) { - await expect(this.notebookProgressBar).not.toBeVisible({ timeout: 30000 }); - await expect(this.code.driver.page.locator(DETECTING_KERNELS_TEXT)).not.toBeVisible({ timeout: 30000 }); - - // Wait for either "select kernel" or "the desired kernel" to appear in KERNEL_LABEL - const kernelRegex = new RegExp(`${SELECT_KERNEL_TEXT}|${desiredKernel}`); - const kernelLabelLocator = this.code.driver.page.locator(KERNEL_LABEL); - await expect(kernelLabelLocator).toHaveText(kernelRegex, { timeout: 10000 }); - - // Retrieve the matched text for conditional logic - const matchedText = await kernelLabelLocator.textContent() || ''; - - if (!new RegExp(desiredKernel).test(matchedText)) { - await this.code.driver.page.locator(KERNEL_ACTION).click(); - await this.quickinput.waitForQuickInputOpened(); - await this.quickinput.selectQuickInputElementContaining(kernelGroup); - await this.quickinput.selectQuickInputElementContaining(desiredKernel); - await this.quickinput.waitForQuickInputClosed(); - } + await test.step(`Select notebook interpreter: ${desiredKernel}`, async () => { + await expect(this.notebookProgressBar).not.toBeVisible({ timeout: 30000 }); + await expect(this.code.driver.page.locator(DETECTING_KERNELS_TEXT)).not.toBeVisible({ timeout: 30000 }); + + // Wait for the desired kernel to populate in dropdown, if no show then wait for "Select Kernel" + const kernelLabelLocator = this.code.driver.page.locator(KERNEL_LABEL); + try { + await expect(kernelLabelLocator).toHaveText(new RegExp(desiredKernel), { timeout: 10000 }); + } catch (e) { + await expect(kernelLabelLocator).toHaveText(new RegExp(SELECT_KERNEL_TEXT), { timeout: 10000 }); + } + + // Retrieve the matched text for conditional logic + const matchedText = await kernelLabelLocator.textContent() || ''; + + this.code.logger.log(`Matched text: ${matchedText}, Desired kernel: ${desiredKernel}`); + if (!new RegExp(desiredKernel).test(matchedText)) { + await this.code.driver.page.locator(KERNEL_ACTION).click(); + await this.quickinput.waitForQuickInputOpened(); + await this.quickinput.selectQuickInputElementContaining(kernelGroup); + await this.quickinput.selectQuickInputElementContaining(desiredKernel); + await this.quickinput.waitForQuickInputClosed(); + } + + // wait for the kernel to load + await expect(this.code.driver.page.locator('.kernel-action-view-item').locator('.codicon-modifier-spin')).not.toBeVisible({ timeout: 30000 }); + }); } async createNewNotebook() { @@ -63,22 +72,28 @@ export class Notebooks { // Opens a Notebook that lives in the current workspace async openNotebook(path: string) { - await this.quickaccess.openFileQuickAccessAndWait(basename(path), 1); - await this.quickinput.selectQuickInputElement(0); + await test.step(`Open notebook: ${path}`, async () => { + await this.quickaccess.openFileQuickAccessAndWait(basename(path), 1); + await this.quickinput.selectQuickInputElement(0); - await expect(this.code.driver.page.locator(ACTIVE_ROW_SELECTOR)).toBeVisible(); - await this.focusFirstCell(); + await expect(this.code.driver.page.locator(ACTIVE_ROW_SELECTOR)).toBeVisible(); + await this.focusFirstCell(); + }); } async addCodeToFirstCell(code: string) { - await this.code.driver.page.locator(CELL_LINE).first().click(); - await this.waitForTypeInEditor(code); - await this.waitForActiveCellEditorContents(code); + await test.step('Add code to first cell', async () => { + await this.code.driver.page.locator(CELL_LINE).first().click(); + await this.typeInEditor(code); + await this.waitForActiveCellEditorContents(code); + }); } async executeCodeInCell() { - await this.quickaccess.runCommand(EXECUTE_CELL_COMMAND); - await expect(this.code.driver.page.locator(EXECUTE_CELL_SPINNER), 'execute cell spinner to not be visible').not.toBeVisible({ timeout: 30000 }); + await test.step('Execute code in cell', async () => { + await this.quickaccess.runCommand(EXECUTE_CELL_COMMAND); + await expect(this.code.driver.page.locator(EXECUTE_CELL_SPINNER), 'execute cell spinner to not be visible').not.toBeVisible({ timeout: 30000 }); + }); } async assertCellOutput(text: string): Promise { @@ -96,27 +111,31 @@ export class Notebooks { } async runAllCells(timeout: number = 30000) { - await this.code.driver.page.getByLabel('Run All').click(); - const stopExecutionLocator = this.code.driver.page.locator('a').filter({ hasText: /Stop Execution|Interrupt/ }); - await expect(stopExecutionLocator).toBeVisible(); - await expect(stopExecutionLocator).not.toBeVisible({ timeout: timeout }); + await test.step('Run all cells', async () => { + await this.code.driver.page.getByLabel('Run All').click(); + const stopExecutionLocator = this.code.driver.page.locator('a').filter({ hasText: /Stop Execution|Interrupt/ }); + await expect(stopExecutionLocator).toBeVisible(); + await expect(stopExecutionLocator).not.toBeVisible({ timeout: timeout }); + }); } async focusFirstCell() { await this.quickaccess.runCommand('notebook.focusTop'); } - async waitForTypeInEditor(text: string): Promise { - const editor = `${ACTIVE_ROW_SELECTOR} .monaco-editor`; + async typeInEditor(text: string): Promise { + await test.step(`Type in editor: ${text}`, async () => { + const editor = `${ACTIVE_ROW_SELECTOR} .monaco-editor`; - await this.code.driver.page.locator(editor).isVisible(); + await this.code.driver.page.locator(editor).isVisible(); - const textarea = `${editor} textarea`; - await expect(this.code.driver.page.locator(textarea)).toBeFocused(); + const textarea = `${editor} textarea`; + await expect(this.code.driver.page.locator(textarea)).toBeFocused(); - await this.code.driver.page.locator(textarea).fill(text); + await this.code.driver.page.locator(textarea).fill(text); - await this._waitForActiveCellEditorContents(c => c.indexOf(text) > -1); + await this._waitForActiveCellEditorContents(c => c.indexOf(text) > -1); + }); } private async _waitForActiveCellEditorContents(accept: (contents: string) => boolean): Promise { diff --git a/test/e2e/pages/quickaccess.ts b/test/e2e/pages/quickaccess.ts index d1b6dab9001..305ae1c23bf 100644 --- a/test/e2e/pages/quickaccess.ts +++ b/test/e2e/pages/quickaccess.ts @@ -6,7 +6,7 @@ import { Code } from '../infra/code'; import { basename, isAbsolute } from 'path'; import { QuickInput } from './quickInput'; -import { expect } from '@playwright/test'; +import test, { expect } from '@playwright/test'; import { Editors } from './editors'; enum QuickAccessKind { @@ -92,25 +92,27 @@ export class QuickAccess { } async openFile(path: string): Promise { - if (!isAbsolute(path)) { - // we require absolute paths to get a single - // result back that is unique and avoid hitting - // the search process to reduce chances of - // search needing longer. - throw new Error('QuickAccess.openFile requires an absolute path'); - } + await test.step('Open file', async () => { + if (!isAbsolute(path)) { + // we require absolute paths to get a single + // result back that is unique and avoid hitting + // the search process to reduce chances of + // search needing longer. + throw new Error('QuickAccess.openFile requires an absolute path'); + } - const fileName = basename(path); + const fileName = basename(path); - // quick access shows files with the basename of the path - await this.openFileQuickAccessAndWait(path, basename(path)); + // quick access shows files with the basename of the path + await this.openFileQuickAccessAndWait(path, basename(path)); - // open first element - await this.quickInput.selectQuickInputElement(0); + // open first element + await this.quickInput.selectQuickInputElement(0); - // wait for editor being focused - await this.editors.waitForActiveTab(fileName); - await this.editors.selectTab(fileName); + // wait for editor being focused + await this.editors.waitForActiveTab(fileName); + await this.editors.selectTab(fileName); + }); } private async openQuickAccessWithRetry(kind: QuickAccessKind, value?: string): Promise { @@ -151,39 +153,44 @@ export class QuickAccess { async runCommand(commandId: string, options?: { keepOpen?: boolean; exactLabelMatch?: boolean }): Promise { - const keepOpen = options?.keepOpen; - const exactLabelMatch = options?.exactLabelMatch; + const stepWrapper = (label: string, fn: () => Promise) => { + try { + // Check if running in a test context + if (test.info().title) { + return test.step(label, fn); // Use test.step if inside a test + } + } catch (e) { + // Catch errors if not in a test context + } + return fn(); // Run directly if not in a test + }; - const openCommandPalletteAndTypeCommand = async (): Promise => { - // open commands picker - await this.openQuickAccessWithRetry(QuickAccessKind.Commands, `>${commandId}`); + await stepWrapper(`Run command: ${commandId}`, async () => { + const keepOpen = options?.keepOpen; + const exactLabelMatch = options?.exactLabelMatch; - // wait for quick input element text - const text = await this.quickInput.waitForQuickInputElementText(); + const openCommandPalletteAndTypeCommand = async (): Promise => { + await this.openQuickAccessWithRetry(QuickAccessKind.Commands, `>${commandId}`); + const text = await this.quickInput.waitForQuickInputElementText(); - if (text === 'No matching commands' || (exactLabelMatch && text !== commandId)) { - return false; - } + return !(text === 'No matching commands' || (exactLabelMatch && text !== commandId)); + }; - return true; - }; + await expect(async () => { + const hasCommandFound = await openCommandPalletteAndTypeCommand(); + if (!hasCommandFound) { + this.code.logger.log(`QuickAccess: No matching commands, retrying...`); + await this.quickInput.closeQuickInput(); + throw new Error('Command not found'); + } + }).toPass({ + timeout: 15000, + intervals: [1000], + }); - await expect(async () => { - const hasCommandFound = await openCommandPalletteAndTypeCommand(); - if (!hasCommandFound) { - this.code.logger.log(`QuickAccess: No matching commands, retrying...`); - await this.quickInput.closeQuickInput(); - throw new Error('Command not found'); // Signal to retry - } - }).toPass({ - timeout: 15000, - intervals: [1000], + this.code.logger.log('QuickAccess: Command found and successfully executed.'); + await this.quickInput.selectQuickInputElement(0, keepOpen); }); - - this.code.logger.log('QuickAccess: Command found and successfully executed.'); - - // wait and click on best choice - await this.quickInput.selectQuickInputElement(0, keepOpen); } async openQuickOutline({ timeout = 30000 }): Promise { diff --git a/test/e2e/tests/notebook/notebook-create.test.ts b/test/e2e/tests/notebook/notebook-create.test.ts index 41381f6cfc7..fc2c644835b 100644 --- a/test/e2e/tests/notebook/notebook-create.test.ts +++ b/test/e2e/tests/notebook/notebook-create.test.ts @@ -33,7 +33,7 @@ test.describe('Notebooks', { const randomText = Math.random().toString(36).substring(7); await app.workbench.notebooks.insertNotebookCell('markdown'); - await app.workbench.notebooks.waitForTypeInEditor(`## ${randomText} `); + await app.workbench.notebooks.typeInEditor(`## ${randomText} `); await app.workbench.notebooks.stopEditingCell(); await app.workbench.notebooks.assertMarkdownText('h2', randomText); }); @@ -60,7 +60,7 @@ test.describe('Notebooks', { const randomText = Math.random().toString(36).substring(7); await app.workbench.notebooks.insertNotebookCell('markdown'); - await app.workbench.notebooks.waitForTypeInEditor(`## ${randomText} `); + await app.workbench.notebooks.typeInEditor(`## ${randomText} `); await app.workbench.notebooks.stopEditingCell(); await app.workbench.notebooks.assertMarkdownText('h2', randomText); });