Skip to content

Commit 35fe3c0

Browse files
DonJayamanneDonJayamanne
authored
Fix getting display name and version when using windows registry (microsoft#1697)
* Fixes microsoft#1660 * Fixes microsoft#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
1 parent ca854b8 commit 35fe3c0

15 files changed

Lines changed: 573 additions & 312 deletions

File tree

‎.vscode/launch.json‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,22 @@
6969
],
7070
"preLaunchTask": "Compile"
7171
},
72+
{
73+
"name": "Debug Unit Tests",
74+
"type": "node",
75+
"request": "launch",
76+
"program": "${workspaceFolder}/out/test/unittests.js",
77+
"stopOnEntry": false,
78+
"sourceMaps": true,
79+
"args": [
80+
"timeout=60000",
81+
"grep="
82+
],
83+
"outFiles": [
84+
"${workspaceFolder}/out/**/*.js"
85+
],
86+
"preLaunchTask": "Compile"
87+
},
7288
{
7389
"name": "Launch Multiroot Tests",
7490
"type": "extensionHost",
@@ -134,4 +150,4 @@
134150
]
135151
}
136152
]
137-
}
153+
}

‎.vscode/tasks.json‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,15 @@
2424
"isDefault": true
2525
}
2626
},
27+
{
28+
"label": "Run Unit Tests",
29+
"type": "npm",
30+
"script": "test:unittests",
31+
"group": {
32+
"kind": "test",
33+
"isDefault": true
34+
}
35+
},
2736
{
2837
"label": "Hygiene",
2938
"type": "gulp",
@@ -105,4 +114,4 @@
105114
}
106115
}
107116
]
108-
}
117+
}

‎gulpfile.js‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -502,5 +502,12 @@ exports.hygiene = hygiene;
502502

503503
// this allows us to run hygiene as a git pre-commit hook.
504504
if (require.main === module) {
505-
run({ exitOnError: true, mode: 'staged' });
505+
const result = run({ exitOnError: true, mode: 'staged' });
506+
// Run unit tests and ensure they pass as well.
507+
if (result && result.on) {
508+
result.on('end', () => {
509+
const main = require('./out/test/unittests');
510+
main.runTests();
511+
})
512+
}
506513
}

‎news/3 Code Health/1703.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Run unit tests as a pre-commit hook.

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1854,6 +1854,7 @@
18541854
"compile": "tsc -watch -p ./",
18551855
"postinstall": "node ./node_modules/vscode/bin/install",
18561856
"test": "node ./out/test/standardTest.js && node ./out/test/multiRootTest.js",
1857+
"test:unittests": "node ./out/test/unittests.js",
18571858
"testDebugger": "node ./out/test/debuggerTest.js",
18581859
"testSingleWorkspace": "node ./out/test/standardTest.js",
18591860
"testMultiWorkspace": "node ./out/test/multiRootTest.js",

