Skip to content

Commit d31db5a

Browse files
authored
e2e: more flake fixes (#10964)
### Summary * Addressing 3 tests that are flaky: * `search.test.ts` Removed final menu collapse step. A tooltip frequently blocks the collapse button, causing intermittent failures. This step is not essential to the test. * `default-python-interpreter.test.ts` Added wait for session initialization before asserting metadata. Previously caused reload loops when metadata was accessed before the session was ready. * `notebook-working-directory.test` Added retry logic for cell execution. Cells occasionally hang for 30s (expected execution time is <1s). Test now stops execution and retries on timeout. * Skipping the reticulate tests * Fixing a timing issue for chromium when selecting interpreters ### QA Notes @:interpreter @:web @:positron-notebooks <!-- Add additional information for QA on how to validate the change, paying special attention to the level of risk, adjacent areas that could be affected by the change, and any important contextual information not present in the linked issues. -->
1 parent 327bd6f commit d31db5a

15 files changed

+96
-83
lines changed

test/e2e/fixtures/test-setup/settings.fixtures.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import * as playwright from '@playwright/test';
76
import { Application } from '../../infra';
87

98
export function SettingsFixture(app: Application) {
@@ -19,10 +18,7 @@ export function SettingsFixture(app: Application) {
1918
await settings.set(newSettings, { keepOpen });
2019

2120
if (reload === true || (reload === 'web' && app.web === true)) {
22-
await app.workbench.hotKeys.reloadWindow();
23-
// wait for the reload to complete
24-
await app.code.driver.page.waitForTimeout(3000);
25-
await playwright.expect(app.code.driver.page.locator('.monaco-workbench')).toBeVisible();
21+
await app.workbench.hotKeys.reloadWindow(true);
2622
}
2723
if (waitMs) {
2824
await app.code.driver.page.waitForTimeout(waitMs); // wait for settings to take effect

test/e2e/infra/workbench.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ export class Workbench {
120120
this.sessions = new Sessions(code, this.quickaccess, this.quickInput, this.console, this.contextMenu, this.modals);
121121
this.notebooks = new Notebooks(code, this.quickInput, this.quickaccess, this.hotKeys);
122122
this.notebooksVscode = new VsCodeNotebooks(code, this.quickInput, this.quickaccess, this.hotKeys);
123-
this.notebooksPositron = new PositronNotebooks(code, this.quickInput, this.quickaccess, this.hotKeys, this.contextMenu);
123+
this.notebooksPositron = new PositronNotebooks(code, this.quickInput, this.quickaccess, this.hotKeys, this.contextMenu, this.sessions);
124124
this.welcome = new Welcome(code);
125125
this.terminal = new Terminal(code, this.quickaccess, this.clipboard);
126126
this.viewer = new Viewer(code);

test/e2e/pages/notebooks.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export class Notebooks {
3737
frameLocator: FrameLocator;
3838
notebookProgressBar: Locator;
3939
cellIndex: (num?: number) => Locator;
40+
interruptButton: Locator;
4041

4142
constructor(code: Code, quickinput: QuickInput, quickaccess: QuickAccess, hotKeys: HotKeys) {
4243
this.code = code;
@@ -49,6 +50,7 @@ export class Notebooks {
4950
this.frameLocator = this.code.driver.page.frameLocator(OUTER_FRAME).frameLocator(INNER_FRAME);
5051
this.notebookProgressBar = this.code.driver.page.locator('[id="workbench\\.parts\\.editor"]').getByRole('progressbar');
5152
this.cellIndex = (num = 0) => this.code.driver.page.locator('.cell-inner-container > .cell').nth(num);
53+
this.interruptButton = this.code.driver.page.getByRole('button', { name: 'Interrupt' });
5254
}
5355

5456
async selectInterpreter(
@@ -162,14 +164,18 @@ export class Notebooks {
162164
await expect(markdownLocator).toHaveText(expectedText);
163165
}
164166

165-
async runAllCells(timeout: number = 30000) {
167+
async runAllCells({ timeout = 15000, throwError = false } = {}): Promise<void> {
166168
await test.step('Run all cells', async () => {
167169
await this.code.driver.page.getByLabel('Run All').click();
168170
const stopExecutionLocator = this.code.driver.page.locator('a').filter({ hasText: /Stop Execution|Interrupt/ });
169171
try {
170-
await expect(stopExecutionLocator).toBeVisible();
172+
await expect(stopExecutionLocator).toBeVisible({ timeout: 5000 });
171173
await expect(stopExecutionLocator).not.toBeVisible({ timeout });
172-
} catch { } // can be normal with very fast execution
174+
} catch {
175+
if (throwError) {
176+
throw new Error('Timeout waiting for all cells to finish executing');
177+
}
178+
} // can be normal with very fast execution
173179
});
174180
}
175181

test/e2e/pages/notebooksPositron.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { QuickAccess } from './quickaccess';
1010
import test, { expect, Locator } from '@playwright/test';
1111
import { HotKeys } from './hotKeys.js';
1212
import { ContextMenu, MenuItemState } from './dialog-contextMenu.js';
13-
import { ACTIVE_STATUS_ICON, DISCONNECTED_STATUS_ICON, IDLE_STATUS_ICON, SessionState } from './sessions.js';
13+
import { ACTIVE_STATUS_ICON, DISCONNECTED_STATUS_ICON, IDLE_STATUS_ICON, Sessions, SessionState } from './sessions.js';
1414
import { basename, relative } from 'path';
1515

1616
const DEFAULT_TIMEOUT = 10000;
@@ -53,7 +53,7 @@ export class PositronNotebooks extends Notebooks {
5353
viewMarkdown = this.code.driver.page.getByRole('button', { name: 'View markdown' });
5454
expandMarkdownEditor = this.code.driver.page.getByRole('button', { name: 'Open markdown editor' });
5555

56-
constructor(code: Code, quickinput: QuickInput, quickaccess: QuickAccess, hotKeys: HotKeys, private contextMenu: ContextMenu) {
56+
constructor(code: Code, quickinput: QuickInput, quickaccess: QuickAccess, hotKeys: HotKeys, private contextMenu: ContextMenu, private sessions: Sessions) {
5757
super(code, quickinput, quickaccess, hotKeys);
5858
this.kernel = new Kernel(this.code, this, this.contextMenu, hotKeys, quickinput);
5959
}
@@ -175,6 +175,7 @@ export class PositronNotebooks extends Notebooks {
175175
'workbench.editorAssociations': { '*.ipynb': 'workbench.editor.positronNotebook' }
176176
};
177177
await settings.set(config, { reload: 'web' });
178+
await this.sessions.expectNoStartUpMessaging();
178179
}
179180

180181
/**

test/e2e/pages/sessions.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ export class Sessions {
451451
// Wait until the desired runtime appears in the list and select it.
452452
// We need to click instead of using 'enter' because the Python select interpreter command
453453
// may include additional items above the desired interpreter string.
454-
await this.quickinput.selectQuickInputElementContaining(`${language} ${version}`, { timeout: 5000 });
454+
await this.quickinput.selectQuickInputElementContaining(`${language} ${version}`);
455455
await this.quickinput.waitForQuickInputClosed();
456456

457457
// Move mouse to prevent tooltip hover

test/e2e/tests/interpreters/default-python-interpreter.test.ts

Lines changed: 25 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
import { expect } from '@playwright/test';
77
import { test, tags } from '../_test.setup';
88
import { deletePositronHistoryFiles } from './helpers/default-interpreters.js';
9+
import { buildPythonPath } from './helpers/include-excludes.js';
10+
import path from 'path';
911

1012
test.use({
1113
suiteId: __filename
@@ -17,50 +19,44 @@ test.describe('Default Interpreters - Python', {
1719
}, () => {
1820

1921
test.beforeAll(async function ({ settings }) {
20-
21-
await settings.remove(['interpreters.startupBehavior']);
2222
await settings.set({ 'interpreters.startupBehavior': 'always' });
23-
2423
await deletePositronHistoryFiles();
2524

26-
// local debugging sample:
27-
// const homeDir = process.env.HOME || '';
28-
// await settings.set({'python.defaultInterpreterPath': `${path.join(homeDir, '.pyenv/versions/3.13.0/bin/python')}`}, { reload: true });
29-
30-
const pythonPath = '/root/scratch/python-env/bin/python';
25+
// Build environment-aware path for default interpreter
26+
// Note: CI uses hidden Python in /root/scratch, local uses pyenv version
27+
const pythonVersion = process.env.POSITRON_PY_VER_SEL || '3.10.12';
28+
const pythonPath = process.env.CI
29+
? `${buildPythonPath('include')}/bin/python` // Hidden Python (POSITRON_HIDDEN_PY)
30+
: path.join(process.env.HOME || '', `.pyenv/versions/${pythonVersion}/bin/python`);
3131

32+
// First reload: "Apply these settings"
3233
await settings.set({ 'python.defaultInterpreterPath': pythonPath }, { reload: true });
33-
3434
});
3535

3636
test.afterAll(async function ({ cleanup }) {
37-
3837
await cleanup.discardAllChanges();
39-
4038
});
4139

42-
test('Python - Add a default interpreter (Conda)', async function ({ runCommand, sessions }) {
43-
44-
await runCommand('workbench.action.reloadWindow');
45-
46-
await expect(async () => {
40+
test('Python - Add a default interpreter (Conda)', async function ({ hotKeys, sessions }) {
41+
// Second reload: "Now actually start the interpreter with these settings"
42+
await hotKeys.reloadWindow(true);
4743

48-
try {
49-
const { name, path } = await sessions.getMetadata();
44+
// Get version from appropriate env var (hidden Python in CI, regular in local)
45+
const pythonVersion = process.env.CI
46+
? (process.env.POSITRON_HIDDEN_PY || '3.12.10').split(' ')[0] // Extract "3.12.10" from "3.12.10 (Conda)"
47+
: process.env.POSITRON_PY_VER_SEL || '3.10.12';
5048

51-
// Local debugging sample:
52-
// expect(name).toMatch(/Python 3\.13\.0/);
53-
// expect(path).toMatch(/.pyenv\/versions\/3.13.0\/bin\/python/);
49+
// Match version with optional text after (e.g., "Python 3.12.10 (Conda)")
50+
const versionRegex = new RegExp(`Python ${pythonVersion.replace(/\./g, '\\.')}(\\s.*)?`);
5451

55-
// hidden CI interpreter:
56-
expect(name).toMatch(/Python 3\.12\.10/);
57-
expect(path).toMatch(/python-env\/bin\/python/);
52+
// Build environment-aware path regex
53+
const pathRegex = process.env.CI
54+
? /python-env\/bin\/python/
55+
: new RegExp(`~?\\.pyenv/versions/${pythonVersion.replace(/\./g, '\\.')}/bin/python`);
5856

59-
} catch (error) {
60-
await runCommand('workbench.action.reloadWindow');
61-
throw error;
62-
}
57+
const { name, path } = await sessions.getMetadata();
6358

64-
}).toPass({ timeout: 60000 });
59+
expect(name).toMatch(versionRegex);
60+
expect(path).toMatch(pathRegex);
6561
});
6662
});

test/e2e/tests/notebook/notebook-large-python.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ test.describe('Large Python Notebook', {
2626
// open the large Python notebook and run all cells
2727
await openDataFile(join('workspaces', 'large_py_notebook', 'spotify.ipynb'));
2828
await notebooks.selectInterpreter('Python');
29-
await notebooks.runAllCells(120000);
29+
await notebooks.runAllCells({ timeout: 12000 });
3030

3131
// scroll through the notebook and count unique plot outputs
3232
await layouts.enterLayout('notebook');

test/e2e/tests/notebook/notebook-large-r.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ test.describe('Large R Notebook', {
2525
// open the large R notebook and run all cells
2626
await openDataFile(join('workspaces', 'large_r_notebook', 'spotify.ipynb'));
2727
await notebooks.selectInterpreter('R');
28-
await notebooks.runAllCells(120000);
28+
await notebooks.runAllCells({ timeout: 12000 });
2929

3030
// scroll through the notebook and count unique plot outputs
3131
await layouts.enterLayout('notebook');

test/e2e/tests/notebook/notebook-working-directory.test.ts

Lines changed: 46 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@
66
import path from 'path';
77
import fs from 'fs';
88
import os from 'os';
9-
import { test, tags } from '../_test.setup';
9+
import { test, tags, expect } from '../_test.setup';
1010
import { Notebooks } from '../../pages/notebooks.js';
1111

1212
test.use({
1313
suiteId: __filename
1414
});
1515

16-
test.describe.skip('Notebook Working Directory Configuration', {
16+
test.describe('Notebook Working Directory Configuration', {
1717
tag: [tags.WIN, tags.WEB, tags.NOTEBOOKS]
1818
}, () => {
1919

@@ -25,53 +25,65 @@ test.describe.skip('Notebook Working Directory Configuration', {
2525
await app.workbench.notebooks.closeNotebookWithoutSaving();
2626
});
2727

28-
test('Default working directory is the notebook parent', async function ({ app, settings }) {
29-
await settings.clear();
30-
await verifyWorkingDirectoryEndsWith(app.workbench.notebooks, 'working-directory-notebook');
31-
});
28+
const testCases = [
29+
{
30+
title: 'Default working directory is the notebook parent',
31+
workingDirectory: null, // null = use default (clear settings)
32+
expectedEnd: 'working-directory-notebook',
33+
},
34+
{
35+
title: 'workspaceFolder works',
36+
workingDirectory: '${workspaceFolder}',
37+
expectedEnd: 'qa-example-content',
38+
},
39+
{
40+
title: 'fileDirname works',
41+
workingDirectory: '${fileDirname}',
42+
expectedEnd: 'working-directory-notebook',
43+
},
44+
{
45+
title: 'Paths that do not exist result in the default notebook parent',
46+
workingDirectory: '/does/not/exist',
47+
expectedEnd: 'working-directory-notebook',
48+
},
49+
{
50+
title: 'Bad variables result in the default notebook parent',
51+
workingDirectory: '${asdasd}',
52+
expectedEnd: 'working-directory-notebook',
53+
},
54+
];
3255

33-
test('workspaceFolder works', async function ({ app, settings }) {
34-
await settings.set({
35-
'notebook.workingDirectory': '${workspaceFolder}'
36-
}, { reload: 'web' });
37-
await verifyWorkingDirectoryEndsWith(app.workbench.notebooks, 'qa-example-content');
38-
});
56+
testCases.forEach(({ title, workingDirectory, expectedEnd }) => {
57+
test(title, async function ({ app, settings }) {
58+
workingDirectory === null
59+
? await settings.clear()
60+
: await settings.set({ 'notebook.workingDirectory': workingDirectory }, { reload: 'web' });
3961

40-
test('fileDirname works', async function ({ app, settings }) {
41-
await settings.set({
42-
'notebook.workingDirectory': '${fileDirname}'
43-
}, { reload: 'web' });
44-
await verifyWorkingDirectoryEndsWith(app.workbench.notebooks, 'working-directory-notebook');
62+
await verifyWorkingDirectoryEndsWith(app.workbench.notebooks, expectedEnd);
63+
});
4564
});
4665

4766
test('A hardcoded path works', async function ({ app, settings }) {
4867
// Make a temp dir
4968
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'notebook-test'));
50-
5169
await settings.set({
5270
'notebook.workingDirectory': tempDir
5371
}, { reload: 'web' });
54-
await verifyWorkingDirectoryEndsWith(app.workbench.notebooks, path.basename(tempDir));
55-
});
56-
57-
test('Paths that do not exist result in the default notebook parent', async function ({ app, settings }) {
58-
await settings.set({
59-
'notebook.workingDirectory': '/does/not/exist'
60-
}, { reload: 'web' });
61-
await verifyWorkingDirectoryEndsWith(app.workbench.notebooks, 'working-directory-notebook');
62-
});
6372

64-
test('Bad variables result in the default notebook parent', async function ({ app, settings }) {
65-
await settings.set({
66-
'notebook.workingDirectory': '${asdasd}'
67-
}, { reload: 'web' });
68-
await verifyWorkingDirectoryEndsWith(app.workbench.notebooks, 'working-directory-notebook');
73+
await verifyWorkingDirectoryEndsWith(app.workbench.notebooks, path.basename(tempDir));
6974
});
7075
});
7176

7277
async function verifyWorkingDirectoryEndsWith(notebooks: Notebooks, expectedEnd: string) {
7378
await notebooks.openNotebook('working-directory.ipynb');
7479
await notebooks.selectInterpreter('Python');
75-
await notebooks.runAllCells();
76-
await notebooks.assertCellOutput(new RegExp(`^'.*${expectedEnd}'$`));
80+
await expect(async () => {
81+
try {
82+
await notebooks.runAllCells({ timeout: 10000, throwError: true });
83+
await notebooks.assertCellOutput(new RegExp(`^'.*${expectedEnd}'$`));
84+
} catch (e) {
85+
await notebooks.interruptButton.click({ timeout: 3000 }).catch(() => { });
86+
throw e;
87+
}
88+
}, 'Expect working directory to end with: ' + expectedEnd).toPass({ timeout: 30000 });
7789
}

test/e2e/tests/reticulate/reticulate-multiple.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ test.use({
1414
// RETICULATE_PYTHON
1515
// to the installed python path
1616

17-
test.describe('Reticulate', {
17+
test.describe.skip('Reticulate', {
1818
tag: [tags.RETICULATE, tags.WEB, tags.ARK, tags.SOFT_FAIL],
1919
}, () => {
2020
test.beforeAll(async function ({ app, settings }) {

0 commit comments

Comments
 (0)