e2e: improve stability of reticulate tests (#10887)

midleman · web-flow · commit 62f21bdae6c3 · 2025-12-04T13:44:30.000-05:00
### Summary
This PR intends to improve the reliability and maintainability of
reticulate e2e tests... at the end of the day there still is an issue so
we will still be seeing some flakes/fails. 😢

### QA Notes

@:reticulate @:web
diff --git a/.github/workflows/test-e2e-ubuntu.yml b/.github/workflows/test-e2e-ubuntu.yml
@@ -86,7 +86,7 @@ on:
         type: number
       skip_extension_test:
         required: false
-        description: "Whether to skip the bootstrap extensions test."
+        description: "Skip the bootstrap extensions test."
         type: boolean
         default: true
       workers:
diff --git a/.github/workflows/test-e2e-windows.yml b/.github/workflows/test-e2e-windows.yml
@@ -67,7 +67,7 @@ on:
         type: number
       skip_extension_test:
         required: false
-        description: "Whether to skip the bootstrap extensions test."
+        description: "Skip the bootstrap extensions test."
         type: boolean
         default: true
 
diff --git a/test/e2e/infra/workbench.ts b/test/e2e/infra/workbench.ts
@@ -117,7 +117,7 @@ export class Workbench {
 		this.console = new Console(code, this.quickInput, this.quickaccess, this.hotKeys, this.contextMenu);
 		this.modals = new Modals(code, this.toasts, this.console);
 		this.clipboard = new Clipboard(code, this.hotKeys);
-		this.sessions = new Sessions(code, this.quickaccess, this.quickInput, this.console, this.contextMenu);
+		this.sessions = new Sessions(code, this.quickaccess, this.quickInput, this.console, this.contextMenu, this.modals);
 		this.notebooks = new Notebooks(code, this.quickInput, this.quickaccess, this.hotKeys);
 		this.notebooksVscode = new VsCodeNotebooks(code, this.quickInput, this.quickaccess, this.hotKeys);
 		this.notebooksPositron = new PositronNotebooks(code, this.quickInput, this.quickaccess, this.hotKeys, this.contextMenu);
diff --git a/test/e2e/pages/console.ts b/test/e2e/pages/console.ts
@@ -12,7 +12,7 @@ import { ContextMenu, MenuItemState } from './dialog-contextMenu.js';
 import { QuickAccess } from './quickaccess.js';
 
 const CONSOLE_INPUT = '.console-input';
-const ACTIVE_CONSOLE_INSTANCE = '.console-instance[style*="z-index: auto"]';
+export const ACTIVE_CONSOLE_INSTANCE = '.console-instance[style*="z-index: auto"]';
 const MAXIMIZE_CONSOLE = '.bottom .codicon-positron-maximize-panel';
 const HISTORY_COMPLETION_ITEM = '.history-completion-item';
 const EMPTY_CONSOLE = '.positron-console .empty-console';
@@ -278,6 +278,10 @@ export class Console {
 		await this.code.driver.page.locator(MAXIMIZE_CONSOLE).click();
 	}
 
+	async sendInterrupt() {
+		await this.hotKeys.sendInterrupt();
+	}
+
 	async pasteCodeToConsole(code: string, sendEnterKey = false) {
 		await test.step(`Paste code to console: ${code}`, async () => {
 			const consoleInput = this.activeConsole.locator(CONSOLE_INPUT);
diff --git a/test/e2e/pages/dialog-modals.ts b/test/e2e/pages/dialog-modals.ts
@@ -43,7 +43,7 @@ export class Modals {
 		try {
 			this.code.logger.log('Checking for modal dialog box');
 			// fail fast if the modal is not present
-			await this.expectToBeVisible();
+			await this.expectToBeVisible(undefined, { timeout: 5000 });
 			await this.clickButton('Install');
 			this.code.logger.log('Installing ipykernel');
 			await this.toasts.expectToBeVisible();
@@ -89,11 +89,11 @@ export class Modals {
 		});
 	}
 
-	async expectToBeVisible(title?: string) {
+	async expectToBeVisible(title?: string, { timeout = 30000 } = {}) {
 		await test.step(`Verify modal dialog box is visible${title ? ` : ${title}` : ''}`, async () => {
-			await expect(this.modalBox).toBeVisible({ timeout: 30000 });
+			await expect(this.modalBox).toBeVisible({ timeout });
 			if (title) {
-				await expect(this.modalTitle).toHaveText(title, { timeout: 30000 });
+				await expect(this.modalTitle).toHaveText(title, { timeout });
 			}
 		});
 	}
diff --git a/test/e2e/pages/dialog-toasts.ts b/test/e2e/pages/dialog-toasts.ts
@@ -16,12 +16,16 @@ export class Toasts {
 
 	// --- Actions ---
 
-	async waitForAppear(timeout = 20000) {
-		await this.toastNotification.waitFor({ state: 'attached', timeout });
+	async waitForAppear(title?: string | RegExp, { timeout = 20000 } = {}) {
+		title
+			? await this.toastNotification.getByText(title).waitFor({ state: 'attached', timeout })
+			: await this.toastNotification.waitFor({ state: 'attached', timeout });
 	}
 
-	async waitForDisappear(timeout = 20000) {
-		await this.toastNotification.waitFor({ state: 'detached', timeout });
+	async waitForDisappear(title?: string | RegExp, { timeout = 20000 } = {}) {
+		title
+			? await this.toastNotification.getByText(title).waitFor({ state: 'detached', timeout })
+			: await this.toastNotification.waitFor({ state: 'detached', timeout });
 	}
 
 	async clickButton(button: string) {
diff --git a/test/e2e/pages/hotKeys.ts b/test/e2e/pages/hotKeys.ts
@@ -144,6 +144,10 @@ export class HotKeys {
 		return this.pressHotKeys('Cmd+J O', 'Execute code in console');
 	}
 
+	public async sendInterrupt() {
+		await this.pressHotKeys('Cmd+C', 'Send interrupt to console');
+	}
+
 	// ----------------------
 	// --- Layout Views ---
 	// ----------------------
diff --git a/test/e2e/pages/quickInput.ts b/test/e2e/pages/quickInput.ts
@@ -40,7 +40,7 @@ export class QuickInput {
 		});
 	}
 
-	async waitForQuickInputOpened({ timeout = 10000 }: { timeout?: number } = {}): Promise<void> {
+	async waitForQuickInputOpened({ timeout = 3000 }: { timeout?: number } = {}): Promise<void> {
 		await expect(this.code.driver.page.locator(QuickInput.QUICK_INPUT_INPUT)).toBeVisible({ timeout });
 	}
 
diff --git a/test/e2e/pages/quickaccess.ts b/test/e2e/pages/quickaccess.ts
@@ -139,7 +139,7 @@ export class QuickAccess {
 
 			// Await for quick input widget opened
 			try {
-				await this.quickInput.waitForQuickInputOpened();
+				await this.quickInput.waitForQuickInputOpened({ timeout: 3000 });
 			} catch (err) {
 				await this.code.driver.page.keyboard.press('Escape');
 				throw err;
diff --git a/test/e2e/pages/sessions.ts b/test/e2e/pages/sessions.ts
@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import test, { expect, Locator, Page } from '@playwright/test';
-import { Code, QuickAccess, Console, ContextMenu } from '../infra';
+import { Code, QuickAccess, Console, ContextMenu, Modals } from '../infra';
 import { QuickInput } from './quickInput';
 
 // Lazy getters for environment variables - these will be evaluated when accessed, not at module load time
@@ -46,7 +46,7 @@ export class Sessions {
 	private consoleInstance = (sessionId: string) => this.page.getByTestId(`console-${sessionId}`);
 	private outputChannel = this.page.getByRole('combobox');
 
-	constructor(private code: Code, private quickaccess: QuickAccess, private quickinput: QuickInput, private console: Console, private contextMenu: ContextMenu) { }
+	constructor(private code: Code, private quickaccess: QuickAccess, private quickinput: QuickInput, private console: Console, private contextMenu: ContextMenu, private modals: Modals) { }
 
 	// -- Actions --
 
@@ -180,8 +180,8 @@ export class Sessions {
 	 * @param options.waitForIdle - wait for the session to display as "idle" (ready)
 	 * @param options.clearConsole - clear the console before restarting
 	 */
-	async restart(sessionIdOrName: string, options?: { waitForIdle?: boolean; clearConsole?: boolean }): Promise<void> {
-		const { waitForIdle = true, clearConsole = true } = options || {};
+	async restart(sessionIdOrName: string, options?: { waitForIdle?: boolean; clearConsole?: boolean, clickModalButton?: string }): Promise<void> {
+		const { waitForIdle = true, clearConsole = true, clickModalButton = '' } = options || {};
 
 		await test.step(`Restart session: ${sessionIdOrName}`, async () => {
 			await this.console.focus();
@@ -197,6 +197,10 @@ export class Sessions {
 			await this.console.restartButton.click();
 			await this.page.mouse.move(0, 0);
 
+			if (clickModalButton) {
+				await this.modals.clickButton(clickModalButton);
+			}
+
 			if (waitForIdle) {
 				await expect(this.page.getByText('restarting.')).not.toBeVisible({ timeout: 90000 });
 				await expect(this.page.locator('.console-instance[style*="z-index: auto"]').getByText('restarted.')).toBeVisible({ timeout: 90000 });
@@ -878,8 +882,7 @@ export class Sessions {
 		await test.step(`Verify runtime is selected: ${runtimeName}`, async () => {
 			const normalizedRuntimeName = runtimeName.replace(/-\s\d+$/, '').trim();
 			await expect(this.sessionPicker).toHaveText(normalizedRuntimeName, { timeout });
-		}
-		);
+		});
 	}
 
 	/**
@@ -945,10 +948,10 @@ export class Sessions {
 	/**
 	 * Verify: all sessions are "ready" (idle or disconnected)
 	 */
-	async expectAllSessionsToBeReady() {
+	async expectAllSessionsToBeReady({ timeout = 15000 }: { timeout?: number } = {}) {
 		await test.step('Expect all sessions to be ready', async () => {
 			await this.expectNoStartUpMessaging();
-			await expect(this.activeStatusIcon).toHaveCount(0);
+			await expect(this.activeStatusIcon).toHaveCount(0, { timeout });
 		});
 	}
 
diff --git a/test/e2e/pages/variables.ts b/test/e2e/pages/variables.ts
@@ -165,7 +165,7 @@ export class Variables {
 		await this.contextMenu.triggerAndClick({
 			menuTrigger: this.code.driver.page.locator('.positron-variables .positron-action-bar').nth(1).locator('button'),
 			menuItemLabel: name,
-		})
+		});
 	}
 
 	async selectVariablesGroup(name: string) {
@@ -220,15 +220,17 @@ export class Variables {
 	 * @param value the expected value of the variable
 	 * @param timeout (optional) timeout in milliseconds for visibility (default 15000)
 	 */
-	async expectVariableToBe(variableName: string, value: string, timeout: number = 15000) {
+	async expectVariableToBe(variableName: string, value: string | RegExp, timeout: number = 15000) {
 		await test.step(`Verify variable: ${variableName} with value: ${value}`, async () => {
 			await this.focusVariablesView();
-			const row = this.code.driver.page
-				.locator('.variables-instance[style*="z-index: 1"] .variable-item')
-				.filter({ hasText: variableName });
-
-			await expect(row).toBeVisible({ timeout });
-			await expect(row.locator('.details-column .value')).toHaveText(value);
+			const variableRow = this.code.driver.page
+				.locator('.variables-instance[style*="z-index: 1"]')
+				.locator('.name-column')
+				.filter({ hasText: variableName })
+				.locator('..');
+
+			await expect(variableRow).toBeVisible({ timeout });
+			await expect(variableRow.locator('.details-column .value')).toHaveText(value, { timeout: 3000 });
 		});
 	}
 
diff --git a/test/e2e/tests/notebook/notebook-working-directory.test.ts b/test/e2e/tests/notebook/notebook-working-directory.test.ts
@@ -13,7 +13,7 @@ test.use({
 	suiteId: __filename
 });
 
-test.describe('Notebook Working Directory Configuration', {
+test.describe.skip('Notebook Working Directory Configuration', {
 	tag: [tags.WIN, tags.WEB, tags.NOTEBOOKS]
 }, () => {
 
diff --git a/test/e2e/tests/reticulate/helpers/verifyReticulateFunction.ts b/test/e2e/tests/reticulate/helpers/verifyReticulateFunction.ts
@@ -3,96 +3,57 @@
  *  Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { expect } from '@playwright/test';
 import { Application } from '../../../infra/index.js';
+import { expect } from '../../_test.setup.js';
+
+export const RETICULATE_SESSION = 'Python (reticulate)';
 
 export async function verifyReticulateFunctionality(
 	app: Application,
 	rSessionId: string,
 	pythonSessionId: string = 'Python (reticulate)',
-	value = '200',
-	value2 = '400',
-	value3 = '6'): Promise<void> {
-	// Verify that reticulate is installed
-	// Create a variable in Python and expect to be able to access it from R
-	await app.workbench.sessions.select(pythonSessionId);
-
-	await app.code.wait(2000); // give ipykernel time to startup
-
-	await app.workbench.console.pasteCodeToConsole(`x=${value}`, true);
-
-	await ensureVariablePresent(app, 'x', value);
-
-	await app.workbench.console.clearButton.click();
-
-	await app.workbench.sessions.select(rSessionId);
-
-	await app.code.wait(2000); // wait a little for python var to get to R
-
-	await app.workbench.console.pasteCodeToConsole('y<-reticulate::py$x', true);
-
-	await ensureVariablePresent(app, 'y', value);
-
-	await app.workbench.console.clearButton.click();
+	xValue = '200',
+	yValue = '400',
+	zValue = '6'): Promise<void> {
+	const { console, sessions, variables } = app.workbench;
 
-	await app.workbench.layouts.enterLayout('fullSizedAuxBar');
-
-	await ensureVariablePresent(app, 'y', value);
-
-	await app.workbench.layouts.enterLayout('stacked');
-
-	// Create a variable in R and expect to be able to access it from Python
-	await app.workbench.console.pasteCodeToConsole(`y <- ${value2}L`, true);
-
-	await ensureVariablePresent(app, 'y', value2);
-
-	// Executing reticulate::repl_python() should not start a new interpreter
-	// but should move focus to the reticulate interpreter
-	await app.workbench.console.pasteCodeToConsole(`reticulate::repl_python(input = "z = ${value3}")`, true);
+	// Create a variable x in Python session
+	await expect(async () => {
+		await console.executeCode('Python', `x = ${xValue}`);
+		await variables.expectVariableToBe('x', xValue, 2000);
+	}, 'Can create variable in Python session').toPass();
 
-	// Expect that focus changed to the reticulate console
+	// Switch to the R session and create an R variable `y` by accessing the Python
+	// variable `x` through reticulate.
 	await expect(async () => {
-		try {
-			await app.workbench.sessions.expectSessionPickerToBe(pythonSessionId);
-		} catch (e) {
-			await app.code.wait(1000); // a little time for session picker to be updated
-			throw e;
-		}
-	}).toPass({ timeout: 20000 });
+		await console.executeCode('R', 'y<-reticulate::py$x');
+		await variables.expectVariableToBe('y', xValue, 2000);
+	}, 'Can access Python variable x from R').toPass();
 
-	await app.workbench.console.pasteCodeToConsole('print(r.y)', true);
-	await ensureConsoleDataPresent(app, value2);
+	// Clear the console again and re-check to ensure the R-side variable persists.
+	await console.clearButton.click();
+	await variables.expectVariableToBe('y', xValue);
 
-	await app.workbench.console.pasteCodeToConsole('print(z)', true);
-	await ensureConsoleDataPresent(app, value3);
-}
+	// Verify able to overwrite the R variable `y` with an integer literal on the R side.
+	await expect(async () => {
+		await console.executeCode('R', `y <- ${yValue}L`);
+		await variables.expectVariableToBe('y', yValue, 2000);
+	}, 'Can overwrite the R variable').toPass();
 
-async function ensureVariablePresent(app: Application, variableName: string, value: string) {
+	// Verify executing reticulate::repl_python() moves focus to the reticulate session
+	await console.pasteCodeToConsole(`reticulate::repl_python(input = "z = ${zValue}")`, true);
+	await sessions.expectSessionPickerToBe(pythonSessionId, 20000);
+	await console.clearButton.click();
 
+	// Print the R variable r.y (should reflect the R-side value) and ensure it appears in the console
 	await expect(async () => {
-		try {
-			await app.workbench.layouts.enterLayout('fullSizedAuxBar');
-			const variablesMap = await app.workbench.variables.getFlatVariables();
-			expect(variablesMap.get(variableName)).toStrictEqual({ value: value, type: 'int' });
-			await app.workbench.layouts.enterLayout('stacked');
-		} catch (e) {
-			await app.workbench.layouts.enterLayout('stacked');
-			console.log('Resending enter key');
-			await app.workbench.console.sendEnterKey();
-			throw e;
-		}
-	}).toPass({ timeout: 10000 });
-}
-
-async function ensureConsoleDataPresent(app: Application, value: string) {
+		await console.executeCode('Python', 'print(r.y)');
+		await console.waitForConsoleContents(yValue, { timeout: 5000 });
+	}, 'Can print the R variable r.y from Python').toPass();
 
+	// Print the Python variable z (created via repl_python) and ensure it appears as well
 	await expect(async () => {
-		try {
-			await app.workbench.console.waitForConsoleContents(value);
-		} catch (e) {
-			console.log('Resending enter key');
-			await app.workbench.console.sendEnterKey();
-			throw e;
-		}
-	}).toPass({ timeout: 10000 });
+		await console.executeCode('Python', 'print(z)');
+		await console.waitForConsoleContents(zValue, { timeout: 5000 });
+	}, 'Can print the Python variable z from Python').toPass();
 }
diff --git a/test/e2e/tests/reticulate/reticulate-multiple.test.ts b/test/e2e/tests/reticulate/reticulate-multiple.test.ts
diff --git a/test/e2e/tests/reticulate/reticulate-repl-python.test.ts b/test/e2e/tests/reticulate/reticulate-repl-python.test.ts
diff --git a/test/e2e/tests/reticulate/reticulate-restart.test.ts b/test/e2e/tests/reticulate/reticulate-restart.test.ts
diff --git a/test/e2e/tests/reticulate/reticulate-stop-start.test.ts b/test/e2e/tests/reticulate/reticulate-stop-start.test.ts
diff --git a/test/e2e/tests/reticulate/reticulate-variables.test.ts b/test/e2e/tests/reticulate/reticulate-variables.test.ts

Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ export class QuickInput {`
`40`	`40`	`});`
`41`	`41`	`}`
`42`	`42`
`43`		`- async waitForQuickInputOpened({ timeout = 10000 }: { timeout?: number } = {}): Promise<void> {`
	`43`	`+ async waitForQuickInputOpened({ timeout = 3000 }: { timeout?: number } = {}): Promise<void> {`
`44`	`44`	`await expect(this.code.driver.page.locator(QuickInput.QUICK_INPUT_INPUT)).toBeVisible({ timeout });`
`45`	`45`	`}`
`46`	`46`