Fix getting display name and version when using windows registry (#1697)

* Fixes #1660
* Fixes #1703
* 🐛 fix getting display name and version when using windows registry
* Git hook for tests
* 📝 add news entry
* Some more unit tests\
* Mock VS Code only inside the test runner
* Singleton mocks for vscode namespcaes
* Fix test

Author: DonJayamanne

.vscode/launch.json

@@ -69,6 +69,22 @@
             ],
             "preLaunchTask": "Compile"
         },
+        {
+            "name": "Debug Unit Tests",
+            "type": "node",
+            "request": "launch",
+            "program": "${workspaceFolder}/out/test/unittests.js",
+            "stopOnEntry": false,
+            "sourceMaps": true,
+            "args": [
+                "timeout=60000",
+                "grep="
+            ],
+            "outFiles": [
+                "${workspaceFolder}/out/**/*.js"
+            ],
+            "preLaunchTask": "Compile"
+        },
         {
             "name": "Launch Multiroot Tests",
             "type": "extensionHost",
@@ -134,4 +150,4 @@
             ]
         }
     ]
-}
+}

.vscode/tasks.json

@@ -24,6 +24,15 @@
                 "isDefault": true
             }
         },
+        {
+            "label": "Run Unit Tests",
+            "type": "npm",
+            "script": "test:unittests",
+            "group": {
+                "kind": "test",
+                "isDefault": true
+            }
+        },
         {
             "label": "Hygiene",
             "type": "gulp",
@@ -105,4 +114,4 @@
             }
         }
     ]
-}
+}

gulpfile.js

@@ -502,5 +502,12 @@ exports.hygiene = hygiene;
 
 // this allows us to run hygiene as a git pre-commit hook.
 if (require.main === module) {
-    run({ exitOnError: true, mode: 'staged' });
+    const result = run({ exitOnError: true, mode: 'staged' });
+    // Run unit tests and ensure they pass as well.
+    if (result && result.on) {
+        result.on('end', () => {
+            const main = require('./out/test/unittests');
+            main.runTests();
+        })
+    }
 }

news/3 Code Health/1703.md

@@ -0,0 +1 @@
+Run unit tests as a pre-commit hook.

package.json

@@ -1854,6 +1854,7 @@
         "compile": "tsc -watch -p ./",
         "postinstall": "node ./node_modules/vscode/bin/install",
         "test": "node ./out/test/standardTest.js && node ./out/test/multiRootTest.js",
+        "test:unittests": "node ./out/test/unittests.js",
         "testDebugger": "node ./out/test/debuggerTest.js",
         "testSingleWorkspace": "node ./out/test/standardTest.js",
         "testMultiWorkspace": "node ./out/test/multiRootTest.js",

src/client/common/platform/pathUtils.ts

@@ -1,12 +1,16 @@
 import { inject, injectable } from 'inversify';
