Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python: Prevent unintended duplication of exited session #7022

Merged
merged 10 commits into from
Mar 28, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -299,10 +299,11 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
* Used to associated a session with a runtime.
*
* @param runtimeId The runtime identifier of the session to retrieve.
* @param includeExited Whether to include exited sessions in the search. (default false, optional)
* @returns The console session with the given runtime identifier, or undefined if
* no console session with the given runtime identifier exists.
*/
getConsoleSessionForRuntime(runtimeId: string): ILanguageRuntimeSession | undefined {
getConsoleSessionForRuntime(runtimeId: string, includeExited: boolean = false): ILanguageRuntimeSession | undefined {
// It's possible that there are multiple consoles for the same runtime.
// In that case, we return the most recently created.
return Array.from(this._activeSessionsBySessionId.values())
Expand All @@ -314,7 +315,8 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
.find(({ info }) =>
info.session.runtimeMetadata.runtimeId === runtimeId &&
info.session.metadata.sessionMode === LanguageRuntimeSessionMode.Console &&
info.state !== RuntimeState.Exited)
(includeExited || info.state !== RuntimeState.Exited)
)
?.info.session;
}

Expand Down Expand Up @@ -436,10 +438,12 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
} else {
if (multiSessionsEnabled) {
// Check if there is a console session for this runtime already
const activeSession = this.getConsoleSessionForRuntime(runtimeId);
if (activeSession) {
const existingSession = this.getConsoleSessionForRuntime(runtimeId, true);
if (existingSession) {
// Set it as the foreground session and return.
this.foregroundSession = activeSession;
if (existingSession.runtimeMetadata.runtimeId !== this.foregroundSession?.runtimeMetadata.runtimeId) {
this.foregroundSession = existingSession;
}
return;
}
} else {
Expand Down
10 changes: 3 additions & 7 deletions test/e2e/pages/sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -668,13 +668,9 @@ export class Sessions {
await expect(sessionTab.locator(statusClass)).toBeVisible({ timeout });
} else if (sessionCount === 1) {
// get status from metadata dialog because there is no tab list view
await expect.poll(
async () => (await this.getMetadata()).state,
{
timeout: 15000,
intervals: [1000]
}
).toBe(expectedStatus);
await this.metadataButton.click();
await expect(this.metadataDialog.getByText(`State: ${expectedStatus}`)).toBeVisible({ timeout });
await this.page.keyboard.press('Escape');
} else {
throw new Error('No sessions found');
}
Expand Down
80 changes: 36 additions & 44 deletions test/e2e/tests/sessions/session-state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,17 @@ test.describe('Sessions: State', {
tag: [tags.WIN, tags.WEB, tags.CONSOLE, tags.SESSIONS]
}, () => {

test.beforeAll(async function ({ userSettings, sessions }) {
test.beforeAll(async function ({ userSettings }) {
await userSettings.set([['console.multipleConsoleSessions', 'true']], true);
});

test.beforeEach(async function ({ app, sessions }) {
await app.workbench.variables.togglePane('hide');
await sessions.deleteDisconnectedSessions();
});

test.afterEach(async function ({ sessions }) {
await sessions.clearConsoleAllSessions();
});

test.skip('Validate state between sessions (active, idle, disconnect)', {
annotation: [{ type: 'issue', description: 'https://github.com/posit-dev/positron/issues/7005' },]
}, async function ({ app, sessions }) {

test.skip('Validate state between sessions (active, idle, disconnect)', async function ({ app, sessions }) {
const { console } = app.workbench;

// Start Python session
Expand All @@ -44,7 +38,7 @@ test.describe('Sessions: State', {
// Note displays as 'starting' in metadata dialog and as 'active' in session tab list
await sessions.restartButton.click();
await sessions.expectStatusToBe(pySession.id, 'starting');
await sessions.expectStatusToBe(pySession.id, 'idle');
await sessions.expectStatusToBe(pySession.id, 'idle', { timeout: 60000 });

// Start R session
const rSession = await sessions.start('r', { waitForReady: false });
Expand Down Expand Up @@ -101,39 +95,37 @@ test.describe('Sessions: State', {
await sessions.expectStatusToBe(pySession.name, 'idle');
});

test.skip('Validate metadata between sessions',
{ annotation: [{ type: 'issue', description: 'https://github.com/posit-dev/positron/issues/6987' }] },
async function ({ app, sessions }) {
const { console } = app.workbench;

// Ensure sessions exist and are idle
const [pySession, rSession, pySessionAlt] = await sessions.start(['python', 'r', 'pythonAlt']);
await sessions.resizeSessionList({ x: -100 });

// Verify Python session metadata
await sessions.expectMetaDataToBe({ ...pySession, state: 'idle' });
await sessions.expectMetaDataToBe({ ...rSession, state: 'idle' });
await sessions.expectMetaDataToBe({ ...pySessionAlt, state: 'idle' });

// Shutdown Python session and verify metadata
await sessions.select(pySession.id);
await console.typeToConsole('exit()', true);
await sessions.expectMetaDataToBe({ ...pySession, state: 'exited' });
await sessions.expectMetaDataToBe({ ...rSession, state: 'idle' });
await sessions.expectMetaDataToBe({ ...pySessionAlt, state: 'idle' });

// Shutdown R session and verify metadata
await sessions.select(rSession.id);
await console.typeToConsole('q()', true);
await sessions.expectMetaDataToBe({ ...pySession, state: 'exited' });
await sessions.expectMetaDataToBe({ ...rSession, state: 'exited' });
await sessions.expectMetaDataToBe({ ...pySessionAlt, state: 'idle' });

// Shutdown Alt Python session and verify metadata
await sessions.select(pySessionAlt.id);
await console.typeToConsole('exit()', true);
await sessions.expectMetaDataToBe({ ...pySession, state: 'exited' });
await sessions.expectMetaDataToBe({ ...rSession, state: 'exited' });
await sessions.expectMetaDataToBe({ ...pySessionAlt, state: 'exited' });
});
test('Validate metadata between sessions', async function ({ app, sessions }) {
const { console } = app.workbench;

// Ensure sessions exist and are idle
const [pySession, rSession, pySessionAlt] = await sessions.start(['python', 'r', 'pythonAlt']);
await sessions.resizeSessionList({ x: -100 });

// Verify Python session metadata
await sessions.expectMetaDataToBe({ ...pySession, state: 'idle' });
await sessions.expectMetaDataToBe({ ...rSession, state: 'idle' });
await sessions.expectMetaDataToBe({ ...pySessionAlt, state: 'idle' });

// Shutdown Python session and verify metadata
await sessions.select(pySession.id);
await console.typeToConsole('exit()', true);
await sessions.expectMetaDataToBe({ ...pySession, state: 'exited' });
await sessions.expectMetaDataToBe({ ...rSession, state: 'idle' });
await sessions.expectMetaDataToBe({ ...pySessionAlt, state: 'idle' });

// Shutdown R session and verify metadata
await sessions.select(rSession.id);
await console.typeToConsole('q()', true);
await sessions.expectMetaDataToBe({ ...pySession, state: 'exited' });
await sessions.expectMetaDataToBe({ ...rSession, state: 'exited' });
await sessions.expectMetaDataToBe({ ...pySessionAlt, state: 'idle' });

// Shutdown Alt Python session and verify metadata
await sessions.select(pySessionAlt.id);
await console.typeToConsole('exit()', true);
await sessions.expectMetaDataToBe({ ...pySession, state: 'exited' });
await sessions.expectMetaDataToBe({ ...rSession, state: 'exited' });
await sessions.expectMetaDataToBe({ ...pySessionAlt, state: 'exited' });
});
});