Update logic used to parse the args passed into the test frameworks

news/2 Fixes/1070.md

@@ -0,0 +1 @@
+Improvements to the logic used to parse the arguments passed into the test frameworks.

package.json

@@ -1955,6 +1955,7 @@
         "@types/semver": "^5.5.0",
         "@types/shortid": "^0.0.29",
         "@types/sinon": "^4.3.0",
+        "@types/tmp": "0.0.33",
         "@types/untildify": "^3.0.0",
         "@types/uuid": "^3.4.3",
         "@types/winreg": "^1.2.30",

src/client/activation/downloader.ts

@@ -7,7 +7,7 @@ import * as request from 'request';
 import * as requestProgress from 'request-progress';
 import { OutputChannel, ProgressLocation, window } from 'vscode';
 import { STANDARD_OUTPUT_CHANNEL } from '../common/constants';
-import { createDeferred, createTemporaryFile } from '../common/helpers';
+import { createDeferred } from '../common/helpers';
 import { IFileSystem, IPlatformService } from '../common/platform/types';
 import { IExtensionContext, IOutputChannel } from '../common/types';
 import { IServiceContainer } from '../ioc/types';
@@ -58,14 +58,14 @@ export class AnalysisEngineDownloader {
     private async downloadFile(location: string, fileName: string, title: string): Promise<string> {
         const uri = `${location}/${fileName}`;
         this.output.append(`Downloading ${uri}... `);
-        const tempFile = await createTemporaryFile(downloadFileExtension);
+        const tempFile = await this.fs.createTemporaryFile(downloadFileExtension);
 
         const deferred = createDeferred();
         const fileStream = fileSystem.createWriteStream(tempFile.filePath);
         fileStream.on('finish', () => {
             fileStream.close();
         }).on('error', (err) => {
-            tempFile.cleanupCallback();
+            tempFile.dispose();
             deferred.reject(err);
         });
 

src/client/application/diagnostics/applicationDiagnostics.ts

@@ -20,7 +20,7 @@ export class ApplicationDiagnostics implements IApplicationDiagnostics {
         const diagnostics = await envHealthCheck.diagnose();
         this.log(diagnostics);
         if (diagnostics.length > 0) {
-            envHealthCheck.handle(diagnostics);
+            await envHealthCheck.handle(diagnostics);
         }
     }
     private log(diagnostics: IDiagnostic[]): void {

src/client/application/diagnostics/checks/envPathVariable.ts

@@ -54,7 +54,7 @@ export class EnvironmentPathVariableDiagnosticsService extends BaseDiagnosticsSe
             return;
         }
         const diagnostic = diagnostics[0];
-        if (this.filterService.shouldIgnoreDiagnostic(diagnostic.code)) {
+        if (await this.filterService.shouldIgnoreDiagnostic(diagnostic.code)) {
             return;
         }
         const commandFactory = this.serviceContainer.get<IDiagnosticsCommandFactory>(IDiagnosticsCommandFactory);

src/client/common/application/types.ts

@@ -407,7 +407,7 @@ export interface IDocumentManager {
     showTextDocument(uri: Uri, options?: TextDocumentShowOptions): Thenable<TextEditor>;
 }
 
-export const IWorkspaceService = Symbol('IWorkspace');
+export const IWorkspaceService = Symbol('IWorkspaceService');
 
 export interface IWorkspaceService {
     /**

src/client/common/platform/fileSystem.ts

@@ -7,8 +7,9 @@ import * as fs from 'fs-extra';
 import * as glob from 'glob';
 import { inject, injectable } from 'inversify';
 import * as path from 'path';
+import * as tmp from 'tmp';
 import { createDeferred } from '../helpers';
-import { IFileSystem, IPlatformService } from './types';
+import { IFileSystem, IPlatformService, TemporaryFile } from './types';
 
 @injectable()
 export class FileSystem implements IFileSystem {
@@ -141,4 +142,15 @@ export class FileSystem implements IFileSystem {
             });
         });
     }
+    public createTemporaryFile(extension: string): Promise<TemporaryFile> {
+        return new Promise<TemporaryFile>((resolve, reject) => {
+            tmp.file({ postfix: extension }, (err, tmpFile, _, cleanupCallback) => {
+                if (err) {
+                    return reject(err);
+                }
+                resolve({ filePath: tmpFile, dispose: cleanupCallback });
+            });
+        });
+
+    }
 }

src/client/common/platform/types.ts

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 import * as fs from 'fs';
+import { Disposable } from 'vscode';
 
 export enum Architecture {
     Unknown = 1,
@@ -28,6 +29,8 @@ export interface IPlatformService {
     virtualEnvBinName: 'bin' | 'scripts';
 }
 
+export type TemporaryFile = { filePath: string } & Disposable;
+
 export const IFileSystem = Symbol('IFileSystem');
 export interface IFileSystem {
     directorySeparatorChar: string;
@@ -48,4 +51,5 @@ export interface IFileSystem {
     deleteFile(filename: string): Promise<void>;
     getFileHash(filePath: string): Promise<string | undefined>;
     search(globPattern: string): Promise<string[]>;
+    createTemporaryFile(extension: string): Promise<TemporaryFile>;
 }

src/client/unittests/common/argumentsHelper.ts

@@ -0,0 +1,105 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+'use strict';
+
+import { inject, injectable } from 'inversify';
+import { ILogger } from '../../common/types';
+import { IServiceContainer } from '../../ioc/types';
+import { IArgumentsHelper } from '../types';
+
+@injectable()
+export class ArgumentsHelper implements IArgumentsHelper {
+    private readonly logger: ILogger;
+    constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
+        this.logger = serviceContainer.get<ILogger>(ILogger);
+    }
+    public getOptionValues(args: string[], option: string): string | string[] | undefined {
+        const values: string[] = [];
+        let returnNextValue = false;
+        for (const arg of args) {
+            if (returnNextValue) {
+                values.push(arg);
+                returnNextValue = false;
+                continue;
+            }
+            if (arg.startsWith(`${option}=`)) {
+                values.push(arg.substring(`${option}=`.length));
+                continue;
+            }
+            if (arg === option) {
+                returnNextValue = true;
+            }
+        }
+        switch (values.length) {
+            case 0: {
+                return;
+            }
+            case 1: {
+                return values[0];
+            }
+            default: {
+                return values;
+            }
+        }
+    }
+    public getPositionalArguments(args: string[], optionsWithArguments: string[] = [], optionsWithoutArguments: string[] = []): string[] {
+        let lastIndexOfOption = -1;
+        args.forEach((arg, index) => {
+            if (optionsWithoutArguments.indexOf(arg) !== -1) {
+                lastIndexOfOption = index;
+                return;
+            } else if (optionsWithArguments.indexOf(arg) !== -1) {
+                // Cuz the next item is the value.
+                lastIndexOfOption = index + 1;
+            } else if (optionsWithArguments.findIndex(item => arg.startsWith(`${item}=`)) !== -1) {
+                lastIndexOfOption = index;
+                return;
+            } else if (arg.startsWith('-')) {
+                // Ok this is an unknown option, lets treat this as one without values.
+                this.logger.logWarning(`Unknown command line option passed into args parser for tests '${arg}'. Please report on https://github.com/Microsoft/vscode-python/issues/new`);
+                lastIndexOfOption = index;
+                return;
+            } else if (args.indexOf('=') > 0) {
+                // Ok this is an unknown option with a value
+                this.logger.logWarning(`Unknown command line option passed into args parser for tests '${arg}'. Please report on https://github.com/Microsoft/vscode-python/issues/new`);
+                lastIndexOfOption = index;
+            }
+        });
+        return args.slice(lastIndexOfOption + 1);
+    }
+    public filterArguments(args: string[], optionsWithArguments: string[] = [], optionsWithoutArguments: string[] = []): string[] {
+        let ignoreIndex = -1;
+        return args.filter((arg, index) => {
+            if (ignoreIndex === index) {
+                return false;
+            }
+            // Options can use willd cards (with trailing '*')
+            if (optionsWithoutArguments.indexOf(arg) >= 0 ||
+                optionsWithoutArguments.filter(option => option.endsWith('*') && arg.startsWith(option.slice(0, -1))).length > 0) {
+                return false;
+            }
+            // Ignore args that match exactly.
+            if (optionsWithArguments.indexOf(arg) >= 0) {
+                ignoreIndex = index + 1;
+                return false;
+            }
+            // Ignore args that match exactly with wild cards & do not have inline values.
+            if (optionsWithArguments.filter(option => arg.startsWith(`${option}=`)).length > 0) {
+                return false;
+            }
+            // Ignore args that match a wild card (ending with *) and no ineline values.
+            // Eg. arg='--log-cli-level' and optionsArguments=['--log-*']
+            if (arg.indexOf('=') === -1 && optionsWithoutArguments.filter(option => option.endsWith('*') && arg.startsWith(option.slice(0, -1))).length > 0) {
+                ignoreIndex = index + 1;
+                return false;
+            }
+            // Ignore args that match a wild card (ending with *) and have ineline values.
+            // Eg. arg='--log-cli-level=XYZ' and optionsArguments=['--log-*']
+            if (arg.indexOf('=') >= 0 && optionsWithoutArguments.filter(option => option.endsWith('*') && arg.startsWith(option.slice(0, -1))).length > 0) {
+                return false;
+            }
+            return true;
+        });
+    }
+}

src/client/unittests/common/managers/baseTestManager.ts

@@ -1,7 +1,7 @@
-import { CancellationToken, CancellationTokenSource, Disposable, OutputChannel, Uri, workspace } from 'vscode';
-import { PythonSettings } from '../../../common/configSettings';
+import { CancellationToken, CancellationTokenSource, Disposable, OutputChannel, Uri } from 'vscode';
+import { IWorkspaceService } from '../../../common/application/types';
 import { isNotInstalledError } from '../../../common/helpers';
-import { IDisposableRegistry, IInstaller, IOutputChannel, IPythonSettings, Product } from '../../../common/types';
+import { IConfigurationService, IDisposableRegistry, IInstaller, IOutputChannel, IPythonSettings, Product } from '../../../common/types';
 import { IServiceContainer } from '../../../ioc/types';
 import { UNITTEST_DISCOVER, UNITTEST_RUN } from '../../../telemetry/constants';
 import { sendTelemetryEvent } from '../../../telemetry/index';
@@ -25,9 +25,9 @@ export abstract class BaseTestManager implements ITestManager {
     }
     private testCollectionStorage: ITestCollectionStorageService;
     private _testResultsService: ITestResultsService;
+    private workspaceService: IWorkspaceService;
     private _outputChannel: OutputChannel;
     private tests?: Tests;
-    // tslint:disable-next-line:variable-name
     private _status: TestStatus = TestStatus.Unknown;
     private testDiscoveryCancellationTokenSource?: CancellationTokenSource;
     private testRunnerCancellationTokenSource?: CancellationTokenSource;
@@ -42,12 +42,14 @@ export abstract class BaseTestManager implements ITestManager {
     constructor(public readonly testProvider: TestProvider, private product: Product, public readonly workspaceFolder: Uri, protected rootDirectory: string,
         protected serviceContainer: IServiceContainer) {
         this._status = TestStatus.Unknown;
-        this.settings = PythonSettings.getInstance(this.rootDirectory ? Uri.file(this.rootDirectory) : undefined);
+        const configService = serviceContainer.get<IConfigurationService>(IConfigurationService);
+        this.settings = configService.getSettings(this.rootDirectory ? Uri.file(this.rootDirectory) : undefined);
         const disposables = serviceContainer.get<Disposable[]>(IDisposableRegistry);
-        disposables.push(this);
         this._outputChannel = this.serviceContainer.get<OutputChannel>(IOutputChannel, TEST_OUTPUT_CHANNEL);
         this.testCollectionStorage = this.serviceContainer.get<ITestCollectionStorageService>(ITestCollectionStorageService);
         this._testResultsService = this.serviceContainer.get<ITestResultsService>(ITestResultsService);
+        this.workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
+        disposables.push(this);
     }
     protected get testDiscoveryCancellationToken(): CancellationToken | undefined {
         return this.testDiscoveryCancellationTokenSource ? this.testDiscoveryCancellationTokenSource.token : undefined;
@@ -62,8 +64,7 @@ export abstract class BaseTestManager implements ITestManager {
         return this._status;
     }
     public get workingDirectory(): string {
-        const settings = PythonSettings.getInstance(Uri.file(this.rootDirectory));
-        return settings.unitTest.cwd && settings.unitTest.cwd.length > 0 ? settings.unitTest.cwd : this.rootDirectory;
+        return this.settings.unitTest.cwd && this.settings.unitTest.cwd.length > 0 ? this.settings.unitTest.cwd : this.rootDirectory;
     }
     public stop() {
         if (this.testDiscoveryCancellationTokenSource) {
@@ -132,7 +133,7 @@ export abstract class BaseTestManager implements ITestManager {
                     const testsHelper = this.serviceContainer.get<ITestsHelper>(ITestsHelper);
                     testsHelper.displayTestErrorMessage('There were some errors in discovering unit tests');
                 }
-                const wkspace = workspace.getWorkspaceFolder(Uri.file(this.rootDirectory))!.uri;
+                const wkspace = this.workspaceService.getWorkspaceFolder(Uri.file(this.rootDirectory))!.uri;
                 this.testCollectionStorage.storeTests(wkspace, tests);
                 this.disposeCancellationToken(CancellationTokenType.testDiscovery);
                 sendTelemetryEvent(UNITTEST_DISCOVER, undefined, telementryProperties);
@@ -156,7 +157,7 @@ export abstract class BaseTestManager implements ITestManager {
                     // tslint:disable-next-line:prefer-template
                     this.outputChannel.appendLine(reason.toString());
                 }
-                const wkspace = workspace.getWorkspaceFolder(Uri.file(this.rootDirectory))!.uri;
+                const wkspace = this.workspaceService.getWorkspaceFolder(Uri.file(this.rootDirectory))!.uri;
                 this.testCollectionStorage.storeTests(wkspace, null);
                 this.disposeCancellationToken(CancellationTokenType.testDiscovery);
                 return Promise.reject(reason);

src/client/unittests/common/runner.ts

@@ -1,3 +1,4 @@
+import { inject, injectable } from 'inversify';
 import * as path from 'path';
 import { CancellationToken, OutputChannel, Uri } from 'vscode';
 import { PythonSettings } from '../../common/configSettings';
@@ -13,15 +14,16 @@ import {
 import { ExecutionInfo, IPythonSettings } from '../../common/types';
 import { IServiceContainer } from '../../ioc/types';
 import { NOSETEST_PROVIDER, PYTEST_PROVIDER, UNITTEST_PROVIDER } from './constants';
-import { ITestsHelper, TestProvider } from './types';
+import { ITestRunner, ITestsHelper, Options, TestProvider } from './types';
+export { Options } from './types';
 
-export type Options = {
-    workspaceFolder: Uri;
-    cwd: string;
-    args: string[];
-    outChannel?: OutputChannel;
-    token: CancellationToken;
-};
+@injectable()
+export class TestRunner implements ITestRunner {
+    constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { }
+    public run(testProvider: TestProvider, options: Options): Promise<string> {
+        return run(this.serviceContainer, testProvider, options);
+    }
+}
 
 export async function run(serviceContainer: IServiceContainer, testProvider: TestProvider, options: Options): Promise<string> {
     const testExecutablePath = getExecutablePath(testProvider, PythonSettings.getInstance(options.workspaceFolder));

src/client/unittests/common/services/storageService.ts

@@ -6,7 +6,7 @@ import { ITestCollectionStorageService, Tests } from './../types';
 @injectable()
 export class TestCollectionStorageService implements ITestCollectionStorageService {
     private testsIndexedByWorkspaceUri = new Map<string, Tests | undefined>();
-    constructor( @inject(IDisposableRegistry) disposables: Disposable[]) {
+    constructor(@inject(IDisposableRegistry) disposables: Disposable[]) {
         disposables.push(this);
     }
     public getTests(wkspace: Uri): Tests | undefined {

src/client/unittests/common/testUtils.ts

@@ -171,4 +171,39 @@ export class TestsHelper implements ITestsHelper {
             }
         });
     }
+    public mergeTests(items: Tests[]): Tests {
+        return items.reduce((tests, otherTests, index) => {
+            if (index === 0) {
+                return tests;
+            }
+
+            tests.summary.errors += otherTests.summary.errors;
+            tests.summary.failures += otherTests.summary.failures;
+            tests.summary.passed += otherTests.summary.passed;
+            tests.summary.skipped += otherTests.summary.skipped;
+            tests.rootTestFolders.push(...otherTests.rootTestFolders);
+            tests.testFiles.push(...otherTests.testFiles);
+            tests.testFolders.push(...otherTests.testFolders);
+            tests.testFunctions.push(...otherTests.testFunctions);
+            tests.testSuites.push(...otherTests.testSuites);
+
+            return tests;
+        }, items[0]);
+    }
+
+    public shouldRunAllTests(testsToRun?: TestsToRun) {
+        if (!testsToRun) {
+            return true;
+        }
+        if (
+            (Array.isArray(testsToRun.testFile) && testsToRun.testFile.length > 0) ||
+            (Array.isArray(testsToRun.testFolder) && testsToRun.testFolder.length > 0) ||
+            (Array.isArray(testsToRun.testFunction) && testsToRun.testFunction.length > 0) ||
+            (Array.isArray(testsToRun.testSuite) && testsToRun.testSuite.length > 0)
+        ) {
+            return false;
+        }
+
+        return true;
+    }
 }