From 24131182fe7918af9724aa18f1d194610ea8f75f Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Mon, 9 Sep 2024 16:04:46 -0700 Subject: [PATCH 01/37] vscode engine to 1.93 --- package-lock.json | 16 ++++++++-------- package.json | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/package-lock.json b/package-lock.json index 06737473c436..1952f20e2430 100644 --- a/package-lock.json +++ b/package-lock.json @@ -57,7 +57,7 @@ "@types/sinon": "^17.0.3", "@types/stack-trace": "0.0.29", "@types/tmp": "^0.0.33", - "@types/vscode": "^1.81.0", + "@types/vscode": "^1.93", "@types/which": "^2.0.1", "@types/winreg": "^1.2.30", "@types/xml2js": "^0.4.2", @@ -116,7 +116,7 @@ "yargs": "^15.3.1" }, "engines": { - "vscode": "^1.91.0" + "vscode": "1.93" } }, "node_modules/@aashutoshrathi/word-wrap": { @@ -1796,9 +1796,9 @@ "dev": true }, "node_modules/@types/vscode": { - "version": "1.81.0", - "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.81.0.tgz", - "integrity": "sha512-YIaCwpT+O2E7WOMq0eCgBEABE++SX3Yl/O02GoMIF2DO3qAtvw7m6BXFYsxnc6XyzwZgh6/s/UG78LSSombl2w==", + "version": "1.93.0", + "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.93.0.tgz", + "integrity": "sha512-kUK6jAHSR5zY8ps42xuW89NLcBpw1kOabah7yv38J8MyiYuOHxLQBi0e7zeXbQgVefDy/mZZetqEFC+Fl5eIEQ==", "dev": true }, "node_modules/@types/which": { @@ -16046,9 +16046,9 @@ "dev": true }, "@types/vscode": { - "version": "1.81.0", - "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.81.0.tgz", - "integrity": "sha512-YIaCwpT+O2E7WOMq0eCgBEABE++SX3Yl/O02GoMIF2DO3qAtvw7m6BXFYsxnc6XyzwZgh6/s/UG78LSSombl2w==", + "version": "1.93.0", + "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.93.0.tgz", + "integrity": "sha512-kUK6jAHSR5zY8ps42xuW89NLcBpw1kOabah7yv38J8MyiYuOHxLQBi0e7zeXbQgVefDy/mZZetqEFC+Fl5eIEQ==", "dev": true }, "@types/which": { diff --git a/package.json b/package.json index e033fde418c0..b7f12a3283a9 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "theme": "dark" }, "engines": { - "vscode": "^1.91.0" + "vscode": "1.93" }, "enableTelemetry": false, "keywords": [ @@ -1570,7 +1570,7 @@ "@types/sinon": "^17.0.3", "@types/stack-trace": "0.0.29", "@types/tmp": "^0.0.33", - "@types/vscode": "^1.81.0", + "@types/vscode": "^1.93", "@types/which": "^2.0.1", "@types/winreg": "^1.2.30", "@types/xml2js": "^0.4.2", From 5de9ca55d8ff7775c079debe23c20aca83f43dff Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Mon, 9 Sep 2024 16:18:06 -0700 Subject: [PATCH 02/37] use executeCommand --- src/client/common/terminal/service.ts | 59 +++- .../common/terminal/syncTerminalService.ts | 6 +- src/client/common/terminal/types.ts | 9 +- .../codeExecution/terminalCodeExecution.ts | 2 +- ...scode.proposed.envCollectionWorkspace.d.ts | 7 - types/vscode.proposed.envShellEvent.d.ts | 16 - ...ode.proposed.terminalShellIntegration.d.ts | 312 ------------------ 7 files changed, 66 insertions(+), 345 deletions(-) delete mode 100644 types/vscode.proposed.envShellEvent.d.ts delete mode 100644 types/vscode.proposed.terminalShellIntegration.d.ts diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index de276762de4b..9443c0c2664b 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -3,7 +3,7 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; -import { CancellationToken, Disposable, Event, EventEmitter, Terminal } from 'vscode'; +import { CancellationToken, Disposable, Event, EventEmitter, Terminal, window } from 'vscode'; import '../../common/extensions'; import { IInterpreterService } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; @@ -20,6 +20,7 @@ import { ITerminalService, TerminalCreationOptions, TerminalShellType, + ITerminalExecutedCommand, } from './types'; @injectable() @@ -32,6 +33,7 @@ export class TerminalService implements ITerminalService, Disposable { private terminalActivator: ITerminalActivator; private terminalAutoActivator: ITerminalAutoActivation; private readonly envVarScript = path.join(EXTENSION_ROOT_DIR, 'python_files', 'pythonrc.py'); + private readonly executeCommandListeners: Set = new Set(); public get onDidCloseTerminal(): Event { return this.terminalClosed.event.bind(this.terminalClosed); } @@ -48,8 +50,10 @@ export class TerminalService implements ITerminalService, Disposable { this.terminalActivator = this.serviceContainer.get(ITerminalActivator); } public dispose() { - if (this.terminal) { - this.terminal.dispose(); + this.terminal?.dispose(); + + for (const d of this.executeCommandListeners) { + d.dispose(); } } public async sendCommand(command: string, args: string[], _?: CancellationToken): Promise { @@ -59,8 +63,9 @@ export class TerminalService implements ITerminalService, Disposable { this.terminal!.show(true); } - this.terminal!.sendText(text, true); + await this.executeCommand(text); } + /** @deprecated */ public async sendText(text: string): Promise { await this.ensureTerminal(); if (!this.options?.hideFromUser) { @@ -68,15 +73,54 @@ export class TerminalService implements ITerminalService, Disposable { } this.terminal!.sendText(text); } + public async executeCommand(commandLine: string): Promise { + const terminal = await this.ensureTerminal(); + if (!this.options?.hideFromUser) { + terminal.show(true); + } + + // If terminal was just launched, wait some time for shell integration to onDidChangeShellIntegration. + if (!terminal.shellIntegration) { + const promise = new Promise((resolve) => { + const shellIntegrationChangeEventListener = window.onDidChangeTerminalShellIntegration(() => { + this.executeCommandListeners.delete(shellIntegrationChangeEventListener); + resolve(true); + }); + setTimeout(() => { + this.executeCommandListeners.add(shellIntegrationChangeEventListener); + resolve(false); + }, 3000); + }); + await promise; + } + + if (terminal.shellIntegration) { + const execution = terminal.shellIntegration.executeCommand(commandLine); + return await new Promise((resolve) => { + const listener = window.onDidEndTerminalShellExecution((e) => { + if (e.execution === execution) { + this.executeCommandListeners.delete(listener); + resolve({ execution, exitCode: e.exitCode }); + } + }); + this.executeCommandListeners.add(listener); + }); + } else { + terminal.sendText(commandLine); + } + + return undefined; + } + public async show(preserveFocus: boolean = true): Promise { await this.ensureTerminal(preserveFocus); if (!this.options?.hideFromUser) { this.terminal!.show(preserveFocus); } } - public async ensureTerminal(preserveFocus: boolean = true): Promise { + public async ensureTerminal(preserveFocus: boolean = true): Promise { if (this.terminal) { - return; + return this.terminal; } this.terminalShellType = this.terminalHelper.identifyTerminalShell(this.terminal); this.terminal = this.terminalManager.createTerminal({ @@ -97,10 +141,11 @@ export class TerminalService implements ITerminalService, Disposable { }); if (!this.options?.hideFromUser) { - this.terminal!.show(preserveFocus); + this.terminal.show(preserveFocus); } this.sendTelemetry().ignoreErrors(); + return this.terminal; } private terminalCloseHandler(terminal: Terminal) { if (terminal === this.terminal) { diff --git a/src/client/common/terminal/syncTerminalService.ts b/src/client/common/terminal/syncTerminalService.ts index 4e95ddab01b5..7b25c714a035 100644 --- a/src/client/common/terminal/syncTerminalService.ts +++ b/src/client/common/terminal/syncTerminalService.ts @@ -14,7 +14,7 @@ import * as internalScripts from '../process/internal/scripts'; import { createDeferred, Deferred } from '../utils/async'; import { noop } from '../utils/misc'; import { TerminalService } from './service'; -import { ITerminalService } from './types'; +import { ITerminalService, ITerminalExecutedCommand } from './types'; enum State { notStarted = 0, @@ -146,9 +146,13 @@ export class SynchronousTerminalService implements ITerminalService, Disposable lockFile.dispose(); } } + /** @deprecated */ public sendText(text: string): Promise { return this.terminalService.sendText(text); } + public executeCommand(commandLine: string): Promise { + return this.terminalService.executeCommand(commandLine); + } public show(preserveFocus?: boolean | undefined): Promise { return this.terminalService.show(preserveFocus); } diff --git a/src/client/common/terminal/types.ts b/src/client/common/terminal/types.ts index 49f42e7c19f6..aa8ff73cc205 100644 --- a/src/client/common/terminal/types.ts +++ b/src/client/common/terminal/types.ts @@ -3,7 +3,7 @@ 'use strict'; -import { CancellationToken, Event, Terminal, Uri } from 'vscode'; +import { CancellationToken, Event, Terminal, Uri, TerminalShellExecution } from 'vscode'; import { PythonEnvironment } from '../../pythonEnvironments/info'; import { IEventNamePropertyMapping } from '../../telemetry/index'; import { IDisposable, Resource } from '../types'; @@ -52,10 +52,17 @@ export interface ITerminalService extends IDisposable { cancel?: CancellationToken, swallowExceptions?: boolean, ): Promise; + /** @deprecated */ sendText(text: string): Promise; + executeCommand(commandLine: string): Promise; show(preserveFocus?: boolean): Promise; } +export interface ITerminalExecutedCommand { + execution: TerminalShellExecution; + exitCode: number | undefined; +} + export const ITerminalServiceFactory = Symbol('ITerminalServiceFactory'); export type TerminalCreationOptions = { diff --git a/src/client/terminals/codeExecution/terminalCodeExecution.ts b/src/client/terminals/codeExecution/terminalCodeExecution.ts index f2750fedaa07..3cba6141763b 100644 --- a/src/client/terminals/codeExecution/terminalCodeExecution.ts +++ b/src/client/terminals/codeExecution/terminalCodeExecution.ts @@ -59,7 +59,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService { this.configurationService.updateSetting('REPL.enableREPLSmartSend', false, resource); } } else { - await this.getTerminalService(resource).sendText(code); + await this.getTerminalService(resource).executeCommand(code); } } diff --git a/types/vscode.proposed.envCollectionWorkspace.d.ts b/types/vscode.proposed.envCollectionWorkspace.d.ts index 494929ba15eb..a03a639b5ee2 100644 --- a/types/vscode.proposed.envCollectionWorkspace.d.ts +++ b/types/vscode.proposed.envCollectionWorkspace.d.ts @@ -27,11 +27,4 @@ declare module 'vscode' { */ getScoped(scope: EnvironmentVariableScope): EnvironmentVariableCollection; } - - export type EnvironmentVariableScope = { - /** - * Any specific workspace folder to get collection for. If unspecified, collection applicable to all workspace folders is returned. - */ - workspaceFolder?: WorkspaceFolder; - }; } diff --git a/types/vscode.proposed.envShellEvent.d.ts b/types/vscode.proposed.envShellEvent.d.ts deleted file mode 100644 index 8fed971ef711..000000000000 --- a/types/vscode.proposed.envShellEvent.d.ts +++ /dev/null @@ -1,16 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -declare module 'vscode' { - - // See https://github.com/microsoft/vscode/issues/160694 - export namespace env { - - /** - * An {@link Event} which fires when the default shell changes. - */ - export const onDidChangeShell: Event; - } -} diff --git a/types/vscode.proposed.terminalShellIntegration.d.ts b/types/vscode.proposed.terminalShellIntegration.d.ts deleted file mode 100644 index 5ff18b60ca72..000000000000 --- a/types/vscode.proposed.terminalShellIntegration.d.ts +++ /dev/null @@ -1,312 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -declare module 'vscode' { - // https://github.com/microsoft/vscode/issues/145234 - - /** - * A command that was executed in a terminal. - */ - export interface TerminalShellExecution { - /** - * The command line that was executed. The {@link TerminalShellExecutionCommandLineConfidence confidence} - * of this value depends on the specific shell's shell integration implementation. This - * value may become more accurate after {@link window.onDidEndTerminalShellExecution} is - * fired. - * - * @example - * // Log the details of the command line on start and end - * window.onDidStartTerminalShellExecution(event => { - * const commandLine = event.execution.commandLine; - * console.log(`Command started\n${summarizeCommandLine(commandLine)}`); - * }); - * window.onDidEndTerminalShellExecution(event => { - * const commandLine = event.execution.commandLine; - * console.log(`Command ended\n${summarizeCommandLine(commandLine)}`); - * }); - * function summarizeCommandLine(commandLine: TerminalShellExecutionCommandLine) { - * return [ - * ` Command line: ${command.ommandLine.value}`, - * ` Confidence: ${command.ommandLine.confidence}`, - * ` Trusted: ${command.ommandLine.isTrusted} - * ].join('\n'); - * } - */ - readonly commandLine: TerminalShellExecutionCommandLine; - - /** - * The working directory that was reported by the shell when this command executed. This - * {@link Uri} may represent a file on another machine (eg. ssh into another machine). This - * requires the shell integration to support working directory reporting. - */ - readonly cwd: Uri | undefined; - - /** - * Creates a stream of raw data (including escape sequences) that is written to the - * terminal. This will only include data that was written after `readData` was called for - * the first time, ie. you must call `readData` immediately after the command is executed - * via {@link TerminalShellIntegration.executeCommand} or - * {@link window.onDidStartTerminalShellExecution} to not miss any data. - * - * @example - * // Log all data written to the terminal for a command - * const command = term.shellIntegration.executeCommand({ commandLine: 'echo "Hello world"' }); - * const stream = command.read(); - * for await (const data of stream) { - * console.log(data); - * } - */ - read(): AsyncIterable; - } - - /** - * A command line that was executed in a terminal. - */ - export interface TerminalShellExecutionCommandLine { - /** - * The full command line that was executed, including both the command and its arguments. - */ - readonly value: string; - - /** - * Whether the command line value came from a trusted source and is therefore safe to - * execute without user additional confirmation, such as a notification that asks "Do you - * want to execute (command)?". This verification is likely only needed if you are going to - * execute the command again. - * - * This is `true` only when the command line was reported explicitly by the shell - * integration script (ie. {@link TerminalShellExecutionCommandLineConfidence.High high confidence}) - * and it used a nonce for verification. - */ - readonly isTrusted: boolean; - - /** - * The confidence of the command line value which is determined by how the value was - * obtained. This depends upon the implementation of the shell integration script. - */ - readonly confidence: TerminalShellExecutionCommandLineConfidence; - } - - /** - * The confidence of a {@link TerminalShellExecutionCommandLine} value. - */ - enum TerminalShellExecutionCommandLineConfidence { - /** - * The command line value confidence is low. This means that the value was read from the - * terminal buffer using markers reported by the shell integration script. Additionally one - * of the following conditions will be met: - * - * - The command started on the very left-most column which is unusual, or - * - The command is multi-line which is more difficult to accurately detect due to line - * continuation characters and right prompts. - * - Command line markers were not reported by the shell integration script. - */ - Low = 0, - - /** - * The command line value confidence is medium. This means that the value was read from the - * terminal buffer using markers reported by the shell integration script. The command is - * single-line and does not start on the very left-most column (which is unusual). - */ - Medium = 1, - - /** - * The command line value confidence is high. This means that the value was explicitly sent - * from the shell integration script or the command was executed via the - * {@link TerminalShellIntegration.executeCommand} API. - */ - High = 2, - } - - export interface Terminal { - /** - * An object that contains [shell integration](https://code.visualstudio.com/docs/terminal/shell-integration)-powered - * features for the terminal. This will always be `undefined` immediately after the terminal - * is created. Listen to {@link window.onDidActivateTerminalShellIntegration} to be notified - * when shell integration is activated for a terminal. - * - * Note that this object may remain undefined if shell integation never activates. For - * example Command Prompt does not support shell integration and a user's shell setup could - * conflict with the automatic shell integration activation. - */ - readonly shellIntegration: TerminalShellIntegration | undefined; - } - - /** - * [Shell integration](https://code.visualstudio.com/docs/terminal/shell-integration)-powered capabilities owned by a terminal. - */ - export interface TerminalShellIntegration { - /** - * The current working directory of the terminal. This {@link Uri} may represent a file on - * another machine (eg. ssh into another machine). This requires the shell integration to - * support working directory reporting. - */ - readonly cwd: Uri | undefined; - - /** - * Execute a command, sending ^C as necessary to interrupt any running command if needed. - * - * @param commandLine The command line to execute, this is the exact text that will be sent - * to the terminal. - * - * @example - * // Execute a command in a terminal immediately after being created - * const myTerm = window.createTerminal(); - * window.onDidActivateTerminalShellIntegration(async ({ terminal, shellIntegration }) => { - * if (terminal === myTerm) { - * const command = shellIntegration.executeCommand('echo "Hello world"'); - * const code = await command.exitCode; - * console.log(`Command exited with code ${code}`); - * } - * })); - * // Fallback to sendText if there is no shell integration within 3 seconds of launching - * setTimeout(() => { - * if (!myTerm.shellIntegration) { - * myTerm.sendText('echo "Hello world"'); - * // Without shell integration, we can't know when the command has finished or what the - * // exit code was. - * } - * }, 3000); - * - * @example - * // Send command to terminal that has been alive for a while - * const commandLine = 'echo "Hello world"'; - * if (term.shellIntegration) { - * const command = term.shellIntegration.executeCommand({ commandLine }); - * const code = await command.exitCode; - * console.log(`Command exited with code ${code}`); - * } else { - * term.sendText(commandLine); - * // Without shell integration, we can't know when the command has finished or what the - * // exit code was. - * } - */ - executeCommand(commandLine: string): TerminalShellExecution; - - /** - * Execute a command, sending ^C as necessary to interrupt any running command if needed. - * - * *Note* This is not guaranteed to work as [shell integration](https://code.visualstudio.com/docs/terminal/shell-integration) - * must be activated. Check whether {@link TerminalShellExecution.exitCode} is rejected to - * verify whether it was successful. - * - * @param command A command to run. - * @param args Arguments to launch the executable with which will be automatically escaped - * based on the executable type. - * - * @example - * // Execute a command in a terminal immediately after being created - * const myTerm = window.createTerminal(); - * window.onDidActivateTerminalShellIntegration(async ({ terminal, shellIntegration }) => { - * if (terminal === myTerm) { - * const command = shellIntegration.executeCommand({ - * command: 'echo', - * args: ['Hello world'] - * }); - * const code = await command.exitCode; - * console.log(`Command exited with code ${code}`); - * } - * })); - * // Fallback to sendText if there is no shell integration within 3 seconds of launching - * setTimeout(() => { - * if (!myTerm.shellIntegration) { - * myTerm.sendText('echo "Hello world"'); - * // Without shell integration, we can't know when the command has finished or what the - * // exit code was. - * } - * }, 3000); - * - * @example - * // Send command to terminal that has been alive for a while - * const commandLine = 'echo "Hello world"'; - * if (term.shellIntegration) { - * const command = term.shellIntegration.executeCommand({ - * command: 'echo', - * args: ['Hello world'] - * }); - * const code = await command.exitCode; - * console.log(`Command exited with code ${code}`); - * } else { - * term.sendText(commandLine); - * // Without shell integration, we can't know when the command has finished or what the - * // exit code was. - * } - */ - executeCommand(executable: string, args: string[]): TerminalShellExecution; - } - - export interface TerminalShellIntegrationChangeEvent { - /** - * The terminal that shell integration has been activated in. - */ - readonly terminal: Terminal; - - /** - * The shell integration object. - */ - readonly shellIntegration: TerminalShellIntegration; - } - - export interface TerminalShellExecutionStartEvent { - /** - * The terminal that shell integration has been activated in. - */ - readonly terminal: Terminal; - - /** - * The shell integration object. - */ - readonly shellIntegration: TerminalShellIntegration; - - /** - * The terminal shell execution that has ended. - */ - readonly execution: TerminalShellExecution; - } - - export interface TerminalShellExecutionEndEvent { - /** - * The terminal that shell integration has been activated in. - */ - readonly terminal: Terminal; - - /** - * The shell integration object. - */ - readonly shellIntegration: TerminalShellIntegration; - - /** - * The terminal shell execution that has ended. - */ - readonly execution: TerminalShellExecution; - - /** - * The exit code reported by the shell. `undefined` means the shell did not report an exit - * code or the shell reported a command started before the command finished. - */ - readonly exitCode: number | undefined; - } - - export namespace window { - /** - * Fires when shell integration activates or one of its properties changes in a terminal. - */ - export const onDidChangeTerminalShellIntegration: Event; - - /** - * This will be fired when a terminal command is started. This event will fire only when - * [shell integration](https://code.visualstudio.com/docs/terminal/shell-integration) is - * activated for the terminal. - */ - export const onDidStartTerminalShellExecution: Event; - - /** - * This will be fired when a terminal command is ended. This event will fire only when - * [shell integration](https://code.visualstudio.com/docs/terminal/shell-integration) is - * activated for the terminal. - */ - export const onDidEndTerminalShellExecution: Event; - } -} From 6d1f52275193e79f5a03fdf685889695106a2204 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Mon, 9 Sep 2024 16:18:46 -0700 Subject: [PATCH 03/37] codeaction mock --- src/test/mocks/vsc/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/mocks/vsc/index.ts b/src/test/mocks/vsc/index.ts index 774ba388bb4e..4ba0c0bcbf92 100644 --- a/src/test/mocks/vsc/index.ts +++ b/src/test/mocks/vsc/index.ts @@ -369,6 +369,8 @@ export class CodeActionKind { public static readonly SourceFixAll: CodeActionKind = new CodeActionKind('source.fix.all'); + public static readonly Notebook: CodeActionKind = new CodeActionKind('notebook'); + private constructor(private _value: string) {} public append(parts: string): CodeActionKind { From c9fc4f1badcfad2ba0569eac4b9e4f65f814c1ef Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Mon, 9 Sep 2024 16:22:34 -0700 Subject: [PATCH 04/37] compile --- .../activation/terminalEnvVarCollectionService.unit.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index 22b5dcf7477a..6d3d7ffd8c74 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -134,7 +134,6 @@ suite('Terminal Environment Variable Collection Service', () => { test('When not in experiment, do not apply activated variables to the collection and clear it instead', async () => { reset(experimentService); - when(context.environmentVariableCollection).thenReturn(instance(collection)); when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(false); const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); applyCollectionStub.resolves(); From 0d78806a8054c81d6d4cbf83e267c4a09390d8ac Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Mon, 9 Sep 2024 23:25:03 -0700 Subject: [PATCH 05/37] switching version allows debugging - magically --- package-lock.json | 4 ++-- package.json | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1952f20e2430..4f7e00dd1394 100644 --- a/package-lock.json +++ b/package-lock.json @@ -57,7 +57,7 @@ "@types/sinon": "^17.0.3", "@types/stack-trace": "0.0.29", "@types/tmp": "^0.0.33", - "@types/vscode": "^1.93", + "@types/vscode": "^1.81.0", "@types/which": "^2.0.1", "@types/winreg": "^1.2.30", "@types/xml2js": "^0.4.2", @@ -116,7 +116,7 @@ "yargs": "^15.3.1" }, "engines": { - "vscode": "1.93" + "vscode": "^1.91.0" } }, "node_modules/@aashutoshrathi/word-wrap": { diff --git a/package.json b/package.json index b7f12a3283a9..e033fde418c0 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "theme": "dark" }, "engines": { - "vscode": "1.93" + "vscode": "^1.91.0" }, "enableTelemetry": false, "keywords": [ @@ -1570,7 +1570,7 @@ "@types/sinon": "^17.0.3", "@types/stack-trace": "0.0.29", "@types/tmp": "^0.0.33", - "@types/vscode": "^1.93", + "@types/vscode": "^1.81.0", "@types/which": "^2.0.1", "@types/winreg": "^1.2.30", "@types/xml2js": "^0.4.2", From a16a1025fd06f6d4441d9a0c515d7239c7b0d8e9 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 10:06:19 -0700 Subject: [PATCH 06/37] pin version to equal to or above 1.93 --- package-lock.json | 4 ++-- package.json | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4f7e00dd1394..7d8cc145dc38 100644 --- a/package-lock.json +++ b/package-lock.json @@ -57,7 +57,7 @@ "@types/sinon": "^17.0.3", "@types/stack-trace": "0.0.29", "@types/tmp": "^0.0.33", - "@types/vscode": "^1.81.0", + "@types/vscode": "^1.93.0", "@types/which": "^2.0.1", "@types/winreg": "^1.2.30", "@types/xml2js": "^0.4.2", @@ -116,7 +116,7 @@ "yargs": "^15.3.1" }, "engines": { - "vscode": "^1.91.0" + "vscode": "^1.93.0" } }, "node_modules/@aashutoshrathi/word-wrap": { diff --git a/package.json b/package.json index e033fde418c0..e588b025c159 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "theme": "dark" }, "engines": { - "vscode": "^1.91.0" + "vscode": "^1.93.0" }, "enableTelemetry": false, "keywords": [ @@ -1570,7 +1570,7 @@ "@types/sinon": "^17.0.3", "@types/stack-trace": "0.0.29", "@types/tmp": "^0.0.33", - "@types/vscode": "^1.81.0", + "@types/vscode": "^1.93.0", "@types/which": "^2.0.1", "@types/winreg": "^1.2.30", "@types/xml2js": "^0.4.2", From ea24d509dd2e6ee2b84c723a85bca9b3533fe665 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 11:00:00 -0700 Subject: [PATCH 07/37] try to fix unit test --- src/client/common/terminal/service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index 9443c0c2664b..c0e1305aaa4d 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -88,7 +88,7 @@ export class TerminalService implements ITerminalService, Disposable { }); setTimeout(() => { this.executeCommandListeners.add(shellIntegrationChangeEventListener); - resolve(false); + resolve(true); }, 3000); }); await promise; From 2cb1eb3c532928b616f3f7fc1faa61699fc2da40 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 14:14:26 -0700 Subject: [PATCH 08/37] Debt: switch Promise in ensureTerminal to Promise --- src/client/common/terminal/service.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index c0e1305aaa4d..69d8f168e8e3 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -74,7 +74,8 @@ export class TerminalService implements ITerminalService, Disposable { this.terminal!.sendText(text); } public async executeCommand(commandLine: string): Promise { - const terminal = await this.ensureTerminal(); + // const terminal = await this.ensureTerminal(); + const terminal = this.terminal!; if (!this.options?.hideFromUser) { terminal.show(true); } @@ -118,9 +119,11 @@ export class TerminalService implements ITerminalService, Disposable { this.terminal!.show(preserveFocus); } } - public async ensureTerminal(preserveFocus: boolean = true): Promise { + // TODO: Debt switch to Promise ---> breaks 20 tests + public async ensureTerminal(preserveFocus: boolean = true): Promise { if (this.terminal) { - return this.terminal; + // return this.terminal; + return; } this.terminalShellType = this.terminalHelper.identifyTerminalShell(this.terminal); this.terminal = this.terminalManager.createTerminal({ @@ -133,7 +136,7 @@ export class TerminalService implements ITerminalService, Disposable { // Sometimes the terminal takes some time to start up before it can start accepting input. await new Promise((resolve) => setTimeout(resolve, 100)); - await this.terminalActivator.activateEnvironmentInTerminal(this.terminal!, { + await this.terminalActivator.activateEnvironmentInTerminal(this.terminal, { resource: this.options?.resource, preserveFocus, interpreter: this.options?.interpreter, @@ -145,7 +148,8 @@ export class TerminalService implements ITerminalService, Disposable { } this.sendTelemetry().ignoreErrors(); - return this.terminal; + // return this.terminal; + return; } private terminalCloseHandler(terminal: Terminal) { if (terminal === this.terminal) { From 88b4e9fea85d74ead2f83d802f331ea67946f1a7 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 16:12:36 -0700 Subject: [PATCH 09/37] passing test when shell integration disabled --- .../common/application/terminalManager.ts | 17 ++++++++- src/client/common/application/types.ts | 6 +++ src/client/common/terminal/service.ts | 16 ++++---- .../common/terminals/service.unit.test.ts | 37 +++++++++++++++++-- 4 files changed, 64 insertions(+), 12 deletions(-) diff --git a/src/client/common/application/terminalManager.ts b/src/client/common/application/terminalManager.ts index e5b758437393..9d0536e85243 100644 --- a/src/client/common/application/terminalManager.ts +++ b/src/client/common/application/terminalManager.ts @@ -2,7 +2,16 @@ // Licensed under the MIT License. import { injectable } from 'inversify'; -import { Event, EventEmitter, Terminal, TerminalOptions, window } from 'vscode'; +import { + Disposable, + Event, + EventEmitter, + Terminal, + TerminalOptions, + TerminalShellExecutionEndEvent, + TerminalShellIntegrationChangeEvent, + window, +} from 'vscode'; import { traceLog } from '../../logging'; import { ITerminalManager } from './types'; @@ -23,6 +32,12 @@ export class TerminalManager implements ITerminalManager { public createTerminal(options: TerminalOptions): Terminal { return monkeyPatchTerminal(window.createTerminal(options)); } + public onDidChangeTerminalShellIntegration(handler: (e: TerminalShellIntegrationChangeEvent) => void): Disposable { + return window.onDidChangeTerminalShellIntegration(handler); + } + public onDidEndTerminalShellExecution(handler: (e: TerminalShellExecutionEndEvent) => void): Disposable { + return window.onDidEndTerminalShellExecution(handler); + } } /** diff --git a/src/client/common/application/types.ts b/src/client/common/application/types.ts index 6705331bf57d..413122f2584b 100644 --- a/src/client/common/application/types.ts +++ b/src/client/common/application/types.ts @@ -40,6 +40,8 @@ import { StatusBarItem, Terminal, TerminalOptions, + TerminalShellExecutionEndEvent, + TerminalShellIntegrationChangeEvent, TextDocument, TextDocumentChangeEvent, TextDocumentShowOptions, @@ -936,6 +938,10 @@ export interface ITerminalManager { * @return A new Terminal. */ createTerminal(options: TerminalOptions): Terminal; + + onDidChangeTerminalShellIntegration(handler: (e: TerminalShellIntegrationChangeEvent) => void): Disposable; + + onDidEndTerminalShellExecution(handler: (e: TerminalShellExecutionEndEvent) => void): Disposable; } export const IDebugService = Symbol('IDebugManager'); diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index 69d8f168e8e3..46107bf6e390 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -3,7 +3,7 @@ import { inject, injectable } from 'inversify'; import * as path from 'path'; -import { CancellationToken, Disposable, Event, EventEmitter, Terminal, window } from 'vscode'; +import { CancellationToken, Disposable, Event, EventEmitter, Terminal } from 'vscode'; import '../../common/extensions'; import { IInterpreterService } from '../../interpreter/contracts'; import { IServiceContainer } from '../../ioc/types'; @@ -53,7 +53,7 @@ export class TerminalService implements ITerminalService, Disposable { this.terminal?.dispose(); for (const d of this.executeCommandListeners) { - d.dispose(); + d?.dispose(); } } public async sendCommand(command: string, args: string[], _?: CancellationToken): Promise { @@ -83,10 +83,12 @@ export class TerminalService implements ITerminalService, Disposable { // If terminal was just launched, wait some time for shell integration to onDidChangeShellIntegration. if (!terminal.shellIntegration) { const promise = new Promise((resolve) => { - const shellIntegrationChangeEventListener = window.onDidChangeTerminalShellIntegration(() => { - this.executeCommandListeners.delete(shellIntegrationChangeEventListener); - resolve(true); - }); + const shellIntegrationChangeEventListener = this.terminalManager.onDidChangeTerminalShellIntegration( + () => { + this.executeCommandListeners.delete(shellIntegrationChangeEventListener); + resolve(true); + }, + ); setTimeout(() => { this.executeCommandListeners.add(shellIntegrationChangeEventListener); resolve(true); @@ -98,7 +100,7 @@ export class TerminalService implements ITerminalService, Disposable { if (terminal.shellIntegration) { const execution = terminal.shellIntegration.executeCommand(commandLine); return await new Promise((resolve) => { - const listener = window.onDidEndTerminalShellExecution((e) => { + const listener = this.terminalManager.onDidEndTerminalShellExecution((e) => { if (e.execution === execution) { this.executeCommandListeners.delete(listener); resolve({ execution, exitCode: e.exitCode }); diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index 61556e3df2d1..07f9c3cff75b 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -4,7 +4,15 @@ import { expect } from 'chai'; import * as path from 'path'; import * as TypeMoq from 'typemoq'; -import { Disposable, Terminal as VSCodeTerminal, WorkspaceConfiguration } from 'vscode'; +import { + Disposable, + EventEmitter, + TerminalShellExecution, + TerminalShellExecutionEndEvent, + TerminalShellIntegration, + Terminal as VSCodeTerminal, + WorkspaceConfiguration, +} from 'vscode'; import { ITerminalManager, IWorkspaceService } from '../../../client/common/application/types'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { IPlatformService } from '../../../client/common/platform/types'; @@ -26,9 +34,29 @@ suite('Terminal Service', () => { let disposables: Disposable[] = []; let mockServiceContainer: TypeMoq.IMock; let terminalAutoActivator: TypeMoq.IMock; + let terminalShellIntegration: TypeMoq.IMock; + let executeCommandListeners: Set; setup(() => { terminal = TypeMoq.Mock.ofType(); + terminalShellIntegration = TypeMoq.Mock.ofType(); + // terminal.setup((t) => t.shellIntegration).returns(() => terminalShellIntegration.object); + terminal.setup((t) => t.shellIntegration).returns(() => undefined); + const shellExecution: TypeMoq.IMock = TypeMoq.Mock.ofType(); + + terminalShellIntegration + .setup((t) => t.executeCommand(TypeMoq.It.isAny())) + .returns(() => shellExecution.object); + terminalManager = TypeMoq.Mock.ofType(); + // terminalManager.setup((t) => t.onDidEndTerminalShellExecution(TypeMoq.It.isAny())) + const onDidEndTerminalShellExecutionEmitter = new EventEmitter(); + terminalManager + .setup((t) => t.onDidEndTerminalShellExecution(TypeMoq.It.isAny())) + .callback((handler) => { + onDidEndTerminalShellExecutionEmitter.event(handler); + }); + + executeCommandListeners = new Set(); platformService = TypeMoq.Mock.ofType(); workspaceService = TypeMoq.Mock.ofType(); terminalHelper = TypeMoq.Mock.ofType(); @@ -37,6 +65,7 @@ suite('Terminal Service', () => { disposables = []; mockServiceContainer = TypeMoq.Mock.ofType(); + mockServiceContainer.setup((c) => c.get(ITerminalManager)).returns(() => terminalManager.object); mockServiceContainer.setup((c) => c.get(ITerminalHelper)).returns(() => terminalHelper.object); mockServiceContainer.setup((c) => c.get(IPlatformService)).returns(() => platformService.object); @@ -78,7 +107,7 @@ suite('Terminal Service', () => { // Sending a command will cause the terminal to be created await service.sendCommand('', []); - terminal.verify((t) => t.show(TypeMoq.It.isValue(true)), TypeMoq.Times.exactly(2)); + terminal.verify((t) => t.show(TypeMoq.It.isValue(true)), TypeMoq.Times.atLeastOnce()); service.dispose(); terminal.verify((t) => t.dispose(), TypeMoq.Times.exactly(1)); }); @@ -99,10 +128,10 @@ suite('Terminal Service', () => { await service.sendCommand(commandToSend, args); - terminal.verify((t) => t.show(TypeMoq.It.isValue(true)), TypeMoq.Times.exactly(2)); + terminal.verify((t) => t.show(TypeMoq.It.isValue(true)), TypeMoq.Times.atLeastOnce()); terminal.verify( (t) => t.sendText(TypeMoq.It.isValue(commandToExpect), TypeMoq.It.isValue(true)), - TypeMoq.Times.exactly(1), + TypeMoq.Times.never(), // should be calling sendText but rather executeCommand() ); }); From 6ecf4fc337d15ca7a2ac61ed6e34ebaa4dcb614a Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 16:33:20 -0700 Subject: [PATCH 10/37] please --- src/test/common/terminals/service.unit.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index 07f9c3cff75b..01c6ed390e6e 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -57,6 +57,8 @@ suite('Terminal Service', () => { }); executeCommandListeners = new Set(); + executeCommandListeners.add(new Disposable(() => {})); + platformService = TypeMoq.Mock.ofType(); workspaceService = TypeMoq.Mock.ofType(); terminalHelper = TypeMoq.Mock.ofType(); From 831f893ff2fd608d4e5083f61d82574d50f36107 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 16:47:01 -0700 Subject: [PATCH 11/37] // @ts-ignore: TS6133 --- src/test/common/terminals/service.unit.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index 01c6ed390e6e..cd8eea7c3768 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +// @ts-ignore: TS6133 import { expect } from 'chai'; import * as path from 'path'; import * as TypeMoq from 'typemoq'; @@ -57,7 +58,7 @@ suite('Terminal Service', () => { }); executeCommandListeners = new Set(); - executeCommandListeners.add(new Disposable(() => {})); + // executeCommandListeners.add(new Disposable(() => {})); platformService = TypeMoq.Mock.ofType(); workspaceService = TypeMoq.Mock.ofType(); From f3aa28e91ad7f1a190f675268d8fad9cacd0c5ca Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 16:50:48 -0700 Subject: [PATCH 12/37] a --- src/test/common/terminals/service.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index cd8eea7c3768..b249f4c99ddd 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -36,7 +36,7 @@ suite('Terminal Service', () => { let mockServiceContainer: TypeMoq.IMock; let terminalAutoActivator: TypeMoq.IMock; let terminalShellIntegration: TypeMoq.IMock; - let executeCommandListeners: Set; + let executeCommandListeners: Set; // eslint-disable-line @typescript-eslint/no-unused-vars setup(() => { terminal = TypeMoq.Mock.ofType(); terminalShellIntegration = TypeMoq.Mock.ofType(); From 342587a4bbc9d8a1ff60621564a59aa2fe573cfa Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 17:00:27 -0700 Subject: [PATCH 13/37] stop --- src/test/common/terminals/service.unit.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index b249f4c99ddd..e30878ab524b 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -36,7 +36,8 @@ suite('Terminal Service', () => { let mockServiceContainer: TypeMoq.IMock; let terminalAutoActivator: TypeMoq.IMock; let terminalShellIntegration: TypeMoq.IMock; - let executeCommandListeners: Set; // eslint-disable-line @typescript-eslint/no-unused-vars + // eslint-disable-line @typescript-eslint/no-unused-vars + let executeCommandListeners: Set; setup(() => { terminal = TypeMoq.Mock.ofType(); terminalShellIntegration = TypeMoq.Mock.ofType(); From 3efaf9ae8f0f8e5b3119d5dcfe99d79e2a1b5c68 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 17:06:58 -0700 Subject: [PATCH 14/37] try idisposable --- src/test/common/terminals/service.unit.test.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index e30878ab524b..6d5ed85d5009 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -19,7 +19,7 @@ import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { IPlatformService } from '../../../client/common/platform/types'; import { TerminalService } from '../../../client/common/terminal/service'; import { ITerminalActivator, ITerminalHelper, TerminalShellType } from '../../../client/common/terminal/types'; -import { IDisposableRegistry } from '../../../client/common/types'; +import { IDisposable, IDisposableRegistry } from '../../../client/common/types'; import { IServiceContainer } from '../../../client/ioc/types'; import { ITerminalAutoActivation } from '../../../client/terminals/types'; import { createPythonInterpreter } from '../../utils/interpreters'; @@ -36,8 +36,7 @@ suite('Terminal Service', () => { let mockServiceContainer: TypeMoq.IMock; let terminalAutoActivator: TypeMoq.IMock; let terminalShellIntegration: TypeMoq.IMock; - // eslint-disable-line @typescript-eslint/no-unused-vars - let executeCommandListeners: Set; + let executeCommandListeners: TypeMoq.IMock[]; setup(() => { terminal = TypeMoq.Mock.ofType(); terminalShellIntegration = TypeMoq.Mock.ofType(); @@ -58,8 +57,8 @@ suite('Terminal Service', () => { onDidEndTerminalShellExecutionEmitter.event(handler); }); - executeCommandListeners = new Set(); - // executeCommandListeners.add(new Disposable(() => {})); + executeCommandListeners = new Array>(); + executeCommandListeners.push(TypeMoq.Mock.ofType()); platformService = TypeMoq.Mock.ofType(); workspaceService = TypeMoq.Mock.ofType(); From d3b2238c9fccccf93d1d2a459a1ee1f2cd9ce03d Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 17:16:42 -0700 Subject: [PATCH 15/37] more trials --- src/client/common/terminal/service.ts | 6 ++++-- src/test/common/terminals/service.unit.test.ts | 5 ++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index 46107bf6e390..8a3b9b114e05 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -52,8 +52,10 @@ export class TerminalService implements ITerminalService, Disposable { public dispose() { this.terminal?.dispose(); - for (const d of this.executeCommandListeners) { - d?.dispose(); + if (this.executeCommandListeners && this.executeCommandListeners.size > 0) { + for (const d of this.executeCommandListeners) { + d?.dispose(); + } } } public async sendCommand(command: string, args: string[], _?: CancellationToken): Promise { diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index 6d5ed85d5009..c7a8b9651cd3 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -36,7 +36,7 @@ suite('Terminal Service', () => { let mockServiceContainer: TypeMoq.IMock; let terminalAutoActivator: TypeMoq.IMock; let terminalShellIntegration: TypeMoq.IMock; - let executeCommandListeners: TypeMoq.IMock[]; + let executeCommandListeners: Disposable[] = []; setup(() => { terminal = TypeMoq.Mock.ofType(); terminalShellIntegration = TypeMoq.Mock.ofType(); @@ -57,8 +57,7 @@ suite('Terminal Service', () => { onDidEndTerminalShellExecutionEmitter.event(handler); }); - executeCommandListeners = new Array>(); - executeCommandListeners.push(TypeMoq.Mock.ofType()); + executeCommandListeners = []; platformService = TypeMoq.Mock.ofType(); workspaceService = TypeMoq.Mock.ofType(); From 28ef3c132a359d192fefce39416e898bfa93699a Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 17:18:49 -0700 Subject: [PATCH 16/37] lint --- src/test/common/terminals/service.unit.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index c7a8b9651cd3..2d45fec932fa 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -19,7 +19,7 @@ import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import { IPlatformService } from '../../../client/common/platform/types'; import { TerminalService } from '../../../client/common/terminal/service'; import { ITerminalActivator, ITerminalHelper, TerminalShellType } from '../../../client/common/terminal/types'; -import { IDisposable, IDisposableRegistry } from '../../../client/common/types'; +import { IDisposableRegistry } from '../../../client/common/types'; import { IServiceContainer } from '../../../client/ioc/types'; import { ITerminalAutoActivation } from '../../../client/terminals/types'; import { createPythonInterpreter } from '../../utils/interpreters'; @@ -36,7 +36,7 @@ suite('Terminal Service', () => { let mockServiceContainer: TypeMoq.IMock; let terminalAutoActivator: TypeMoq.IMock; let terminalShellIntegration: TypeMoq.IMock; - let executeCommandListeners: Disposable[] = []; + setup(() => { terminal = TypeMoq.Mock.ofType(); terminalShellIntegration = TypeMoq.Mock.ofType(); @@ -57,8 +57,6 @@ suite('Terminal Service', () => { onDidEndTerminalShellExecutionEmitter.event(handler); }); - executeCommandListeners = []; - platformService = TypeMoq.Mock.ofType(); workspaceService = TypeMoq.Mock.ofType(); terminalHelper = TypeMoq.Mock.ofType(); From 2375490fed656725f2bc5337780bfd9b4d560b4b Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 17:37:17 -0700 Subject: [PATCH 17/37] slowly migrate test from sendText to executeCommand --- .../terminals/codeExecution/terminalCodeExec.unit.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts b/src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts index 4f60adb3b931..4b5537f515d2 100644 --- a/src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts +++ b/src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts @@ -643,10 +643,10 @@ suite('Terminal - Code Execution', () => { terminalSettings.setup((t) => t.launchArgs).returns(() => terminalArgs); await executor.execute('cmd1'); - terminalService.verify(async (t) => t.sendText('cmd1'), TypeMoq.Times.once()); + terminalService.verify(async (t) => t.executeCommand('cmd1'), TypeMoq.Times.once()); await executor.execute('cmd2'); - terminalService.verify(async (t) => t.sendText('cmd2'), TypeMoq.Times.once()); + terminalService.verify(async (t) => t.executeCommand('cmd2'), TypeMoq.Times.once()); }); test('Ensure code is sent to the same terminal for a particular resource', async () => { @@ -668,10 +668,10 @@ suite('Terminal - Code Execution', () => { terminalSettings.setup((t) => t.launchArgs).returns(() => terminalArgs); await executor.execute('cmd1', resource); - terminalService.verify(async (t) => t.sendText('cmd1'), TypeMoq.Times.once()); + terminalService.verify(async (t) => t.executeCommand('cmd1'), TypeMoq.Times.once()); await executor.execute('cmd2', resource); - terminalService.verify(async (t) => t.sendText('cmd2'), TypeMoq.Times.once()); + terminalService.verify(async (t) => t.executeCommand('cmd2'), TypeMoq.Times.once()); }); }); }); From a75656b759fed7c1720032654cb21abb12406587 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 18:30:59 -0700 Subject: [PATCH 18/37] TODO: test for when shellIntegration is active, mock and fire onDidEndTerminalShellExecution --- src/client/common/terminal/service.ts | 5 +++- .../common/terminals/service.unit.test.ts | 28 ++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index 8a3b9b114e05..63cedd404e7b 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -108,7 +108,10 @@ export class TerminalService implements ITerminalService, Disposable { resolve({ execution, exitCode: e.exitCode }); } }); - this.executeCommandListeners.add(listener); + if (listener) { + this.executeCommandListeners.add(listener); + } + // this.executeCommandListeners.add(listener); }); } else { terminal.sendText(commandLine); diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index 2d45fec932fa..cb6c2c00fc94 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -36,7 +36,8 @@ suite('Terminal Service', () => { let mockServiceContainer: TypeMoq.IMock; let terminalAutoActivator: TypeMoq.IMock; let terminalShellIntegration: TypeMoq.IMock; - + // let onDidEndTerminalShellExecutionEmitter: EventEmitter; + // let event: TerminalShellExecutionEndEvent; setup(() => { terminal = TypeMoq.Mock.ofType(); terminalShellIntegration = TypeMoq.Mock.ofType(); @@ -57,6 +58,30 @@ suite('Terminal Service', () => { onDidEndTerminalShellExecutionEmitter.event(handler); }); + // const execution: TerminalShellExecution = { + // commandLine: { + // value: 'dummy text', + // isTrusted: true, + // confidence: 2, + // }, + // cwd: undefined, + // read: function (): AsyncIterable { + // throw new Error('Function not implemented.'); + // }, + // }; + // const exitCode = 0; + + // // Mock the execution object and exitCode + // event = { + // execution, + // exitCode, + // terminal: terminal.object, + // shellIntegration: terminalShellIntegration.object, + // }; + + // // Trigger the event + // onDidEndTerminalShellExecutionEmitter.fire(event); + platformService = TypeMoq.Mock.ofType(); workspaceService = TypeMoq.Mock.ofType(); terminalHelper = TypeMoq.Mock.ofType(); @@ -104,6 +129,7 @@ suite('Terminal Service', () => { .setup((h) => h.buildCommandForTerminal(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(() => 'dummy text'); + // onDidEndTerminalShellExecutionEmitter.fire(event); // Sending a command will cause the terminal to be created await service.sendCommand('', []); From a51b6b475fca0dd001d3744822f23c1292bb8ced Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 18:45:45 -0700 Subject: [PATCH 19/37] onDidEndTerminalShellExecution never gets fired --- .../common/terminals/service.unit.test.ts | 64 +++++++++---------- 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index cb6c2c00fc94..b8cbdd220414 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -36,13 +36,13 @@ suite('Terminal Service', () => { let mockServiceContainer: TypeMoq.IMock; let terminalAutoActivator: TypeMoq.IMock; let terminalShellIntegration: TypeMoq.IMock; - // let onDidEndTerminalShellExecutionEmitter: EventEmitter; - // let event: TerminalShellExecutionEndEvent; + let onDidEndTerminalShellExecutionEmitter: EventEmitter; + let event: TerminalShellExecutionEndEvent; setup(() => { terminal = TypeMoq.Mock.ofType(); terminalShellIntegration = TypeMoq.Mock.ofType(); - // terminal.setup((t) => t.shellIntegration).returns(() => terminalShellIntegration.object); - terminal.setup((t) => t.shellIntegration).returns(() => undefined); + terminal.setup((t) => t.shellIntegration).returns(() => terminalShellIntegration.object); + // terminal.setup((t) => t.shellIntegration).returns(() => undefined); const shellExecution: TypeMoq.IMock = TypeMoq.Mock.ofType(); terminalShellIntegration @@ -51,36 +51,34 @@ suite('Terminal Service', () => { terminalManager = TypeMoq.Mock.ofType(); // terminalManager.setup((t) => t.onDidEndTerminalShellExecution(TypeMoq.It.isAny())) - const onDidEndTerminalShellExecutionEmitter = new EventEmitter(); + onDidEndTerminalShellExecutionEmitter = new EventEmitter(); terminalManager - .setup((t) => t.onDidEndTerminalShellExecution(TypeMoq.It.isAny())) - .callback((handler) => { - onDidEndTerminalShellExecutionEmitter.event(handler); - }); - - // const execution: TerminalShellExecution = { - // commandLine: { - // value: 'dummy text', - // isTrusted: true, - // confidence: 2, - // }, - // cwd: undefined, - // read: function (): AsyncIterable { - // throw new Error('Function not implemented.'); - // }, - // }; - // const exitCode = 0; - - // // Mock the execution object and exitCode - // event = { - // execution, - // exitCode, - // terminal: terminal.object, - // shellIntegration: terminalShellIntegration.object, - // }; - - // // Trigger the event - // onDidEndTerminalShellExecutionEmitter.fire(event); + .setup((t) => t.onDidEndTerminalShellExecution) + .returns(() => onDidEndTerminalShellExecutionEmitter.event); + + const execution: TerminalShellExecution = { + commandLine: { + value: 'dummy text', + isTrusted: true, + confidence: 2, + }, + cwd: undefined, + read: function (): AsyncIterable { + throw new Error('Function not implemented.'); + }, + }; + const exitCode = 0; + + // Mock the execution object and exitCode + event = { + execution, + exitCode, + terminal: terminal.object, + shellIntegration: terminalShellIntegration.object, + }; + + // Trigger the event + onDidEndTerminalShellExecutionEmitter.fire(event); platformService = TypeMoq.Mock.ofType(); workspaceService = TypeMoq.Mock.ofType(); From 7c01537888b9149eeb76c7da0a421c639725d364 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 21:36:32 -0700 Subject: [PATCH 20/37] switch up the order --- src/client/common/terminal/service.ts | 1 - src/test/common/terminals/service.unit.test.ts | 11 +++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index 63cedd404e7b..d218fe9dd54e 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -111,7 +111,6 @@ export class TerminalService implements ITerminalService, Disposable { if (listener) { this.executeCommandListeners.add(listener); } - // this.executeCommandListeners.add(listener); }); } else { terminal.sendText(commandLine); diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index b8cbdd220414..6e1086ad7e86 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -50,12 +50,8 @@ suite('Terminal Service', () => { .returns(() => shellExecution.object); terminalManager = TypeMoq.Mock.ofType(); - // terminalManager.setup((t) => t.onDidEndTerminalShellExecution(TypeMoq.It.isAny())) - onDidEndTerminalShellExecutionEmitter = new EventEmitter(); - terminalManager - .setup((t) => t.onDidEndTerminalShellExecution) - .returns(() => onDidEndTerminalShellExecutionEmitter.event); + onDidEndTerminalShellExecutionEmitter = new EventEmitter(); const execution: TerminalShellExecution = { commandLine: { value: 'dummy text', @@ -76,6 +72,9 @@ suite('Terminal Service', () => { terminal: terminal.object, shellIntegration: terminalShellIntegration.object, }; + terminalManager + .setup((t) => t.onDidEndTerminalShellExecution) + .returns(() => onDidEndTerminalShellExecutionEmitter.event); // Trigger the event onDidEndTerminalShellExecutionEmitter.fire(event); @@ -127,7 +126,7 @@ suite('Terminal Service', () => { .setup((h) => h.buildCommandForTerminal(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(() => 'dummy text'); - // onDidEndTerminalShellExecutionEmitter.fire(event); + onDidEndTerminalShellExecutionEmitter.fire(event); // Sending a command will cause the terminal to be created await service.sendCommand('', []); From 923df5c13ef51b501d42db030f3b76c5e2391e12 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 22:44:04 -0700 Subject: [PATCH 21/37] add test onDidEndTerminalShellExecutionEmitter.fire(event) --- .../common/terminals/service.unit.test.ts | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index 6e1086ad7e86..ee0bf8384aeb 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -52,6 +52,7 @@ suite('Terminal Service', () => { terminalManager = TypeMoq.Mock.ofType(); onDidEndTerminalShellExecutionEmitter = new EventEmitter(); + const execution: TerminalShellExecution = { commandLine: { value: 'dummy text', @@ -72,10 +73,21 @@ suite('Terminal Service', () => { terminal: terminal.object, shellIntegration: terminalShellIntegration.object, }; + terminalManager .setup((t) => t.onDidEndTerminalShellExecution) .returns(() => onDidEndTerminalShellExecutionEmitter.event); + // Add a listener to capture the event argument + onDidEndTerminalShellExecutionEmitter.event((e) => { + try { + expect(e.execution).to.equal(execution); + expect(e.exitCode).to.equal(exitCode); + } catch (error) { + console.error(error); + } + }); + // Trigger the event onDidEndTerminalShellExecutionEmitter.fire(event); @@ -103,6 +115,24 @@ suite('Terminal Service', () => { disposables.filter((item) => !!item).forEach((item) => item.dispose()); }); + test('Ensure terminal shell execution end event is handled', (done) => { + service = new TerminalService(mockServiceContainer.object); + + // Add a listener to capture the event argument + onDidEndTerminalShellExecutionEmitter.event((e) => { + try { + expect(e.execution).to.equal(event.execution); + expect(e.exitCode).to.equal(event.exitCode); + done(); + } catch (error) { + done(error); + } + }); + + // Trigger the event + onDidEndTerminalShellExecutionEmitter.fire(event); + }); + test('Ensure terminal is disposed', async () => { terminalHelper .setup((helper) => helper.getEnvironmentActivationCommands(TypeMoq.It.isAny(), TypeMoq.It.isAny())) From 16839c9092ce70d0ede1cd6c7741c05324fa1820 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 23:36:28 -0700 Subject: [PATCH 22/37] attach callback to executeCommand so onDidEndTerminalShellExecutionEmitter fires --- src/client/common/terminal/service.ts | 3 ++ .../common/terminals/service.unit.test.ts | 29 +++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index d218fe9dd54e..fbbc4f1d8091 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -111,6 +111,9 @@ export class TerminalService implements ITerminalService, Disposable { if (listener) { this.executeCommandListeners.add(listener); } + // setTimeout(() => { + // resolve(undefined); + // }, 10000); // This would be a work around for testing scenario -- would it be a good design outside of testing case? }); } else { terminal.sendText(commandLine); diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index ee0bf8384aeb..b6f88e7e4dbb 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -42,11 +42,34 @@ suite('Terminal Service', () => { terminal = TypeMoq.Mock.ofType(); terminalShellIntegration = TypeMoq.Mock.ofType(); terminal.setup((t) => t.shellIntegration).returns(() => terminalShellIntegration.object); - // terminal.setup((t) => t.shellIntegration).returns(() => undefined); + // terminal.setup((t) => t.shellIntegration).returns(() => undefined); -- always disable shell integration => passes test const shellExecution: TypeMoq.IMock = TypeMoq.Mock.ofType(); terminalShellIntegration .setup((t) => t.executeCommand(TypeMoq.It.isAny())) + .callback(() => { + const execution: TerminalShellExecution = { + commandLine: { + value: 'dummy text', + isTrusted: true, + confidence: 2, + }, + cwd: undefined, + read: function (): AsyncIterable { + throw new Error('Function not implemented.'); + }, + }; + const exitCode = 0; + + const event: TerminalShellExecutionEndEvent = { + execution, + exitCode, + terminal: terminal.object, + shellIntegration: terminalShellIntegration.object, + }; + + onDidEndTerminalShellExecutionEmitter.fire(event); + }) .returns(() => shellExecution.object); terminalManager = TypeMoq.Mock.ofType(); @@ -81,8 +104,10 @@ suite('Terminal Service', () => { // Add a listener to capture the event argument onDidEndTerminalShellExecutionEmitter.event((e) => { try { - expect(e.execution).to.equal(execution); + expect(e.execution.commandLine.value).to.equal(execution.commandLine.value); expect(e.exitCode).to.equal(exitCode); + // resolve + return; } catch (error) { console.error(error); } From e30bb7741925a2b4f19d291f0fc03c2c64315134 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 10 Sep 2024 23:47:54 -0700 Subject: [PATCH 23/37] switch up the order abit + comment --- src/client/common/terminal/service.ts | 3 +- .../common/terminals/service.unit.test.ts | 66 +++++++++---------- 2 files changed, 34 insertions(+), 35 deletions(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index fbbc4f1d8091..3b52a7f45dc7 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -113,7 +113,8 @@ export class TerminalService implements ITerminalService, Disposable { } // setTimeout(() => { // resolve(undefined); - // }, 10000); // This would be a work around for testing scenario -- would it be a good design outside of testing case? + // }, 10000); // This would be a work around for testing scenario & safeguard for when onDidEndTerminalShellExecution never gets fired + // -- would it be a good design outside of testing case? }); } else { terminal.sendText(commandLine); diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index b6f88e7e4dbb..b151d47a6ee9 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -45,37 +45,8 @@ suite('Terminal Service', () => { // terminal.setup((t) => t.shellIntegration).returns(() => undefined); -- always disable shell integration => passes test const shellExecution: TypeMoq.IMock = TypeMoq.Mock.ofType(); - terminalShellIntegration - .setup((t) => t.executeCommand(TypeMoq.It.isAny())) - .callback(() => { - const execution: TerminalShellExecution = { - commandLine: { - value: 'dummy text', - isTrusted: true, - confidence: 2, - }, - cwd: undefined, - read: function (): AsyncIterable { - throw new Error('Function not implemented.'); - }, - }; - const exitCode = 0; - - const event: TerminalShellExecutionEndEvent = { - execution, - exitCode, - terminal: terminal.object, - shellIntegration: terminalShellIntegration.object, - }; - - onDidEndTerminalShellExecutionEmitter.fire(event); - }) - .returns(() => shellExecution.object); - - terminalManager = TypeMoq.Mock.ofType(); - onDidEndTerminalShellExecutionEmitter = new EventEmitter(); - + terminalManager = TypeMoq.Mock.ofType(); const execution: TerminalShellExecution = { commandLine: { value: 'dummy text', @@ -97,10 +68,6 @@ suite('Terminal Service', () => { shellIntegration: terminalShellIntegration.object, }; - terminalManager - .setup((t) => t.onDidEndTerminalShellExecution) - .returns(() => onDidEndTerminalShellExecutionEmitter.event); - // Add a listener to capture the event argument onDidEndTerminalShellExecutionEmitter.event((e) => { try { @@ -113,6 +80,37 @@ suite('Terminal Service', () => { } }); + terminalManager + .setup((t) => t.onDidEndTerminalShellExecution) + .returns(() => onDidEndTerminalShellExecutionEmitter.event); + + terminalShellIntegration + .setup((t) => t.executeCommand(TypeMoq.It.isAny())) + .callback(() => { + const execution: TerminalShellExecution = { + commandLine: { + value: 'dummy text', + isTrusted: true, + confidence: 2, + }, + cwd: undefined, + read: function (): AsyncIterable { + throw new Error('Function not implemented.'); + }, + }; + const exitCode = 0; + + const event: TerminalShellExecutionEndEvent = { + execution, + exitCode, + terminal: terminal.object, + shellIntegration: terminalShellIntegration.object, + }; + + onDidEndTerminalShellExecutionEmitter.fire(event); + }) + .returns(() => shellExecution.object); + // Trigger the event onDidEndTerminalShellExecutionEmitter.fire(event); From e69c49071c50160a626a817a4db99e8666f96eda Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 11 Sep 2024 10:39:21 -0700 Subject: [PATCH 24/37] wow --- .../common/terminals/service.unit.test.ts | 71 +++++++------------ ...rminalEnvVarCollectionService.unit.test.ts | 2 +- 2 files changed, 26 insertions(+), 47 deletions(-) diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index b151d47a6ee9..d1c2606c01f9 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -38,12 +38,13 @@ suite('Terminal Service', () => { let terminalShellIntegration: TypeMoq.IMock; let onDidEndTerminalShellExecutionEmitter: EventEmitter; let event: TerminalShellExecutionEndEvent; + let shellExecution: TypeMoq.IMock; setup(() => { terminal = TypeMoq.Mock.ofType(); terminalShellIntegration = TypeMoq.Mock.ofType(); terminal.setup((t) => t.shellIntegration).returns(() => terminalShellIntegration.object); // terminal.setup((t) => t.shellIntegration).returns(() => undefined); -- always disable shell integration => passes test - const shellExecution: TypeMoq.IMock = TypeMoq.Mock.ofType(); + shellExecution = TypeMoq.Mock.ofType(); onDidEndTerminalShellExecutionEmitter = new EventEmitter(); terminalManager = TypeMoq.Mock.ofType(); @@ -58,62 +59,35 @@ suite('Terminal Service', () => { throw new Error('Function not implemented.'); }, }; - const exitCode = 0; // Mock the execution object and exitCode event = { execution, - exitCode, + exitCode: 0, terminal: terminal.object, shellIntegration: terminalShellIntegration.object, }; // Add a listener to capture the event argument - onDidEndTerminalShellExecutionEmitter.event((e) => { - try { - expect(e.execution.commandLine.value).to.equal(execution.commandLine.value); - expect(e.exitCode).to.equal(exitCode); - // resolve - return; - } catch (error) { - console.error(error); - } - }); + // onDidEndTerminalShellExecutionEmitter.event((e) => { + // try { + // expect(e.execution.commandLine.value).to.equal(execution.commandLine.value); + // expect(e.exitCode).to.equal(exitCode); + // // resolve + // return; + // } catch (error) { + // console.error(error); + // } + // }); + + terminalShellIntegration.setup((t) => t.executeCommand(TypeMoq.It.isAny())).returns(() => execution); terminalManager .setup((t) => t.onDidEndTerminalShellExecution) - .returns(() => onDidEndTerminalShellExecutionEmitter.event); - - terminalShellIntegration - .setup((t) => t.executeCommand(TypeMoq.It.isAny())) - .callback(() => { - const execution: TerminalShellExecution = { - commandLine: { - value: 'dummy text', - isTrusted: true, - confidence: 2, - }, - cwd: undefined, - read: function (): AsyncIterable { - throw new Error('Function not implemented.'); - }, - }; - const exitCode = 0; - - const event: TerminalShellExecutionEndEvent = { - execution, - exitCode, - terminal: terminal.object, - shellIntegration: terminalShellIntegration.object, - }; - - onDidEndTerminalShellExecutionEmitter.fire(event); - }) - .returns(() => shellExecution.object); - - // Trigger the event - onDidEndTerminalShellExecutionEmitter.fire(event); - + .returns(() => { + setTimeout(() => onDidEndTerminalShellExecutionEmitter.fire(event), 100); + return onDidEndTerminalShellExecutionEmitter.event; + }); platformService = TypeMoq.Mock.ofType(); workspaceService = TypeMoq.Mock.ofType(); terminalHelper = TypeMoq.Mock.ofType(); @@ -179,7 +153,12 @@ suite('Terminal Service', () => { .setup((h) => h.buildCommandForTerminal(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(() => 'dummy text'); - onDidEndTerminalShellExecutionEmitter.fire(event); + terminalManager + .setup((t) => t.onDidEndTerminalShellExecution) + .returns(() => { + setTimeout(() => onDidEndTerminalShellExecutionEmitter.fire(event), 100); + return onDidEndTerminalShellExecutionEmitter.event; + }); // Sending a command will cause the terminal to be created await service.sendCommand('', []); diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index 6d3d7ffd8c74..5d1027d12702 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -146,7 +146,7 @@ suite('Terminal Environment Variable Collection Service', () => { verify(applicationEnvironment.onDidChangeShell(anything(), anything(), anything())).never(); assert(applyCollectionStub.notCalled, 'Collection should not be applied on activation'); - verify(collection.clear()).atLeast(1); + verify(globalCollection.clear()).atLeast(1); }); test('When interpreter changes, apply new activated variables to the collection', async () => { From e2e71d0ae4952af3bf88556a0373d7704d1ba34c Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 11 Sep 2024 10:42:10 -0700 Subject: [PATCH 25/37] remove junk --- src/test/common/terminals/service.unit.test.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index d1c2606c01f9..537fb5f78927 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -68,18 +68,6 @@ suite('Terminal Service', () => { shellIntegration: terminalShellIntegration.object, }; - // Add a listener to capture the event argument - // onDidEndTerminalShellExecutionEmitter.event((e) => { - // try { - // expect(e.execution.commandLine.value).to.equal(execution.commandLine.value); - // expect(e.exitCode).to.equal(exitCode); - // // resolve - // return; - // } catch (error) { - // console.error(error); - // } - // }); - terminalShellIntegration.setup((t) => t.executeCommand(TypeMoq.It.isAny())).returns(() => execution); terminalManager From 3d31d1a981a37b683cb3cc2128089f0f207fece3 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 11 Sep 2024 10:47:01 -0700 Subject: [PATCH 26/37] remove unused --- src/test/common/terminals/service.unit.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index 537fb5f78927..73c6ae994aa9 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -38,13 +38,12 @@ suite('Terminal Service', () => { let terminalShellIntegration: TypeMoq.IMock; let onDidEndTerminalShellExecutionEmitter: EventEmitter; let event: TerminalShellExecutionEndEvent; - let shellExecution: TypeMoq.IMock; + setup(() => { terminal = TypeMoq.Mock.ofType(); terminalShellIntegration = TypeMoq.Mock.ofType(); terminal.setup((t) => t.shellIntegration).returns(() => terminalShellIntegration.object); // terminal.setup((t) => t.shellIntegration).returns(() => undefined); -- always disable shell integration => passes test - shellExecution = TypeMoq.Mock.ofType(); onDidEndTerminalShellExecutionEmitter = new EventEmitter(); terminalManager = TypeMoq.Mock.ofType(); @@ -174,7 +173,7 @@ suite('Terminal Service', () => { terminal.verify((t) => t.show(TypeMoq.It.isValue(true)), TypeMoq.Times.atLeastOnce()); terminal.verify( (t) => t.sendText(TypeMoq.It.isValue(commandToExpect), TypeMoq.It.isValue(true)), - TypeMoq.Times.never(), // should be calling sendText but rather executeCommand() + TypeMoq.Times.never(), ); }); From 46d28d29035ccf0935dbeab0fd3f0ba589e5418b Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 11 Sep 2024 11:23:40 -0700 Subject: [PATCH 27/37] remove comment --- src/client/common/terminal/service.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index 3b52a7f45dc7..d218fe9dd54e 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -111,10 +111,6 @@ export class TerminalService implements ITerminalService, Disposable { if (listener) { this.executeCommandListeners.add(listener); } - // setTimeout(() => { - // resolve(undefined); - // }, 10000); // This would be a work around for testing scenario & safeguard for when onDidEndTerminalShellExecution never gets fired - // -- would it be a good design outside of testing case? }); } else { terminal.sendText(commandLine); From 5d3878f9046360da622b28c265b459de340a758a Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 11 Sep 2024 11:56:10 -0700 Subject: [PATCH 28/37] TODO: smart send smoke test are flaky on windows --- src/test/smoke/smartSend.smoke.test.ts | 131 +++++++++++++------------ 1 file changed, 67 insertions(+), 64 deletions(-) diff --git a/src/test/smoke/smartSend.smoke.test.ts b/src/test/smoke/smartSend.smoke.test.ts index a35c02ceaa63..0cf27c7dd2e0 100644 --- a/src/test/smoke/smartSend.smoke.test.ts +++ b/src/test/smoke/smartSend.smoke.test.ts @@ -6,78 +6,81 @@ import { EXTENSION_ROOT_DIR_FOR_TESTS, IS_SMOKE_TEST } from '../constants'; import { closeActiveWindows, initialize, initializeTest } from '../initialize'; import { openFile, waitForCondition } from '../common'; -suite('Smoke Test: Run Smart Selection and Advance Cursor', () => { - suiteSetup(async function () { - if (!IS_SMOKE_TEST) { - return this.skip(); - } - await initialize(); - return undefined; - }); +// TODO: This test is being flaky for windows, need to investigate why only fails on windows +if (process.platform === 'win32') { + suite('Smoke Test: Run Smart Selection and Advance Cursor', () => { + suiteSetup(async function () { + if (!IS_SMOKE_TEST) { + return this.skip(); + } + await initialize(); + return undefined; + }); - setup(initializeTest); - suiteTeardown(closeActiveWindows); - teardown(closeActiveWindows); + setup(initializeTest); + suiteTeardown(closeActiveWindows); + teardown(closeActiveWindows); - test('Smart Send', async () => { - const file = path.join( - EXTENSION_ROOT_DIR_FOR_TESTS, - 'src', - 'testMultiRootWkspc', - 'smokeTests', - 'create_delete_file.py', - ); - const outputFile = path.join( - EXTENSION_ROOT_DIR_FOR_TESTS, - 'src', - 'testMultiRootWkspc', - 'smokeTests', - 'smart_send_smoke.txt', - ); + test('Smart Send', async () => { + const file = path.join( + EXTENSION_ROOT_DIR_FOR_TESTS, + 'src', + 'testMultiRootWkspc', + 'smokeTests', + 'create_delete_file.py', + ); + const outputFile = path.join( + EXTENSION_ROOT_DIR_FOR_TESTS, + 'src', + 'testMultiRootWkspc', + 'smokeTests', + 'smart_send_smoke.txt', + ); - await fs.remove(outputFile); + await fs.remove(outputFile); - const textDocument = await openFile(file); + const textDocument = await openFile(file); - if (vscode.window.activeTextEditor) { - const myPos = new vscode.Position(0, 0); - vscode.window.activeTextEditor!.selections = [new vscode.Selection(myPos, myPos)]; - } - await vscode.commands - .executeCommand('python.execSelectionInTerminal', textDocument.uri) - .then(undefined, (err) => { - assert.fail(`Something went wrong running the Python file in the terminal: ${err}`); - }); + if (vscode.window.activeTextEditor) { + const myPos = new vscode.Position(0, 0); + vscode.window.activeTextEditor!.selections = [new vscode.Selection(myPos, myPos)]; + } + await vscode.commands + .executeCommand('python.execSelectionInTerminal', textDocument.uri) + .then(undefined, (err) => { + assert.fail(`Something went wrong running the Python file in the terminal: ${err}`); + }); - const checkIfFileHasBeenCreated = () => fs.pathExists(outputFile); - await waitForCondition(checkIfFileHasBeenCreated, 10_000, `"${outputFile}" file not created`); + const checkIfFileHasBeenCreated = () => fs.pathExists(outputFile); + await waitForCondition(checkIfFileHasBeenCreated, 10_000, `"${outputFile}" file not created`); - await vscode.commands - .executeCommand('python.execSelectionInTerminal', textDocument.uri) - .then(undefined, (err) => { - assert.fail(`Something went wrong running the Python file in the terminal: ${err}`); - }); - await vscode.commands - .executeCommand('python.execSelectionInTerminal', textDocument.uri) - .then(undefined, (err) => { - assert.fail(`Something went wrong running the Python file in the terminal: ${err}`); - }); + await vscode.commands + .executeCommand('python.execSelectionInTerminal', textDocument.uri) + .then(undefined, (err) => { + assert.fail(`Something went wrong running the Python file in the terminal: ${err}`); + }); + await vscode.commands + .executeCommand('python.execSelectionInTerminal', textDocument.uri) + .then(undefined, (err) => { + assert.fail(`Something went wrong running the Python file in the terminal: ${err}`); + }); - async function wait() { - return new Promise((resolve) => { - setTimeout(() => { - resolve(); - }, 10000); - }); - } + async function wait() { + return new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, 10000); + }); + } - await wait(); + await wait(); - const deletedFile = !(await fs.pathExists(outputFile)); - if (deletedFile) { - assert.ok(true, `"${outputFile}" file has been deleted`); - } else { - assert.fail(`"${outputFile}" file still exists`); - } + const deletedFile = !(await fs.pathExists(outputFile)); + if (deletedFile) { + assert.ok(true, `"${outputFile}" file has been deleted`); + } else { + assert.fail(`"${outputFile}" file still exists`); + } + }); }); -}); +} From dfe61dee28b931979902e1243b37bd2508d67348 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 11 Sep 2024 11:56:46 -0700 Subject: [PATCH 29/37] a --- src/test/smoke/smartSend.smoke.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/smoke/smartSend.smoke.test.ts b/src/test/smoke/smartSend.smoke.test.ts index 0cf27c7dd2e0..7f894df923ee 100644 --- a/src/test/smoke/smartSend.smoke.test.ts +++ b/src/test/smoke/smartSend.smoke.test.ts @@ -7,7 +7,7 @@ import { closeActiveWindows, initialize, initializeTest } from '../initialize'; import { openFile, waitForCondition } from '../common'; // TODO: This test is being flaky for windows, need to investigate why only fails on windows -if (process.platform === 'win32') { +if (process.platform !== 'win32') { suite('Smoke Test: Run Smart Selection and Advance Cursor', () => { suiteSetup(async function () { if (!IS_SMOKE_TEST) { From 8f9ab0dc951af27a9742bf6ab9a95ed83b70c9f1 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 11 Sep 2024 18:08:22 -0700 Subject: [PATCH 30/37] take recommended feedbacks --- src/client/common/terminal/service.ts | 9 +++++---- .../common/terminals/service.unit.test.ts | 19 ------------------- 2 files changed, 5 insertions(+), 23 deletions(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index d218fe9dd54e..9d1bd3971acb 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -22,6 +22,7 @@ import { TerminalShellType, ITerminalExecutedCommand, } from './types'; +import { TIMEOUT } from 'dns'; @injectable() export class TerminalService implements ITerminalService, Disposable { @@ -53,9 +54,9 @@ export class TerminalService implements ITerminalService, Disposable { this.terminal?.dispose(); if (this.executeCommandListeners && this.executeCommandListeners.size > 0) { - for (const d of this.executeCommandListeners) { + this.executeCommandListeners.forEach((d) => { d?.dispose(); - } + }); } } public async sendCommand(command: string, args: string[], _?: CancellationToken): Promise { @@ -76,7 +77,6 @@ export class TerminalService implements ITerminalService, Disposable { this.terminal!.sendText(text); } public async executeCommand(commandLine: string): Promise { - // const terminal = await this.ensureTerminal(); const terminal = this.terminal!; if (!this.options?.hideFromUser) { terminal.show(true); @@ -91,10 +91,11 @@ export class TerminalService implements ITerminalService, Disposable { resolve(true); }, ); + const TIMEOUT_DURATION = 3000; setTimeout(() => { this.executeCommandListeners.add(shellIntegrationChangeEventListener); resolve(true); - }, 3000); + }, TIMEOUT_DURATION); }); await promise; } diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index 73c6ae994aa9..3cf13189a267 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -59,7 +59,6 @@ suite('Terminal Service', () => { }, }; - // Mock the execution object and exitCode event = { execution, exitCode: 0, @@ -99,24 +98,6 @@ suite('Terminal Service', () => { disposables.filter((item) => !!item).forEach((item) => item.dispose()); }); - test('Ensure terminal shell execution end event is handled', (done) => { - service = new TerminalService(mockServiceContainer.object); - - // Add a listener to capture the event argument - onDidEndTerminalShellExecutionEmitter.event((e) => { - try { - expect(e.execution).to.equal(event.execution); - expect(e.exitCode).to.equal(event.exitCode); - done(); - } catch (error) { - done(error); - } - }); - - // Trigger the event - onDidEndTerminalShellExecutionEmitter.fire(event); - }); - test('Ensure terminal is disposed', async () => { terminalHelper .setup((helper) => helper.getEnvironmentActivationCommands(TypeMoq.It.isAny(), TypeMoq.It.isAny())) From 6d095c8cb7ab9242a7d9272b06ffb2ece45b1d64 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 11 Sep 2024 18:10:20 -0700 Subject: [PATCH 31/37] remove leftover comments - done --- src/client/common/terminal/service.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index 9d1bd3971acb..908160ee045e 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -129,7 +129,6 @@ export class TerminalService implements ITerminalService, Disposable { // TODO: Debt switch to Promise ---> breaks 20 tests public async ensureTerminal(preserveFocus: boolean = true): Promise { if (this.terminal) { - // return this.terminal; return; } this.terminalShellType = this.terminalHelper.identifyTerminalShell(this.terminal); From 563fb12f0f3a2df7fb7aa410e5945f740dce3262 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 11 Sep 2024 18:10:56 -0700 Subject: [PATCH 32/37] final --- src/client/common/terminal/service.ts | 1 - src/test/common/terminals/service.unit.test.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index 908160ee045e..c824438183c1 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -154,7 +154,6 @@ export class TerminalService implements ITerminalService, Disposable { } this.sendTelemetry().ignoreErrors(); - // return this.terminal; return; } private terminalCloseHandler(terminal: Terminal) { diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index 3cf13189a267..b0e889745fd9 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -43,7 +43,6 @@ suite('Terminal Service', () => { terminal = TypeMoq.Mock.ofType(); terminalShellIntegration = TypeMoq.Mock.ofType(); terminal.setup((t) => t.shellIntegration).returns(() => terminalShellIntegration.object); - // terminal.setup((t) => t.shellIntegration).returns(() => undefined); -- always disable shell integration => passes test onDidEndTerminalShellExecutionEmitter = new EventEmitter(); terminalManager = TypeMoq.Mock.ofType(); From 145e2fd4c633f816bc550fc04e3176cb75dcbe44 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 11 Sep 2024 18:17:30 -0700 Subject: [PATCH 33/37] remove weird import --- src/client/common/terminal/service.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client/common/terminal/service.ts b/src/client/common/terminal/service.ts index c824438183c1..19cdf0aea0a1 100644 --- a/src/client/common/terminal/service.ts +++ b/src/client/common/terminal/service.ts @@ -22,7 +22,6 @@ import { TerminalShellType, ITerminalExecutedCommand, } from './types'; -import { TIMEOUT } from 'dns'; @injectable() export class TerminalService implements ITerminalService, Disposable { From 8bc39511f26cbd3edb00d5a84f92d8aeda8b1736 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Thu, 12 Sep 2024 10:45:48 -0700 Subject: [PATCH 34/37] last missed comment --- src/test/common/terminals/service.unit.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/common/terminals/service.unit.test.ts b/src/test/common/terminals/service.unit.test.ts index b0e889745fd9..f0754948a233 100644 --- a/src/test/common/terminals/service.unit.test.ts +++ b/src/test/common/terminals/service.unit.test.ts @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -// @ts-ignore: TS6133 import { expect } from 'chai'; import * as path from 'path'; import * as TypeMoq from 'typemoq'; From 610d0fb87d7b0c487ea5daece8473ca18f41ea8a Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Thu, 12 Sep 2024 11:27:24 -0700 Subject: [PATCH 35/37] upgrade actions/download-artifact@v4 --- .github/actions/smoke-tests/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/smoke-tests/action.yml b/.github/actions/smoke-tests/action.yml index cc2912115176..81cb7d2bc500 100644 --- a/.github/actions/smoke-tests/action.yml +++ b/.github/actions/smoke-tests/action.yml @@ -43,7 +43,7 @@ runs: # Bits from the VSIX are reused by smokeTest.ts to speed things up. - name: Download VSIX - uses: actions/download-artifact@v2 + uses: actions/download-artifact@v4 with: name: ${{ inputs.artifact_name }} From c6a4713d0f90cf98dec44cadcdff5514c1ed4fb7 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Thu, 12 Sep 2024 11:48:31 -0700 Subject: [PATCH 36/37] why --- .github/actions/smoke-tests/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/smoke-tests/action.yml b/.github/actions/smoke-tests/action.yml index 81cb7d2bc500..cc2912115176 100644 --- a/.github/actions/smoke-tests/action.yml +++ b/.github/actions/smoke-tests/action.yml @@ -43,7 +43,7 @@ runs: # Bits from the VSIX are reused by smokeTest.ts to speed things up. - name: Download VSIX - uses: actions/download-artifact@v4 + uses: actions/download-artifact@v2 with: name: ${{ inputs.artifact_name }} From 2b4c2bf29ad8138e76238e17a6cd21109ea2a312 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Thu, 12 Sep 2024 12:09:16 -0700 Subject: [PATCH 37/37] try with actions/download-artifact@v3 --- .github/actions/smoke-tests/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/smoke-tests/action.yml b/.github/actions/smoke-tests/action.yml index cc2912115176..d4ac73b1a803 100644 --- a/.github/actions/smoke-tests/action.yml +++ b/.github/actions/smoke-tests/action.yml @@ -43,7 +43,7 @@ runs: # Bits from the VSIX are reused by smokeTest.ts to speed things up. - name: Download VSIX - uses: actions/download-artifact@v2 + uses: actions/download-artifact@v3 with: name: ${{ inputs.artifact_name }}