Update logic used to parse the args passed into the test frameworks (#1917)

* Refactor unit test discovery
* test
* Unit tests
* Update logic used to parse the args passed into the test frameworks
* fix tests
* Fix tests
* Fix path to test files
* Fix tests
* Fix more tests
* Fix pytest tests
* Fix tests
* More fixes
* Bug fix
* fix code review comments
* add missing awaits
* Create tests for filtering of arguments

Author: DonJayamanne

news/2 Fixes/1070.md

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

package-lock.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",

package.json

@@ -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/activation/downloader.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/applicationDiagnostics.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/application/diagnostics/checks/envPathVariable.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/application/types.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/fileSystem.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/common/platform/types.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;
+        });
+    }
+}