Skip to content

Commit

Permalink
e2e-test: fix notebook-create flakes (#5976)
Browse files Browse the repository at this point in the history 
### 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
  • Loading branch information
@midleman
midleman authored Jan 13, 2025
1 parent 29917ca commit 905ca26
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 84 deletions.
97 changes: 58 additions & 39 deletions test/e2e/pages/notebooks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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() {
Expand All @@ -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<void> {
Expand All @@ -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<any> {
const editor = `${ACTIVE_ROW_SELECTOR} .monaco-editor`;
async typeInEditor(text: string): Promise<any> {
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<string> {
Expand Down
93 changes: 50 additions & 43 deletions test/e2e/pages/quickaccess.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -92,25 +92,27 @@ export class QuickAccess {
}

async openFile(path: string): Promise<void> {
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<void> {
Expand Down Expand Up @@ -151,39 +153,44 @@ export class QuickAccess {


async runCommand(commandId: string, options?: { keepOpen?: boolean; exactLabelMatch?: boolean }): Promise<void> {
const keepOpen = options?.keepOpen;
const exactLabelMatch = options?.exactLabelMatch;
const stepWrapper = (label: string, fn: () => Promise<void>) => {
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<boolean> => {
// 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<boolean> => {
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<void> {
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/tests/notebook/notebook-create.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand All @@ -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);
});
Expand Down

0 comments on commit 905ca26

Please sign in to comment.