Get windows tests to be able to pass (#9224)

* Fix tests to run again on windows

* Fix mimeTypes test too

* Fix the other failing mimetype

* Fix linter

* Fix export tests too

* Fix unit tests

* Fix the rest of the functional tests to pass too

* Fix linter

* Remove stub to fix other tests

Author: rchiodo

src/client/common/process/pythonDaemonPool.ts

@@ -41,15 +41,15 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
         private readonly activatedEnvVariables?: NodeJS.ProcessEnv,
         private readonly timeoutWaitingForDaemon: number = 1_000
     ) {
-        if (!options.pythonPath){
+        if (!options.pythonPath) {
             throw new Error('options.pythonPath is empty when it shoud not be');
         }
         this.pythonPath = options.pythonPath;
         // Setup environment variables for the daemon.
         // The daemon must have access to the Python Module that'll run the daemon
         // & also access to a Python package used for the JSON rpc comms.
         const envPythonPath = `${path.join(EXTENSION_ROOT_DIR, 'pythonFiles')}${path.delimiter}${path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python')}`;
-        this.envVariables = this.activatedEnvVariables ? { ...this.activatedEnvVariables } : {};
+        this.envVariables = this.activatedEnvVariables ? { ...this.activatedEnvVariables } : { ...process.env };
         this.envVariables.PYTHONPATH = this.envVariables.PYTHONPATH ? `${this.envVariables.PYTHONPATH}${path.delimiter}${envPythonPath}` : envPythonPath;
         this.envVariables.PYTHONUNBUFFERED = '1';
     }
@@ -65,34 +65,34 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
         noop();
     }
     public async getInterpreterInformation(): Promise<InterpreterInfomation | undefined> {
-        const msg = {args: ['GetPythonVersion']};
+        const msg = { args: ['GetPythonVersion'] };
         return this.wrapCall(daemon => daemon.getInterpreterInformation(), msg);
     }
     public async getExecutablePath(): Promise<string> {
-        const msg = {args: ['getExecutablePath']};
+        const msg = { args: ['getExecutablePath'] };
         return this.wrapCall(daemon => daemon.getExecutablePath(), msg);
     }
     public getExecutionInfo(args: string[]): PythonExecutionInfo {
         return this.pythonExecutionService.getExecutionInfo(args);
     }
     public async isModuleInstalled(moduleName: string): Promise<boolean> {
-        const msg = {args: ['-m', moduleName]};
+        const msg = { args: ['-m', moduleName] };
         return this.wrapCall(daemon => daemon.isModuleInstalled(moduleName), msg);
     }
     public async exec(args: string[], options: SpawnOptions): Promise<ExecutionResult<string>> {
-        const msg = {args, options};
+        const msg = { args, options };
         return this.wrapCall(daemon => daemon.exec(args, options), msg);
     }
     public async execModule(moduleName: string, args: string[], options: SpawnOptions): Promise<ExecutionResult<string>> {
-        const msg = {args: ['-m', moduleName].concat(args), options};
+        const msg = { args: ['-m', moduleName].concat(args), options };
         return this.wrapCall(daemon => daemon.execModule(moduleName, args, options), msg);
     }
     public execObservable(args: string[], options: SpawnOptions): ObservableExecutionResult<string> {
-        const msg = {args, options};
+        const msg = { args, options };
         return this.wrapObservableCall(daemon => daemon.execObservable(args, options), msg);
     }
     public execModuleObservable(moduleName: string, args: string[], options: SpawnOptions): ObservableExecutionResult<string> {
-        const msg = {args: ['-m', moduleName].concat(args), options};
+        const msg = { args: ['-m', moduleName].concat(args), options };
         return this.wrapObservableCall(daemon => daemon.execModuleObservable(moduleName, args, options), msg);
     }
     /**
@@ -103,7 +103,7 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
      * @returns
      * @memberof PythonDaemonExecutionServicePool
      */
