Skip to content

Commit

Permalink
e2e-test: more notebook fixes (#5154)
Browse files Browse the repository at this point in the history 
### Intent
Make the `notebookCreate.test.ts` more stable / less flakey.

### Approach
The main issue I noticed with the notebook creation test was that we
weren’t properly asserting the text content of locators. We were
fetching the locator and then immediately verifying its text. This works
fine for non-async code, but since we are dealing with asynchronous
behavior, this approach isn’t reliable. Instead, we should leverage
Playwright’s built-in retry mechanism in the expect methods, which will
automatically keep checking the locator until its text matches the
expected value or the timeout is reached.

I'm hoping some of the fixes help other tests that rely on executing
code, etc. 🤞

### Examples
#### Don't Do This
This approach fails immediately if the element’s text is not ready.
```
const textContent = await page.locator('.element').textContent();
expect(textContent).toBe('Expected Text');  // Immediate assertion
```

#### Do This
Playwright will retry until the element contains the expected text,
making it more resilient.
```
await expect(page.locator('.element')).toHaveText('Expected Text');  // Waits for the condition to be met
```

#### Tip
Your `await` should always precede the `expect` and never be "inside" it
(like in the first example).
* Use await on the expect statement when you are testing dynamic or
asynchronous content where the state of the page may change over time.
* Use await on the locator or property directly when you are certain
that the content is ready and available immediately, and you want to
perform the assertion right away.
  • Loading branch information
@midleman
midleman authored Oct 24, 2024
1 parent 69b1efb commit e58dd99
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 64 deletions.
36 changes: 12 additions & 24 deletions test/automation/src/positron/positronNotebooks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { Notebook } from '../notebook';
import { QuickAccess } from '../quickaccess';
import { QuickInput } from '../quickinput';
import { basename } from 'path';
import { expect } from '@playwright/test';

const KERNEL_LABEL = '.kernel-label';
const KERNEL_ACTION = '.kernel-action-view-item';
Expand All @@ -16,19 +17,19 @@ const DETECTING_KERNELS_TEXT = 'Detecting Kernels';
const NEW_NOTEBOOK_COMMAND = 'ipynb.newUntitledIpynb';
const CELL_LINE = '.cell div.view-lines';
const EXECUTE_CELL_COMMAND = 'notebook.cell.execute';
const OUTER_FRAME = '.webview';
const EXECUTE_CELL_SPINNER = '.cell-status-item .codicon-modifier-spin';
const INNER_FRAME = '#active-frame';
const PYTHON_OUTPUT = '.output-plaintext';
const R_OUTPUT = '.output_container .output';
const REVERT_AND_CLOSE = 'workbench.action.revertAndCloseActiveEditor';
const MARKDOWN_TEXT = '#preview';
const ACTIVE_ROW_SELECTOR = `.notebook-editor .monaco-list-row.focused`;


/*
* Reuseable Positron notebook functionality for tests to leverage. Includes selecting the notebook's interpreter.
*/
export class PositronNotebooks {
kernelLabel = this.code.driver.getLocator(KERNEL_LABEL);
frameLocator = this.code.driver.page.frameLocator('iframe').frameLocator(INNER_FRAME);

constructor(private code: Code, private quickinput: QuickInput, private quickaccess: QuickAccess, private notebook: Notebook) { }

Expand Down Expand Up @@ -75,40 +76,27 @@ export class PositronNotebooks {
}

async addCodeToFirstCell(code: string) {
await this.code.driver.getLocator(CELL_LINE).first().click();
await this.code.driver.page.locator(CELL_LINE).first().click();
await this.notebook.waitForTypeInEditor(code);
await this.notebook.waitForActiveCellEditorContents(code);
}

async executeCodeInCell() {
await this.quickaccess.runCommand(EXECUTE_CELL_COMMAND);
await expect(this.code.driver.page.locator(EXECUTE_CELL_SPINNER)).not.toBeVisible({ timeout: 30000 });
}

async getPythonCellOutput(): Promise<string> {
// basic CSS selection doesn't support frames (or nested frames)
const notebookFrame = this.code.driver.getFrame(OUTER_FRAME).frameLocator(INNER_FRAME);
const outputLocator = notebookFrame.locator(PYTHON_OUTPUT);
const outputText = await outputLocator.textContent();
return outputText!;
}

async getRCellOutput(): Promise<string> {
// basic CSS selection doesn't support frames (or nested frames)
const notebookFrame = this.code.driver.getFrame(OUTER_FRAME).frameLocator(INNER_FRAME);
const outputLocator = notebookFrame.locator(R_OUTPUT).nth(0);
const outputText = await outputLocator.textContent();
return outputText!;
async assertCellOutput(text: string): Promise<void> {
await expect(this.frameLocator.getByText(text)).toBeVisible();
}

async closeNotebookWithoutSaving() {
await this.quickaccess.runCommand(REVERT_AND_CLOSE);
}

async getMarkdownText(tag: string): Promise<string> {
// basic CSS selection doesn't support frames (or nested frames)
const frame = this.code.driver.getFrame(OUTER_FRAME).frameLocator(INNER_FRAME);
const element = frame.locator(`${MARKDOWN_TEXT} ${tag}`);
const text = await element.textContent();
return text!;
async assertMarkdownText(tag: string, expectedText: string): Promise<void> {
const markdownLocator = this.frameLocator.locator(`${MARKDOWN_TEXT} ${tag}`);
await expect(markdownLocator).toBeVisible();
await expect(markdownLocator).toHaveText(expectedText);
}
}
69 changes: 29 additions & 40 deletions test/smoke/src/areas/positron/notebook/notebookCreate.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,92 +3,81 @@
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
*--------------------------------------------------------------------------------------------*/


import { expect } from '@playwright/test';
import { Application, PositronPythonFixtures, PositronRFixtures } from '../../../../../automation';
import { Application, PositronNotebooks, PositronPythonFixtures, PositronRFixtures } from '../../../../../automation';
import { setupAndStartApp } from '../../../test-runner/test-hooks';

describe('Notebooks #pr #web #win', () => {
setupAndStartApp();

describe('Python Notebooks', () => {
describe('R Notebooks', () => {
let app: Application;
let notebooks: PositronNotebooks;

before(async function () {
app = this.app as Application;
await PositronPythonFixtures.SetupFixtures(app);
await app.workbench.positronLayouts.enterLayout('notebook');
notebooks = app.workbench.positronNotebooks;
await PositronRFixtures.SetupFixtures(this.app as Application);
});

beforeEach(async function () {
await app.workbench.positronNotebooks.createNewNotebook();
await app.workbench.positronNotebooks.selectInterpreter('Python Environments', process.env.POSITRON_PY_VER_SEL!);
await notebooks.createNewNotebook();
await notebooks.selectInterpreter('R Environments', process.env.POSITRON_R_VER_SEL!);
});

afterEach(async function () {
await app.workbench.positronNotebooks.closeNotebookWithoutSaving();
await notebooks.closeNotebookWithoutSaving();
});

it('Python - Basic notebook creation and execution (code) [C628631]', async function () {
await expect(async () => {
await app.workbench.positronNotebooks.addCodeToFirstCell('eval("8**2")');
await app.workbench.positronNotebooks.executeCodeInCell();

expect(await app.workbench.positronNotebooks.getPythonCellOutput()).toBe('64');
}).toPass({ timeout: 120000 });
it('R - Basic notebook creation and execution (code) [C628629]', async function () {
await notebooks.addCodeToFirstCell('eval(parse(text="8**2"))');
await notebooks.executeCodeInCell();
await notebooks.assertCellOutput('[1] 64');
});

it('Python - Basic notebook creation and execution (markdown) [C628632]', async function () {
it('R - Basic notebook creation and execution (markdown) [C628630]', async function () {
const randomText = Math.random().toString(36).substring(7);

await app.workbench.notebook.insertNotebookCell('markdown');
await app.workbench.notebook.waitForTypeInEditor(`## ${randomText} `);
await app.workbench.notebook.stopEditingCell();

expect(await app.workbench.positronNotebooks.getMarkdownText(`h2 >> text="${randomText}"`)).toBe(randomText);
await notebooks.assertMarkdownText('h2', randomText);
});
});
});

describe('Notebooks #pr #web #win', () => {
setupAndStartApp();

describe('R Notebooks', () => {
describe('Python Notebooks', () => {
let app: Application;
let notebooks: PositronNotebooks;

before(async function () {
app = this.app as Application;
await PositronRFixtures.SetupFixtures(this.app as Application);
notebooks = app.workbench.positronNotebooks;
await PositronPythonFixtures.SetupFixtures(app);
});

beforeEach(async function () {
await app.workbench.positronNotebooks.createNewNotebook();
await app.workbench.positronNotebooks.selectInterpreter('R Environments', process.env.POSITRON_R_VER_SEL!);
await notebooks.createNewNotebook();
await notebooks.selectInterpreter('Python Environments', process.env.POSITRON_PY_VER_SEL!);
});

afterEach(async function () {
await app.workbench.positronNotebooks.closeNotebookWithoutSaving();
await notebooks.closeNotebookWithoutSaving();
});

it('R - Basic notebook creation and execution (code) [C628629]', async function () {
await app.workbench.positronNotebooks.addCodeToFirstCell('eval(parse(text="8**2"))');

await expect(async () => {
await app.workbench.positronNotebooks.executeCodeInCell();
expect(await app.workbench.positronNotebooks.getRCellOutput()).toBe('[1] 64');
}).toPass({ timeout: 60000 });

it('Python - Basic notebook creation and execution (code) [C628631]', async function () {
await notebooks.addCodeToFirstCell('eval("8**2")');
await notebooks.executeCodeInCell();
await notebooks.assertCellOutput('64');
});

it('R - Basic notebook creation and execution (markdown) [C628630]', async function () {
it('Python - Basic notebook creation and execution (markdown) [C628632]', async function () {
const randomText = Math.random().toString(36).substring(7);

await app.workbench.notebook.insertNotebookCell('markdown');
await app.workbench.notebook.waitForTypeInEditor(`## ${randomText} `);
await app.workbench.notebook.stopEditingCell();

expect(await app.workbench.positronNotebooks.getMarkdownText(`h2 >> text="${randomText}"`)).toBe(randomText);

await notebooks.assertMarkdownText('h2', randomText);
});
});


});

0 comments on commit e58dd99

Please sign in to comment.