Skip to content

Commit 5404728

Browse files
authored
Await default interpreter startup before starting a new one (#4500)
Occasionally we see startup crashes in tests that have been difficult to debug. One theory as to why this is happening is that the test might be trying to start the test interpreter at the same time the default interpreter is still starting up. This PR adds code to wait for the default interpreter (or definitively no interpreter) to start up before initiating the process of starting a new one. ### QA Notes All smoke tests should pass.
1 parent 9159a65 commit 5404728

9 files changed

+29
-19
lines changed

test/automation/src/positron/fixtures/positronPythonFixtures.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@ export class PositronPythonFixtures {
1414

1515
constructor(private app: Application) { }
1616

17-
static async SetupFixtures(app: Application) {
17+
static async SetupFixtures(app: Application, skipReadinessCheck: boolean = false) {
1818
const fixtures = new PositronPythonFixtures(app);
19-
await fixtures.startPythonInterpreter();
19+
await fixtures.startPythonInterpreter(skipReadinessCheck);
2020
}
2121

22-
async startPythonInterpreter() {
22+
async startPythonInterpreter(skipReadinessCheck: boolean = false) {
2323

2424
const desiredPython = process.env.POSITRON_PY_VER_SEL;
2525
if (desiredPython === undefined) {
@@ -28,7 +28,7 @@ export class PositronPythonFixtures {
2828

2929
// We currently don't capture fixtures in the Playwright trace, so take a screenshot on failure
3030
try {
31-
await this.app.workbench.positronConsole.selectInterpreter(InterpreterType.Python, desiredPython);
31+
await this.app.workbench.positronConsole.selectInterpreter(InterpreterType.Python, desiredPython, skipReadinessCheck);
3232
await this.app.workbench.positronConsole.waitForReady('>>>', 2000);
3333
} catch (e) {
3434
this.app.code.driver.takeScreenshot('startPythonInterpreter');

test/automation/src/positron/fixtures/positronRFixtures.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@ export class PositronRFixtures {
1414

1515
constructor(private app: Application) { }
1616

17-
static async SetupFixtures(app: Application) {
17+
static async SetupFixtures(app: Application, skipReadinessCheck: boolean = false) {
1818
const fixtures = new PositronRFixtures(app);
19-
await fixtures.startRInterpreter();
19+
await fixtures.startRInterpreter(skipReadinessCheck);
2020
}
2121

22-
async startRInterpreter() {
22+
async startRInterpreter(skipReadinessCheck: boolean = false) {
2323

2424
const desiredR = process.env.POSITRON_R_VER_SEL;
2525
if (desiredR === undefined) {
@@ -29,7 +29,7 @@ export class PositronRFixtures {
2929

3030
// We currently don't capture fixtures in the Playwright trace, so take a screenshot on failure
3131
try {
32-
await this.app.workbench.positronConsole.selectInterpreter(InterpreterType.R, desiredR);
32+
await this.app.workbench.positronConsole.selectInterpreter(InterpreterType.R, desiredR, skipReadinessCheck);
3333
await this.app.workbench.positronConsole.waitForReady('>', 2000);
3434
} catch (e) {
3535
this.app.code.driver.takeScreenshot('startRInterpreter');

test/automation/src/positron/positronConsole.ts

+10-3
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,13 @@ export class PositronConsole {
4444
this.consoleRestartButton = new PositronBaseElement(CONSOLE_RESTART_BUTTON, this.code);
4545
}
4646

47-
async selectInterpreter(desiredInterpreterType: InterpreterType, desiredInterpreterString: string): Promise<IElement | undefined> {
47+
async selectInterpreter(desiredInterpreterType: InterpreterType, desiredInterpreterString: string, skipWait: boolean = false): Promise<IElement | undefined> {
48+
49+
// don't try to start a new interpreter if one is currently starting up
50+
if (!skipWait) {
51+
await this.waitForReadyOrNoInterpreter();
52+
}
53+
4854
let command: string;
4955
if (desiredInterpreterType === InterpreterType.Python) {
5056
command = 'python.setInterpreter';
@@ -73,7 +79,8 @@ export class PositronConsole {
7379
): Promise<InterpreterInfo | undefined> {
7480
const interpreterElem = await this.selectInterpreter(
7581
desiredInterpreterType,
76-
desiredInterpreter
82+
desiredInterpreter,
83+
true
7784
);
7885

7986
if (interpreterElem) {
@@ -144,7 +151,7 @@ export class PositronConsole {
144151
* @param retryCount The number of times to retry waiting for the console to be ready.
145152
* @throws An error if the console is not ready after the retry count.
146153
*/
147-
async waitForReadyOrNoInterpreter(retryCount: number = 200) {
154+
async waitForReadyOrNoInterpreter(retryCount: number = 800) {
148155
for (let i = 0; i < retryCount; i++) {
149156
// Check if the console is ready with Python.
150157
try {

test/automation/src/positron/positronEditor.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export class PositronEditor {
3232
* current-line div parent.
3333
*/
3434
async getCurrentLineTop(): Promise<number> {
35-
let retries = 10;
35+
let retries = 20;
3636
let topValue: number = NaN;
3737

3838
const currentLine = this.code.driver.getLocator(CURRENT_LINE);

test/automation/src/positron/positronVariables.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ interface FlatVariables {
1414
type: string;
1515
}
1616

17-
const VARIABLE_ITEMS = '.variables-instance .list .variable-item';
17+
const VARIABLE_ITEMS = '.variables-instance[style*="z-index: 1"] .list .variable-item';
1818
const VARIABLE_NAMES = 'name-column';
1919
const VARIABLE_DETAILS = 'details-column';
2020
const VARIABLES_NAME_COLUMN = '.variable-item .name-column';

test/smoke/src/areas/positron/apps/shiny.test.ts

+4-3
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export function setup(logger: Logger) {
3838

3939
const headerLocator = app.workbench.positronViewer.getViewerLocator('h1');
4040

41-
const headerText = await headerLocator.innerText();
41+
const headerText = await headerLocator.innerText({ timeout: 30000 });
4242

4343
expect(headerText).toBe('Restaurant tipping');
4444

@@ -53,7 +53,8 @@ export function setup(logger: Logger) {
5353

5454
describe('Shiny Application - R', () => {
5555
before(async function () {
56-
await PositronRFixtures.SetupFixtures(this.app as Application);
56+
// setup R but do not wait for a default interpreter to finish starting
57+
await PositronRFixtures.SetupFixtures(this.app as Application, true);
5758
});
5859

5960
it('R - Verify Basic Shiny App [C699100]', async function () {
@@ -68,7 +69,7 @@ runExample("01_hello")`;
6869

6970
const headerLocator = app.workbench.positronViewer.getViewerLocator('h1');
7071

71-
const headerText = await headerLocator.innerText();
72+
const headerText = await headerLocator.innerText({ timeout: 30000 });
7273

7374
expect(headerText).toBe('Hello Shiny!');
7475

test/smoke/src/areas/positron/console/consoleClipboard.test.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ export function setup(logger: Logger) {
7777

7878
describe('Console Clipboard - R', () => {
7979
before(async function () {
80-
await PositronRFixtures.SetupFixtures(this.app as Application);
80+
// setup R but do not wait for a default interpreter to finish starting
81+
await PositronRFixtures.SetupFixtures(this.app as Application, true);
8182
});
8283

8384
it('R - Copy from console & paste to console [C663725]', async function () {

test/smoke/src/areas/positron/console/consoleHistory.test.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ export function setup(logger: Logger) {
7474

7575
describe('Console History - R', () => {
7676
before(async function () {
77-
await PositronRFixtures.SetupFixtures(this.app as Application);
77+
// setup R but do not wait for a default interpreter to finish starting
78+
await PositronRFixtures.SetupFixtures(this.app as Application, true);
7879
});
7980

8081
after(async function () {

test/smoke/src/areas/positron/editor/fast-execution.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export function setup(logger: Logger) {
4040
// for every line of the file
4141
for (let i = 0; i < 10; i++) {
4242
let currentTop = await app.workbench.positronEditor.getCurrentLineTop();
43-
let retries = 10;
43+
let retries = 20;
4444

4545
// Note that top is a measurement of the distance from the top of the editor
4646
// to the top of the current line. By monitoring the top value, we can determine

0 commit comments

Comments
 (0)