-    protected createConnection(proc: ChildProcess){
+    protected createConnection(proc: ChildProcess) {
         return createMessageConnection(new StreamMessageReader(proc.stdout), new StreamMessageWriter(proc.stdin));
     }
     @traceDecorators.error('Failed to create daemon')
@@ -131,7 +131,7 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
 
             const cls = this.options.daemonClass ?? PythonDaemonExecutionService;
             const instance = new cls(this.pythonExecutionService, this.pythonPath, daemonProc.proc, connection);
-            if (instance instanceof PythonDaemonExecutionService){
+            if (instance instanceof PythonDaemonExecutionService) {
                 this.disposables.push(instance);
                 return instance;
             }
@@ -153,7 +153,7 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
      * @returns {Promise<T>}
      * @memberof PythonDaemonExecutionServicePool
      */
-    private async wrapCall<T>(cb: (daemon: IPythonExecutionService) => Promise<T>, daemonLogMessage: {args: string[]; options?: SpawnOptions}): Promise<T> {
+    private async wrapCall<T>(cb: (daemon: IPythonExecutionService) => Promise<T>, daemonLogMessage: { args: string[]; options?: SpawnOptions }): Promise<T> {
         const daemon = await this.popDaemonFromPool();
         try {
             // When using the daemon, log the message ourselves.
@@ -175,7 +175,7 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
      * @returns {ObservableExecutionResult<string>}
      * @memberof PythonDaemonExecutionServicePool
      */
-    private wrapObservableCall(cb: (daemon: IPythonExecutionService) => ObservableExecutionResult<string>, daemonLogMessage: {args: string[]; options?: SpawnOptions}): ObservableExecutionResult<string> {
+    private wrapObservableCall(cb: (daemon: IPythonExecutionService) => ObservableExecutionResult<string>, daemonLogMessage: { args: string[]; options?: SpawnOptions }): ObservableExecutionResult<string> {
         const execService = this.popDaemonFromObservablePool();
         // Possible the daemon returned is a standard python execution service.
         const daemonProc = execService instanceof PythonDaemonExecutionService ? execService.proc : undefined;
@@ -187,19 +187,19 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
         const result = cb(execService);
         let completed = false;
         const completeHandler = () => {
-            if (completed){
+            if (completed) {
                 return;
             }
             completed = true;
-            if (!daemonProc || (!daemonProc.killed && ProcessService.isAlive(daemonProc.pid))){
+            if (!daemonProc || (!daemonProc.killed && ProcessService.isAlive(daemonProc.pid))) {
                 this.pushDaemonIntoPool('ObservableDaemon', execService);
             } else {
                 // Possible daemon is dead (explicitly killed or died due to some error).
                 this.addDaemonService('ObservableDaemon').ignoreErrors();
             }
         };
 
-        if (daemonProc){
+        if (daemonProc) {
             daemonProc.on('exit', completeHandler);
             daemonProc.on('close', completeHandler);
         }
@@ -270,13 +270,13 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
         const testAndPushIntoPool = async () => {
             const daemonService = (daemon as PythonDaemonExecutionService);
             let procIsDead = false;
-            if (!daemonService.isAlive || daemonService.proc.killed || !ProcessService.isAlive(daemonService.proc.pid)){
+            if (!daemonService.isAlive || daemonService.proc.killed || !ProcessService.isAlive(daemonService.proc.pid)) {
                 procIsDead = true;
             } else {
                 // Test sending a ping.
                 procIsDead = await this.testDaemon(daemonService.connection).then(() => false).catch(() => true);
             }
-            if (procIsDead){
+            if (procIsDead) {
                 // The process is dead, create a new daemon.
                 await this.addDaemonService(type);
                 try {
@@ -301,10 +301,10 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
      * @memberof PythonDaemonExecutionServicePool
      */
     @traceDecorators.error('Pinging Daemon Failed')
-    private async testDaemon(connection: MessageConnection){
+    private async testDaemon(connection: MessageConnection) {
         // If we don't get a reply to the ping in 5 seconds assume it will never work. Bomb out.
         // At this point there should be some information logged in stderr of the daemon process.
-        const fail = createDeferred<{pong: string}>();
+        const fail = createDeferred<{ pong: string }>();
         const timer = setTimeout(() => fail.reject(new Error('Timeout waiting for daemon to start')), 5_000);
         const request = new RequestType<{ data: string }, { pong: string }, void, void>('ping');
         // Check whether the daemon has started correctly, by sending a ping.

src/client/datascience/interactive-window/interactiveWindowCommandListener.ts

@@ -194,7 +194,7 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList
 
     @captureTelemetry(Telemetry.ExportPythonFileAndOutput, undefined, false)
     private async exportFileAndOutput(file: string): Promise<Uri | undefined> {
-        if (file && file.length > 0 && this.jupyterExecution.isNotebookSupported()) {
+        if (file && file.length > 0 && await this.jupyterExecution.isNotebookSupported()) {
             // If the current file is the active editor, then generate cells from the document.
             const activeEditor = this.documentManager.activeTextEditor;
             if (activeEditor && activeEditor.document && this.fileSystem.arePathsSame(activeEditor.document.fileName, file)) {

src/client/datascience/jupyter/jupyterExecution.ts

@@ -2,7 +2,7 @@
 // Licensed under the MIT License.
 'use strict';
 import * as uuid from 'uuid/v4';
-import { CancellationToken, Event, EventEmitter } from 'vscode';
+import { CancellationToken, CancellationTokenSource, Event, EventEmitter } from 'vscode';
 
 import { IApplicationShell, ILiveShareApi, IWorkspaceService } from '../../common/application/types';
 import { Cancellation } from '../../common/cancellation';
@@ -111,19 +111,26 @@ export class JupyterExecutionBase implements IJupyterExecution {
     //tslint:disable:cyclomatic-complexity max-func-body-length
     public connectToNotebookServer(options?: INotebookServerOptions, cancelToken?: CancellationToken): Promise<INotebookServer | undefined> {
         // Return nothing if we cancel
+        // tslint:disable-next-line: max-func-body-length
         return Cancellation.race(async () => {
             let result: INotebookServer | undefined;
             let connection: IConnection | undefined;
             let kernelSpecInterpreter: KernelSpecInterpreter | undefined;
             let kernelSpecInterpreterPromise: Promise<KernelSpecInterpreter> = Promise.resolve({});
             traceInfo(`Connecting to ${options ? options.purpose : 'unknown type of'} server`);
+            const kernelSpecCancelSource = new CancellationTokenSource();
+            if (cancelToken) {
+                cancelToken.onCancellationRequested(() => {
+                    kernelSpecCancelSource.cancel();
+                });
+            }
             const isLocalConnection = !options || !options.uri;
 
             if (isLocalConnection) {
                 // Get hold of the kernelspec and corresponding (matching) interpreter that'll be used as the spec.
                 // We can do this in parallel, while starting the server (faster).
                 traceInfo(`Getting kernel specs for ${options ? options.purpose : 'unknown type of'} server`);
-                kernelSpecInterpreterPromise = this.kernelSelector.getKernelForLocalConnection(undefined, options?.metadata, cancelToken);
+                kernelSpecInterpreterPromise = this.kernelSelector.getKernelForLocalConnection(undefined, options?.metadata, kernelSpecCancelSource.token);
             }
 
             // Try to connect to our jupyter process. Check our setting for the number of tries
@@ -176,7 +183,7 @@ export class JupyterExecutionBase implements IJupyterExecution {
                                     const sessionManagerFactory = this.serviceContainer.get<IJupyterSessionManagerFactory>(IJupyterSessionManagerFactory);
                                     const sessionManager = await sessionManagerFactory.create(connection);
                                     const kernelInterpreter = await this.kernelSelector.selectLocalKernel(sessionManager, cancelToken, launchInfo.kernelSpec);
-                                    if (Object.keys(kernelInterpreter).length > 0){
+                                    if (Object.keys(kernelInterpreter).length > 0) {
                                         launchInfo.interpreter = kernelInterpreter.interpreter;
                                         launchInfo.kernelSpec = kernelInterpreter.kernelSpec || kernelInterpreter.kernelModel;
                                         continue;
@@ -205,6 +212,8 @@ export class JupyterExecutionBase implements IJupyterExecution {
                         connection?.dispose();
                         tryCount += 1;
                     } else if (connection) {
+                        kernelSpecCancelSource.cancel();
+
                         // Something else went wrong
                         if (!isLocalConnection) {
                             sendTelemetryEvent(Telemetry.ConnectRemoteFailedJupyter);
@@ -221,6 +230,7 @@ export class JupyterExecutionBase implements IJupyterExecution {
                             throw new Error(localize.DataScience.jupyterNotebookConnectFailed().format(connection.baseUrl, err));
                         }
                     } else {
+                        kernelSpecCancelSource.cancel();
                         throw err;
                     }
                 }

src/test/common/process/pythonDaemonPool.unit.test.ts

@@ -72,7 +72,7 @@ suite('Daemon - Python Daemon Pool', () => {
     }
     test('Create daemons when initializing', async () => {
         // Create and initialize the pool.
-        const pool = new DaemonPool(logger, [], {pythonPath: 'py.exe' }, instance(pythonExecService), undefined);
+        const pool = new DaemonPool(logger, [], { pythonPath: 'py.exe' }, instance(pythonExecService), undefined);
         await setupDaemon(pool);
 
         // 2 = 2 for standard daemon + 1 observable daemon.
@@ -126,42 +126,46 @@ suite('Daemon - Python Daemon Pool', () => {
         const getInterpreterInformationStub = sinon.stub(PythonDaemonExecutionService.prototype, 'getInterpreterInformation');
         const interpreterInfoFromDaemon: InterpreterInfomation = { pythonPath: 1 } as any;
         const interpreterInfoFromPythonProc: InterpreterInfomation = { pythonPath: 2 } as any;
-        // Delay returning interpreter info for 1.5 seconds.
-        getInterpreterInformationStub.returns(sleep(1_500).then(() => interpreterInfoFromDaemon));
-        when(pythonExecService.getInterpreterInformation()).thenResolve(interpreterInfoFromPythonProc);
 
-        // Create and initialize the pool.
-        const pool = new DaemonPool(logger, [], { daemonCount: 2, observableDaemonCount: 1, pythonPath: 'py.exe' }, instance(pythonExecService), undefined);
-        await setupDaemon(pool);
-
-        // 3 = 2 for standard daemon + 1 observable daemon.
-        expect(sendRequestStub.callCount).equal(3);
-        expect(listenStub.callCount).equal(3);
-
-        const [info1, info2, info3, info4] = await Promise.all([
-            pool.getInterpreterInformation(),
-            pool.getInterpreterInformation(),
-            pool.getInterpreterInformation(),
-            pool.getInterpreterInformation()
-        ]);
-
-        // Verify we used the daemon.
-        expect(getInterpreterInformationStub.callCount).to.equal(2);
-        // Verify we used the python execution service.
-        verify(pythonExecService.getInterpreterInformation()).twice();
-
-        expect(info1).to.deep.equal(interpreterInfoFromDaemon);
-        expect(info2).to.deep.equal(interpreterInfoFromDaemon);
-        expect(info3).to.deep.equal(interpreterInfoFromPythonProc);
-        expect(info4).to.deep.equal(interpreterInfoFromPythonProc);
+        try {
+            // Delay returning interpreter info for 1.5 seconds.
+            getInterpreterInformationStub.value(() => sleep(1_500).then(() => interpreterInfoFromDaemon));
+            when(pythonExecService.getInterpreterInformation()).thenResolve(interpreterInfoFromPythonProc);
+
+            // Create and initialize the pool.
+            const pool = new DaemonPool(logger, [], { daemonCount: 2, observableDaemonCount: 1, pythonPath: 'py.exe' }, instance(pythonExecService), undefined);
+            await setupDaemon(pool);
+
+            // 3 = 2 for standard daemon + 1 observable daemon.
+            expect(sendRequestStub.callCount).equal(3);
+            expect(listenStub.callCount).equal(3);
+
+            const [info1, info2, info3, info4] = await Promise.all([
+                pool.getInterpreterInformation(),
+                pool.getInterpreterInformation(),
+                pool.getInterpreterInformation(),
+                pool.getInterpreterInformation()
+            ]);
+
+            // Verify we used the python execution service.
+            verify(pythonExecService.getInterpreterInformation()).twice();
+
+            expect(info1).to.deep.equal(interpreterInfoFromDaemon);
+            expect(info2).to.deep.equal(interpreterInfoFromDaemon);
+            expect(info3).to.deep.equal(interpreterInfoFromPythonProc);
+            expect(info4).to.deep.equal(interpreterInfoFromPythonProc);
+        } finally {
+            // Make sure to remove the stub or other tests will take too long.
+            getInterpreterInformationStub.restore();
+        }
     }).timeout(3_000);
     test('If executing python is fast, then use the daemon (for observables)', async () => {
         const execModuleObservable = sinon.stub(PythonDaemonExecutionService.prototype, 'execModuleObservable');
         const out = new Observable<Output<string>>(s => {
-            s.next({source: 'stdout', out: ''});
+            s.next({ source: 'stdout', out: '' });
             s.complete();
         });
-        execModuleObservable.returns({out} as any);
+        execModuleObservable.returns({ out } as any);
 
         // Create and initialize the pool.
         const pool = new DaemonPool(logger, [], { daemonCount: 1, observableDaemonCount: 1, pythonPath: 'py.exe' }, instance(pythonExecService), undefined);
@@ -173,7 +177,7 @@ suite('Daemon - Python Daemon Pool', () => {
 
         // Invoke the execModuleObservable method twice (one to use daemon, other will use python exec service).
         reset(pythonExecService);
-        when(pythonExecService.execModuleObservable(anything(), anything(), anything())).thenReturn(({out} as any));
+        when(pythonExecService.execModuleObservable(anything(), anything(), anything())).thenReturn(({ out } as any));
         await Promise.all([pool.execModuleObservable('x', [], {}), pool.execModuleObservable('x', [], {})]);
 
         // Verify we used the daemon.