Prevent first Python command being lost

src/client/terminals/codeExecution/djangoShellCodeExecution.ts

@@ -6,7 +6,12 @@
 import { inject, injectable } from 'inversify';
 import * as path from 'path';
 import { Disposable, Uri } from 'vscode';
-import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../common/application/types';
+import {
+    IApplicationShell,
+    ICommandManager,
+    IDocumentManager,
+    IWorkspaceService,
+} from '../../common/application/types';
 import '../../common/extensions';
 import { IFileSystem, IPlatformService } from '../../common/platform/types';
 import { ITerminalServiceFactory } from '../../common/terminal/types';
@@ -28,6 +33,7 @@ export class DjangoShellCodeExecutionProvider extends TerminalCodeExecutionProvi
         @inject(IFileSystem) fileSystem: IFileSystem,
         @inject(IDisposableRegistry) disposableRegistry: Disposable[],
         @inject(IInterpreterService) interpreterService: IInterpreterService,
+        @inject(IApplicationShell) applicationShell: IApplicationShell,
     ) {
         super(
             terminalServiceFactory,
@@ -37,6 +43,7 @@ export class DjangoShellCodeExecutionProvider extends TerminalCodeExecutionProvi
             platformService,
             interpreterService,
             commandManager,
+            applicationShell,
         );
         this.terminalTitle = 'Django Shell';
         disposableRegistry.push(new DjangoContextInitializer(documentManager, workspace, fileSystem, commandManager));

src/client/terminals/codeExecution/repl.ts

@@ -5,7 +5,7 @@
 
 import { inject, injectable } from 'inversify';
 import { Disposable } from 'vscode';
-import { ICommandManager, IWorkspaceService } from '../../common/application/types';
+import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../common/application/types';
 import { IPlatformService } from '../../common/platform/types';
 import { ITerminalServiceFactory } from '../../common/terminal/types';
 import { IConfigurationService, IDisposableRegistry } from '../../common/types';
@@ -22,6 +22,7 @@ export class ReplProvider extends TerminalCodeExecutionProvider {
         @inject(IPlatformService) platformService: IPlatformService,
         @inject(IInterpreterService) interpreterService: IInterpreterService,
         @inject(ICommandManager) commandManager: ICommandManager,
+        @inject(IApplicationShell) applicationShell: IApplicationShell,
     ) {
         super(
             terminalServiceFactory,
@@ -31,6 +32,7 @@ export class ReplProvider extends TerminalCodeExecutionProvider {
             platformService,
             interpreterService,
             commandManager,
+            applicationShell,
         );
         this.terminalTitle = 'REPL';
     }

src/client/terminals/codeExecution/terminalCodeExecution.ts

@@ -6,11 +6,11 @@
 import { inject, injectable } from 'inversify';
 import * as path from 'path';
 import { Disposable, Uri } from 'vscode';
-import { ICommandManager, IWorkspaceService } from '../../common/application/types';
+import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../common/application/types';
 import '../../common/extensions';
 import { IPlatformService } from '../../common/platform/types';
 import { ITerminalService, ITerminalServiceFactory } from '../../common/terminal/types';
-import { IConfigurationService, IDisposableRegistry, Resource } from '../../common/types';
+import { IConfigurationService, IDisposable, IDisposableRegistry, Resource } from '../../common/types';
 import { Diagnostics, Repl } from '../../common/utils/localize';
 import { showWarningMessage } from '../../common/vscodeApis/windowApis';
 import { IInterpreterService } from '../../interpreter/contracts';
@@ -30,6 +30,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
         @inject(IPlatformService) protected readonly platformService: IPlatformService,
         @inject(IInterpreterService) protected readonly interpreterService: IInterpreterService,
         @inject(ICommandManager) protected readonly commandManager: ICommandManager,
+        @inject(IApplicationShell) protected readonly applicationShell: IApplicationShell,
     ) {}
 
     public async executeFile(file: Uri, options?: { newTerminalPerFile: boolean }) {
@@ -65,10 +66,34 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
         }
         this.replActive = new Promise<boolean>(async (resolve) => {
             const replCommandArgs = await this.getExecutableInfo(resource);
+            let listener: IDisposable;
+            Promise.race([
+                new Promise<boolean>((resolve) => setTimeout(() => resolve(true), 3000)),
+                new Promise<boolean>((resolve) => {
+                    let count = 0;
+                    const terminalDataTimeout = setTimeout(() => {
+                        resolve(true); // Fall back for test case scenarios.
+                    }, 3000);
+                    // Watch TerminalData to see if REPL launched.
+                    listener = this.applicationShell.onDidWriteTerminalData((e) => {
+                        for (let i = 0; i < e.data.length; i++) {
+                            if (e.data[i] === '>') {
+                                count++;
+                                if (count === 3) {
+                                    clearTimeout(terminalDataTimeout);
+                                    resolve(true);
+                                }
+                            }
+                        }
+                    });
+                }),
+            ]).then(() => {
+                if (listener) {
+                    listener.dispose();
+                }
+                resolve(true);
+            });
             terminalService.sendCommand(replCommandArgs.command, replCommandArgs.args);
-
-            // Give python repl time to start before we start sending text.
-            setTimeout(() => resolve(true), 1000);
         });
         this.disposables.push(
             terminalService.onDidCloseTerminal(() => {

src/test/terminals/codeExecution/djangoShellCodeExect.unit.test.ts

@@ -6,7 +6,12 @@ import * as path from 'path';
 import * as TypeMoq from 'typemoq';
 import * as sinon from 'sinon';
 import { Disposable, Uri, WorkspaceFolder } from 'vscode';
-import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../../client/common/application/types';
+import {
+    IApplicationShell,
+    ICommandManager,
+    IDocumentManager,
+    IWorkspaceService,
+} from '../../../client/common/application/types';
 import { IFileSystem, IPlatformService } from '../../../client/common/platform/types';
 import { createCondaEnv } from '../../../client/common/process/pythonEnvironment';
 import { createPythonProcessService } from '../../../client/common/process/pythonProcess';
@@ -32,12 +37,14 @@ suite('Terminal - Django Shell Code Execution', () => {
     let settings: TypeMoq.IMock<IPythonSettings>;
     let interpreterService: TypeMoq.IMock<IInterpreterService>;
     let pythonExecutionFactory: TypeMoq.IMock<IPythonExecutionFactory>;
+    let applicationShell: TypeMoq.IMock<IApplicationShell>;
     let disposables: Disposable[] = [];
     setup(() => {
         const terminalFactory = TypeMoq.Mock.ofType<ITerminalServiceFactory>();
         terminalSettings = TypeMoq.Mock.ofType<ITerminalSettings>();
         terminalService = TypeMoq.Mock.ofType<ITerminalService>();
         const configService = TypeMoq.Mock.ofType<IConfigurationService>();
+        applicationShell = TypeMoq.Mock.ofType<IApplicationShell>();
         workspace = TypeMoq.Mock.ofType<IWorkspaceService>();
         workspace
             .setup((c) => c.onDidChangeWorkspaceFolders(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
@@ -62,6 +69,7 @@ suite('Terminal - Django Shell Code Execution', () => {
             fileSystem.object,
             disposables,
             interpreterService.object,
+            applicationShell.object,
         );
 
         terminalFactory.setup((f) => f.getTerminalService(TypeMoq.It.isAny())).returns(() => terminalService.object);

src/test/terminals/codeExecution/terminalCodeExec.unit.test.ts

@@ -6,7 +6,12 @@ import * as path from 'path';
 import { SemVer } from 'semver';
 import * as TypeMoq from 'typemoq';
 import { Disposable, Uri, WorkspaceFolder } from 'vscode';
-import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../../client/common/application/types';
+import {
+    IApplicationShell,
+    ICommandManager,
+    IDocumentManager,
+    IWorkspaceService,
+} from '../../../client/common/application/types';
 import { IFileSystem, IPlatformService } from '../../../client/common/platform/types';
 import { createCondaEnv } from '../../../client/common/process/pythonEnvironment';
 import { createPythonProcessService } from '../../../client/common/process/pythonProcess';
@@ -47,6 +52,7 @@ suite('Terminal - Code Execution', () => {
         let pythonExecutionFactory: TypeMoq.IMock<IPythonExecutionFactory>;
         let interpreterService: TypeMoq.IMock<IInterpreterService>;
         let isDjangoRepl: boolean;
+        let applicationShell: TypeMoq.IMock<IApplicationShell>;
 
         teardown(() => {
             disposables.forEach((disposable) => {
@@ -71,6 +77,7 @@ suite('Terminal - Code Execution', () => {
             fileSystem = TypeMoq.Mock.ofType<IFileSystem>();
             pythonExecutionFactory = TypeMoq.Mock.ofType<IPythonExecutionFactory>();
             interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
+            applicationShell = TypeMoq.Mock.ofType<IApplicationShell>();
             settings = TypeMoq.Mock.ofType<IPythonSettings>();
             settings.setup((s) => s.terminal).returns(() => terminalSettings.object);
             configService.setup((c) => c.getSettings(TypeMoq.It.isAny())).returns(() => settings.object);
@@ -85,6 +92,7 @@ suite('Terminal - Code Execution', () => {
                         platform.object,
                         interpreterService.object,
                         commandManager.object,
+                        applicationShell.object,
                     );
                     break;
                 }
@@ -97,6 +105,7 @@ suite('Terminal - Code Execution', () => {
                         platform.object,
                         interpreterService.object,
                         commandManager.object,
+                        applicationShell.object,
                     );
                     expectedTerminalTitle = 'REPL';
                     break;
@@ -120,6 +129,7 @@ suite('Terminal - Code Execution', () => {
                         fileSystem.object,
                         disposables,
                         interpreterService.object,
+                        applicationShell.object,
                     );
                     expectedTerminalTitle = 'Django Shell';
                     break;
@@ -590,7 +600,7 @@ suite('Terminal - Code Execution', () => {
                 );
             });
 
-            test('Ensure repl is re-initialized when terminal is closed', async () => {
+            test('Ensure REPL launches after reducing risk of command being ignored or duplicated', async () => {
                 const pythonPath = 'usr/bin/python1234';
                 const terminalArgs = ['-a', 'b', 'c'];
                 platform.setup((p) => p.isWindows).returns(() => false);
@@ -599,43 +609,27 @@ suite('Terminal - Code Execution', () => {
                     .returns(() => Promise.resolve(({ path: pythonPath } as unknown) as PythonEnvironment));
                 terminalSettings.setup((t) => t.launchArgs).returns(() => terminalArgs);
 
-                let closeTerminalCallback: undefined | (() => void);
-                terminalService
-                    .setup((t) => t.onDidCloseTerminal(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
-                    .returns((callback) => {
-                        closeTerminalCallback = callback;
-                        return {
-                            dispose: noop,
-                        };
-                    });
-
                 await executor.execute('cmd1');
                 await executor.execute('cmd2');
                 await executor.execute('cmd3');
 
-                const expectedTerminalArgs = isDjangoRepl ? terminalArgs.concat(['manage.py', 'shell']) : terminalArgs;
-
-                expect(closeTerminalCallback).not.to.be.an('undefined', 'Callback not initialized');
-                terminalService.verify(
-                    async (t) =>
-                        t.sendCommand(TypeMoq.It.isValue(pythonPath), TypeMoq.It.isValue(expectedTerminalArgs)),
-                    TypeMoq.Times.once(),
+                // Now check if sendCommand from the initializeRepl is called atLeastOnce.
+                // This is due to newly added Promise race and fallback to lower risk of swollen first command.
+                applicationShell.verify(
+                    async (t) => t.onDidWriteTerminalData(TypeMoq.It.isAny(), TypeMoq.It.isAny()),
+                    TypeMoq.Times.atLeastOnce(),
                 );
 
-                closeTerminalCallback!.call(terminalService.object);
                 await executor.execute('cmd4');
-                terminalService.verify(
-                    async (t) =>
-                        t.sendCommand(TypeMoq.It.isValue(pythonPath), TypeMoq.It.isValue(expectedTerminalArgs)),
-                    TypeMoq.Times.exactly(2),
+                applicationShell.verify(
+                    async (t) => t.onDidWriteTerminalData(TypeMoq.It.isAny(), TypeMoq.It.isAny()),
+                    TypeMoq.Times.atLeastOnce(),
                 );
 
-                closeTerminalCallback!.call(terminalService.object);
                 await executor.execute('cmd5');
-                terminalService.verify(
-                    async (t) =>
-                        t.sendCommand(TypeMoq.It.isValue(pythonPath), TypeMoq.It.isValue(expectedTerminalArgs)),
-                    TypeMoq.Times.exactly(3),
+                applicationShell.verify(
+                    async (t) => t.onDidWriteTerminalData(TypeMoq.It.isAny(), TypeMoq.It.isAny()),
+                    TypeMoq.Times.atLeastOnce(),
                 );
             });