‎src/client/common/platform/pathUtils.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
import { inject, injectable } from 'inversify';
2+
import * as path from 'path';
23
import { IPathUtils, IsWindows } from '../types';
34
import { NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from './constants';
45

56
// TO DO: Deprecate in favor of IPlatformService
67
@injectable()
78
export class PathUtils implements IPathUtils {
8-
constructor( @inject(IsWindows) private isWindows: boolean) { }
9+
constructor(@inject(IsWindows) private isWindows: boolean) { }
910
public getPathVariableName() {
1011
return this.isWindows ? WINDOWS_PATH_VARIABLE_NAME : NON_WINDOWS_PATH_VARIABLE_NAME;
1112
}
13+
public basename(pathValue: string, ext?: string): string{
14+
return path.basename(pathValue, ext);
15+
}
1216
}

‎src/client/common/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ export const IPathUtils = Symbol('IPathUtils');
8484

8585
export interface IPathUtils {
8686
getPathVariableName(): 'Path' | 'PATH';
87+
basename(pathValue: string, ext?: string): string;
8788
}
8889

8990
export const ICurrentProcess = Symbol('ICurrentProcess');

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import * as _ from 'lodash';
44
import * as path from 'path';
55
import { Uri } from 'vscode';
66
import { Architecture, IRegistry, RegistryHive } from '../../../common/platform/types';
7-
import { Is64Bit } from '../../../common/types';
7+
import { IPathUtils, Is64Bit } from '../../../common/types';
88
import { IServiceContainer } from '../../../ioc/types';
99
import { IInterpreterHelper, InterpreterType, PythonInterpreter } from '../../contracts';
1010
import { CacheableLocatorService } from './cacheableLocatorService';
@@ -27,10 +27,12 @@ type CompanyInterpreter = {
2727

2828
@injectable()
2929
export class WindowsRegistryService extends CacheableLocatorService {
30+
private readonly pathUtils: IPathUtils;
3031
constructor(@inject(IRegistry) private registry: IRegistry,
3132
@inject(Is64Bit) private is64Bit: boolean,
3233
@inject(IServiceContainer) serviceContainer: IServiceContainer) {
3334
super('WindowsRegistryService', serviceContainer);
35+
this.pathUtils = serviceContainer.get<IPathUtils>(IPathUtils);
3436
}
3537
// tslint:disable-next-line:no-empty
3638
public dispose() { }
@@ -72,7 +74,7 @@ export class WindowsRegistryService extends CacheableLocatorService {
7274
private async getCompanies(hive: RegistryHive, arch?: Architecture): Promise<CompanyInterpreter[]> {
7375
return this.registry.getKeys('\\Software\\Python', hive, arch)
7476
.then(companyKeys => companyKeys
75-
.filter(companyKey => CompaniesToIgnore.indexOf(path.basename(companyKey).toUpperCase()) === -1)
77+
.filter(companyKey => CompaniesToIgnore.indexOf(this.pathUtils.basename(companyKey).toUpperCase()) === -1)
7678
.map(companyKey => {
7779
return { companyKey, hive, arch };
7880
}));
@@ -125,7 +127,7 @@ export class WindowsRegistryService extends CacheableLocatorService {
125127
if (!details) {
126128
return;
127129
}
128-
const version = interpreterInfo.version ? path.basename(interpreterInfo.version) : details.version;
130+
const version = interpreterInfo.version ? this.pathUtils.basename(interpreterInfo.version) : this.pathUtils.basename(tagKey);
129131
// tslint:disable-next-line:prefer-type-cast no-object-literal-type-assertion
130132
return {
131133
...(details as PythonInterpreter),
@@ -155,7 +157,7 @@ export class WindowsRegistryService extends CacheableLocatorService {
155157
if (displayName && displayName.length > 0) {
156158
return displayName;
157159
}
158-
const company = path.basename(companyKey);
160+
const company = this.pathUtils.basename(companyKey);
159161
return company.toUpperCase() === PythonCoreComany ? PythonCoreCompanyDisplayName : company;
160162
}
161163
}

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

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,46 @@
11
import * as path from 'path';
2-
import * as vscode from 'vscode';
3-
import { Uri } from 'vscode';
2+
import { OutputChannel, QuickPickItem, Uri, window } from 'vscode';
43
import { createDeferred } from '../../../common/helpers';
5-
import { IInstaller } from '../../../common/types';
4+
import { IInstaller, Product } from '../../../common/types';
65
import { getSubDirectories } from '../../../common/utils';
76
import { ITestConfigSettingsService, UnitTestProduct } from './../types';
87

98
export abstract class TestConfigurationManager {
109
constructor(protected workspace: Uri,
1110
protected product: UnitTestProduct,
12-
protected readonly outputChannel: vscode.OutputChannel,
11+
protected readonly outputChannel: OutputChannel,
1312
protected installer: IInstaller,
1413
protected testConfigSettingsService: ITestConfigSettingsService) { }
1514
// tslint:disable-next-line:no-any
1615
public abstract configure(wkspace: Uri): Promise<any>;
1716
public async enable() {
17+
// Disable other test frameworks.
18+
const testProducsToDisable = [Product.pytest, Product.unittest, Product.nosetest]
19+
.filter(item => item !== this.product) as UnitTestProduct[];
20+
21+
for (const prod of testProducsToDisable) {
22+
await this.testConfigSettingsService.disable(this.workspace, prod);
23+
}
24+
1825
return this.testConfigSettingsService.enable(this.workspace, this.product);
1926
}
2027
// tslint:disable-next-line:no-any
2128
public async disable() {
2229
return this.testConfigSettingsService.enable(this.workspace, this.product);
2330
}
24-
protected selectTestDir(rootDir: string, subDirs: string[], customOptions: vscode.QuickPickItem[] = []): Promise<string> {
31+
protected selectTestDir(rootDir: string, subDirs: string[], customOptions: QuickPickItem[] = []): Promise<string> {
2532
const options = {
2633
matchOnDescription: true,
2734
matchOnDetail: true,
2835
placeHolder: 'Select the directory containing the unit tests'
2936
};
30-
let items: vscode.QuickPickItem[] = subDirs
37+
let items: QuickPickItem[] = subDirs
3138
.map(dir => {
3239
const dirName = path.relative(rootDir, dir);
3340
if (dirName.indexOf('.') === 0) {
3441
return;
3542
}
36-
return <vscode.QuickPickItem>{
43+
return {
3744
label: dirName,
3845
description: ''
3946
};
@@ -44,7 +51,7 @@ export abstract class TestConfigurationManager {
4451
items = [{ label: '.', description: 'Root directory' }, ...items];
4552
items = customOptions.concat(items);
4653
const def = createDeferred<string>();
47-
vscode.window.showQuickPick(items, options).then(item => {
54+
window.showQuickPick(items, options).then(item => {
4855
if (!item) {
4956
return def.resolve();
5057
}
@@ -61,7 +68,7 @@ export abstract class TestConfigurationManager {
6168
matchOnDetail: true,
6269
placeHolder: 'Select the pattern to identify test files'
6370
};
64-
const items: vscode.QuickPickItem[] = [
71+
const items: QuickPickItem[] = [
6572
{ label: '*test.py', description: 'Python Files ending with \'test\'' },
6673
{ label: '*_test.py', description: 'Python Files ending with \'_test\'' },
6774
{ label: 'test*.py', description: 'Python Files begining with \'test\'' },
@@ -70,7 +77,7 @@ export abstract class TestConfigurationManager {
7077
];
7178

7279
const def = createDeferred<string>();
73-
vscode.window.showQuickPick(items, options).then(item => {
80+
window.showQuickPick(items, options).then(item => {
7481
if (!item) {
7582
return def.resolve();
7683
}

0 commit comments

Comments
 (0)