+import * as path from 'path';
 import { IPathUtils, IsWindows } from '../types';
 import { NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from './constants';
 
 // TO DO: Deprecate in favor of IPlatformService
 @injectable()
 export class PathUtils implements IPathUtils {
-    constructor( @inject(IsWindows) private isWindows: boolean) { }
+    constructor(@inject(IsWindows) private isWindows: boolean) { }
     public getPathVariableName() {
         return this.isWindows ? WINDOWS_PATH_VARIABLE_NAME : NON_WINDOWS_PATH_VARIABLE_NAME;
     }
+    public basename(pathValue: string, ext?: string): string{
+        return path.basename(pathValue, ext);
+    }
 }

src/client/common/types.ts

@@ -84,6 +84,7 @@ export const IPathUtils = Symbol('IPathUtils');
 
 export interface IPathUtils {
     getPathVariableName(): 'Path' | 'PATH';
+    basename(pathValue: string, ext?: string): string;
 }
 
 export const ICurrentProcess = Symbol('ICurrentProcess');

src/client/interpreter/locators/services/windowsRegistryService.ts

@@ -4,7 +4,7 @@ import * as _ from 'lodash';
 import * as path from 'path';
 import { Uri } from 'vscode';
 import { Architecture, IRegistry, RegistryHive } from '../../../common/platform/types';
-import { Is64Bit } from '../../../common/types';
+import { IPathUtils, Is64Bit } from '../../../common/types';
 import { IServiceContainer } from '../../../ioc/types';
 import { IInterpreterHelper, InterpreterType, PythonInterpreter } from '../../contracts';
 import { CacheableLocatorService } from './cacheableLocatorService';
@@ -27,10 +27,12 @@ type CompanyInterpreter = {
 
 @injectable()
 export class WindowsRegistryService extends CacheableLocatorService {
+    private readonly pathUtils: IPathUtils;
     constructor(@inject(IRegistry) private registry: IRegistry,
         @inject(Is64Bit) private is64Bit: boolean,
         @inject(IServiceContainer) serviceContainer: IServiceContainer) {
         super('WindowsRegistryService', serviceContainer);
+        this.pathUtils = serviceContainer.get<IPathUtils>(IPathUtils);
     }
     // tslint:disable-next-line:no-empty
     public dispose() { }
@@ -72,7 +74,7 @@ export class WindowsRegistryService extends CacheableLocatorService {
     private async getCompanies(hive: RegistryHive, arch?: Architecture): Promise<CompanyInterpreter[]> {
         return this.registry.getKeys('\\Software\\Python', hive, arch)
             .then(companyKeys => companyKeys
-                .filter(companyKey => CompaniesToIgnore.indexOf(path.basename(companyKey).toUpperCase()) === -1)
+                .filter(companyKey => CompaniesToIgnore.indexOf(this.pathUtils.basename(companyKey).toUpperCase()) === -1)
                 .map(companyKey => {
                     return { companyKey, hive, arch };
                 }));
@@ -125,7 +127,7 @@ export class WindowsRegistryService extends CacheableLocatorService {
                 if (!details) {
                     return;
                 }
-                const version = interpreterInfo.version ? path.basename(interpreterInfo.version) : details.version;
+                const version = interpreterInfo.version ? this.pathUtils.basename(interpreterInfo.version) : this.pathUtils.basename(tagKey);
                 // tslint:disable-next-line:prefer-type-cast no-object-literal-type-assertion
                 return {
                     ...(details as PythonInterpreter),
@@ -155,7 +157,7 @@ export class WindowsRegistryService extends CacheableLocatorService {
         if (displayName && displayName.length > 0) {
             return displayName;
         }
-        const company = path.basename(companyKey);
+        const company = this.pathUtils.basename(companyKey);
         return company.toUpperCase() === PythonCoreComany ? PythonCoreCompanyDisplayName : company;
     }
 }

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

@@ -1,39 +1,46 @@
 import * as path from 'path';
-import * as vscode from 'vscode';
-import { Uri } from 'vscode';
+import { OutputChannel, QuickPickItem, Uri, window } from 'vscode';
 import { createDeferred } from '../../../common/helpers';
-import { IInstaller } from '../../../common/types';
+import { IInstaller, Product } from '../../../common/types';
 import { getSubDirectories } from '../../../common/utils';
 import { ITestConfigSettingsService, UnitTestProduct } from './../types';
 
 export abstract class TestConfigurationManager {
     constructor(protected workspace: Uri,
         protected product: UnitTestProduct,
-        protected readonly outputChannel: vscode.OutputChannel,
+        protected readonly outputChannel: OutputChannel,
         protected installer: IInstaller,
         protected testConfigSettingsService: ITestConfigSettingsService) { }
     // tslint:disable-next-line:no-any
     public abstract configure(wkspace: Uri): Promise<any>;
     public async enable() {
+        // Disable other test frameworks.
+        const testProducsToDisable = [Product.pytest, Product.unittest, Product.nosetest]
+            .filter(item => item !== this.product) as UnitTestProduct[];
+
+        for (const prod of testProducsToDisable) {
+            await this.testConfigSettingsService.disable(this.workspace, prod);
+        }
+
         return this.testConfigSettingsService.enable(this.workspace, this.product);
     }
     // tslint:disable-next-line:no-any
     public async disable() {
         return this.testConfigSettingsService.enable(this.workspace, this.product);
     }
-    protected selectTestDir(rootDir: string, subDirs: string[], customOptions: vscode.QuickPickItem[] = []): Promise<string> {
+    protected selectTestDir(rootDir: string, subDirs: string[], customOptions: QuickPickItem[] = []): Promise<string> {
         const options = {
             matchOnDescription: true,
             matchOnDetail: true,
             placeHolder: 'Select the directory containing the unit tests'
         };
-        let items: vscode.QuickPickItem[] = subDirs
+        let items: QuickPickItem[] = subDirs
             .map(dir => {
                 const dirName = path.relative(rootDir, dir);
                 if (dirName.indexOf('.') === 0) {
                     return;
                 }
-                return <vscode.QuickPickItem>{
+                return {
                     label: dirName,
                     description: ''
                 };
@@ -44,7 +51,7 @@ export abstract class TestConfigurationManager {
         items = [{ label: '.', description: 'Root directory' }, ...items];
         items = customOptions.concat(items);
         const def = createDeferred<string>();
-        vscode.window.showQuickPick(items, options).then(item => {
+        window.showQuickPick(items, options).then(item => {
             if (!item) {
                 return def.resolve();
             }
@@ -61,7 +68,7 @@ export abstract class TestConfigurationManager {
             matchOnDetail: true,
             placeHolder: 'Select the pattern to identify test files'
         };
-        const items: vscode.QuickPickItem[] = [
+        const items: QuickPickItem[] = [
             { label: '*test.py', description: 'Python Files ending with \'test\'' },
             { label: '*_test.py', description: 'Python Files ending with \'_test\'' },
             { label: 'test*.py', description: 'Python Files begining with \'test\'' },
@@ -70,7 +77,7 @@ export abstract class TestConfigurationManager {
         ];
 
         const def = createDeferred<string>();
-        vscode.window.showQuickPick(items, options).then(item => {
+        window.showQuickPick(items, options).then(item => {
             if (!item) {
                 return def.resolve();
             }