Skip to content

Commit

Permalink
Ensure file paths are not sent in telemetry when running unit tests (#…
Browse files Browse the repository at this point in the history
…1197)

* 🐛 1st arg of command handlers is {} or Uri based on how invoked
* 📝 change log
* Fixes #1180
  • Loading branch information
DonJayamanne authored Mar 26, 2018
1 parent 3626de2 commit e2b9037
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 39 deletions.
1 change: 1 addition & 0 deletions news/3 Code Health/1180.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Ensure file paths are not sent in telemetry when running unit tests.
21 changes: 11 additions & 10 deletions src/client/unittests/codeLenses/testFiles.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
'use strict';

import { CancellationToken, CancellationTokenSource, CodeLens, CodeLensProvider, Event, EventEmitter, Position, Range, SymbolInformation, SymbolKind, TextDocument, workspace } from 'vscode';
import { Uri } from 'vscode';
// tslint:disable:no-object-literal-type-assertion

import { CancellationToken, CancellationTokenSource, CodeLens, CodeLensProvider, Event, EventEmitter, Position, Range, SymbolInformation, SymbolKind, TextDocument, Uri, workspace } from 'vscode';
import * as constants from '../../common/constants';
import { PythonSymbolProvider } from '../../providers/symbolProvider';
import { CommandSource } from '../common/constants';
Expand Down Expand Up @@ -108,12 +109,12 @@ export class TestFileCodeLensProvider implements CodeLensProvider {
new CodeLens(range, {
title: getTestStatusIcon(cls.status) + constants.Text.CodeLensRunUnitTest,
command: constants.Commands.Tests_Run,
arguments: [CommandSource.codelens, file, <TestsToRun>{ testSuite: [cls] }]
arguments: [undefined, CommandSource.codelens, file, <TestsToRun>{ testSuite: [cls] }]
}),
new CodeLens(range, {
title: getTestStatusIcon(cls.status) + constants.Text.CodeLensDebugUnitTest,
command: constants.Commands.Tests_Debug,
arguments: [CommandSource.codelens, file, <TestsToRun>{ testSuite: [cls] }]
arguments: [undefined, CommandSource.codelens, file, <TestsToRun>{ testSuite: [cls] }]
})
];
}
Expand Down Expand Up @@ -182,12 +183,12 @@ function getFunctionCodeLens(file: Uri, functionsAndSuites: FunctionsAndSuites,
new CodeLens(range, {
title: getTestStatusIcon(fn.status) + constants.Text.CodeLensRunUnitTest,
command: constants.Commands.Tests_Run,
arguments: [CommandSource.codelens, file, <TestsToRun>{ testFunction: [fn] }]
arguments: [undefined, CommandSource.codelens, file, <TestsToRun>{ testFunction: [fn] }]
}),
new CodeLens(range, {
title: getTestStatusIcon(fn.status) + constants.Text.CodeLensDebugUnitTest,
command: constants.Commands.Tests_Debug,
arguments: [CommandSource.codelens, file, <TestsToRun>{ testFunction: [fn] }]
arguments: [undefined, CommandSource.codelens, file, <TestsToRun>{ testFunction: [fn] }]
})
];
}
Expand All @@ -203,12 +204,12 @@ function getFunctionCodeLens(file: Uri, functionsAndSuites: FunctionsAndSuites,
new CodeLens(range, {
title: constants.Text.CodeLensRunUnitTest,
command: constants.Commands.Tests_Run,
arguments: [CommandSource.codelens, file, <TestsToRun>{ testFunction: functions }]
arguments: [undefined, CommandSource.codelens, file, <TestsToRun>{ testFunction: functions }]
}),
new CodeLens(range, {
title: constants.Text.CodeLensDebugUnitTest,
command: constants.Commands.Tests_Debug,
arguments: [CommandSource.codelens, file, <TestsToRun>{ testFunction: functions }]
arguments: [undefined, CommandSource.codelens, file, <TestsToRun>{ testFunction: functions }]
})
];
}
Expand All @@ -218,12 +219,12 @@ function getFunctionCodeLens(file: Uri, functionsAndSuites: FunctionsAndSuites,
new CodeLens(range, {
title: `${getTestStatusIcons(functions)}${constants.Text.CodeLensRunUnitTest} (Multiple)`,
command: constants.Commands.Tests_Picker_UI,
arguments: [CommandSource.codelens, file, functions]
arguments: [undefined, CommandSource.codelens, file, functions]
}),
new CodeLens(range, {
title: `${getTestStatusIcons(functions)}${constants.Text.CodeLensDebugUnitTest} (Multiple)`,
command: constants.Commands.Tests_Picker_UI_Debug,
arguments: [CommandSource.codelens, file, functions]
arguments: [undefined, CommandSource.codelens, file, functions]
})
];
}
Expand Down
18 changes: 8 additions & 10 deletions src/client/unittests/common/testUtils.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
import { inject, injectable, named } from 'inversify';
import * as path from 'path';
import * as vscode from 'vscode';
import { Uri, workspace } from 'vscode';
import { window } from 'vscode';
import { commands, Uri, window, workspace } from 'vscode';
import * as constants from '../../common/constants';
import { IUnitTestSettings } from '../../common/types';
import { Product } from '../../common/types';
import { IUnitTestSettings, Product } from '../../common/types';
import { CommandSource } from './constants';
import { TestFlatteningVisitor } from './testVisitors/flatteningVisitor';
import { ITestVisitor, TestFile, TestFolder, TestProvider, Tests, TestSettingsPropertyNames, TestsToRun, UnitTestProduct } from './types';
import { ITestsHelper } from './types';
import { ITestsHelper, ITestVisitor, TestFile, TestFolder, TestProvider, Tests, TestSettingsPropertyNames, TestsToRun, UnitTestProduct } from './types';

export async function selectTestWorkspace(): Promise<Uri | undefined> {
if (!Array.isArray(workspace.workspaceFolders) || workspace.workspaceFolders.length === 0) {
Expand All @@ -24,9 +20,9 @@ export async function selectTestWorkspace(): Promise<Uri | undefined> {
}

export function displayTestErrorMessage(message: string) {
vscode.window.showErrorMessage(message, constants.Button_Text_Tests_View_Output).then(action => {
window.showErrorMessage(message, constants.Button_Text_Tests_View_Output).then(action => {
if (action === constants.Button_Text_Tests_View_Output) {
vscode.commands.executeCommand(constants.Commands.Tests_ViewOutput, CommandSource.ui);
commands.executeCommand(constants.Commands.Tests_ViewOutput, undefined, CommandSource.ui);
}
});

Expand All @@ -44,7 +40,7 @@ export function convertFileToPackage(filePath: string): string {

@injectable()
export class TestsHelper implements ITestsHelper {
constructor( @inject(ITestVisitor) @named('TestFlatteningVisitor') private flatteningVisitor: TestFlatteningVisitor) { }
constructor(@inject(ITestVisitor) @named('TestFlatteningVisitor') private flatteningVisitor: TestFlatteningVisitor) { }
public parseProviderName(product: UnitTestProduct): TestProvider {
switch (product) {
case Product.nosetest: return 'nosetest';
Expand Down Expand Up @@ -96,6 +92,7 @@ export class TestsHelper implements ITestsHelper {
public flattenTestFiles(testFiles: TestFile[]): Tests {
testFiles.forEach(testFile => this.flatteningVisitor.visitTestFile(testFile));

// tslint:disable-next-line:no-object-literal-type-assertion
const tests = <Tests>{
testFiles: testFiles,
testFunctions: this.flatteningVisitor.flattenedTestFunctions,
Expand Down Expand Up @@ -165,6 +162,7 @@ export class TestsHelper implements ITestsHelper {
if (testFns.length > 0) { return { testFunction: testFns }; }

// Just return this as a test file.
// tslint:disable-next-line:no-object-literal-type-assertion
return <TestsToRun>{ testFile: [{ name: name, nameToRun: name, functions: [], suites: [], xmlName: name, fullPath: '', time: 0 }] };
}
}
13 changes: 6 additions & 7 deletions src/client/unittests/display/picker.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as path from 'path';
import { QuickPickItem, Uri, window } from 'vscode';
import * as vscode from 'vscode';
import { commands, QuickPickItem, Uri, window } from 'vscode';
import * as constants from '../../common/constants';
import { CommandSource } from '../common/constants';
import { FlattenedTestFunction, ITestCollectionStorageService, TestFile, TestFunction, Tests, TestStatus, TestsToRun } from '../common/types';
Expand All @@ -10,7 +9,7 @@ export class TestDisplay {
public displayStopTestUI(workspace: Uri, message: string) {
window.showQuickPick([message]).then(item => {
if (item === message) {
vscode.commands.executeCommand(constants.Commands.Tests_Stop, workspace);
commands.executeCommand(constants.Commands.Tests_Stop, undefined, workspace);
}
});
}
Expand Down Expand Up @@ -194,7 +193,7 @@ function onItemSelected(cmdSource: CommandSource, wkspace: Uri, selection: TestI
}
let cmd = '';
// tslint:disable-next-line:no-any
const args: any[] = [cmdSource, wkspace];
const args: any[] = [undefined, cmdSource, wkspace];
switch (selection.type) {
case Type.Null: {
return;
Expand All @@ -221,13 +220,13 @@ function onItemSelected(cmdSource: CommandSource, wkspace: Uri, selection: TestI
}
case Type.RunMethod: {
cmd = constants.Commands.Tests_Run;
// tslint:disable-next-line:prefer-type-cast
// tslint:disable-next-line:prefer-type-cast no-object-literal-type-assertion
args.push({ testFunction: [selection.fn.testFunction] } as TestsToRun);
break;
}
case Type.DebugMethod: {
cmd = constants.Commands.Tests_Debug;
// tslint:disable-next-line:prefer-type-cast
// tslint:disable-next-line:prefer-type-cast no-object-literal-type-assertion
args.push({ testFunction: [selection.fn.testFunction] } as TestsToRun);
args.push(true);
break;
Expand All @@ -237,5 +236,5 @@ function onItemSelected(cmdSource: CommandSource, wkspace: Uri, selection: TestI
}
}

vscode.commands.executeCommand(cmd, ...args);
commands.executeCommand(cmd, ...args);
}
24 changes: 12 additions & 12 deletions src/client/unittests/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,34 +87,34 @@ function dispose() {
}
function registerCommands(): vscode.Disposable[] {
const disposables: Disposable[] = [];
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Discover, (cmdSource: CommandSource = CommandSource.commandPalette, resource?: Uri) => {
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Discover, (_, cmdSource: CommandSource = CommandSource.commandPalette, resource?: Uri) => {
// Ignore the exceptions returned.
// This command will be invoked else where in the extension.
// tslint:disable-next-line:no-empty
discoverTests(cmdSource, resource, true, true).catch(() => { });
}));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Run_Failed, (cmdSource: CommandSource = CommandSource.commandPalette, resource: Uri) => runTestsImpl(cmdSource, resource, undefined, true)));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Run_Failed, (_, cmdSource: CommandSource = CommandSource.commandPalette, resource: Uri) => runTestsImpl(cmdSource, resource, undefined, true)));
// tslint:disable-next-line:no-unnecessary-callback-wrapper
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Run, (cmdSource: CommandSource = CommandSource.commandPalette, file: Uri, testToRun?: TestsToRun) => runTestsImpl(cmdSource, file, testToRun)));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Debug, (cmdSource: CommandSource = CommandSource.commandPalette, file: Uri, testToRun: TestsToRun) => runTestsImpl(cmdSource, file, testToRun, false, true)));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Run, (_, cmdSource: CommandSource = CommandSource.commandPalette, file: Uri, testToRun?: TestsToRun) => runTestsImpl(cmdSource, file, testToRun)));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Debug, (_, cmdSource: CommandSource = CommandSource.commandPalette, file: Uri, testToRun: TestsToRun) => runTestsImpl(cmdSource, file, testToRun, false, true)));
// tslint:disable-next-line:no-unnecessary-callback-wrapper
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_View_UI, () => displayUI(CommandSource.commandPalette)));
// tslint:disable-next-line:no-unnecessary-callback-wrapper
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Picker_UI, (cmdSource: CommandSource = CommandSource.commandPalette, file: Uri, testFunctions: TestFunction[]) => displayPickerUI(cmdSource, file, testFunctions)));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Picker_UI_Debug, (cmdSource: CommandSource = CommandSource.commandPalette, file: Uri, testFunctions: TestFunction[]) => displayPickerUI(cmdSource, file, testFunctions, true)));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Picker_UI, (_, cmdSource: CommandSource = CommandSource.commandPalette, file: Uri, testFunctions: TestFunction[]) => displayPickerUI(cmdSource, file, testFunctions)));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Picker_UI_Debug, (_, cmdSource: CommandSource = CommandSource.commandPalette, file: Uri, testFunctions: TestFunction[]) => displayPickerUI(cmdSource, file, testFunctions, true)));
// tslint:disable-next-line:no-unnecessary-callback-wrapper
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Stop, (resource: Uri) => stopTests(resource)));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Stop, (_, resource: Uri) => stopTests(resource)));
// tslint:disable-next-line:no-unnecessary-callback-wrapper
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_ViewOutput, (cmdSource: CommandSource = CommandSource.commandPalette) => viewOutput(cmdSource)));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_ViewOutput, (_, cmdSource: CommandSource = CommandSource.commandPalette) => viewOutput(cmdSource)));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Ask_To_Stop_Discovery, () => displayStopUI('Stop discovering tests')));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Ask_To_Stop_Test, () => displayStopUI('Stop running tests')));
// tslint:disable-next-line:no-unnecessary-callback-wrapper
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Select_And_Run_Method, (cmdSource: CommandSource = CommandSource.commandPalette, resource: Uri) => selectAndRunTestMethod(cmdSource, resource)));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Select_And_Debug_Method, (cmdSource: CommandSource = CommandSource.commandPalette, resource: Uri) => selectAndRunTestMethod(cmdSource, resource, true)));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Select_And_Run_Method, (_, cmdSource: CommandSource = CommandSource.commandPalette, resource: Uri) => selectAndRunTestMethod(cmdSource, resource)));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Select_And_Debug_Method, (_, cmdSource: CommandSource = CommandSource.commandPalette, resource: Uri) => selectAndRunTestMethod(cmdSource, resource, true)));
// tslint:disable-next-line:no-unnecessary-callback-wrapper
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Select_And_Run_File, (cmdSource: CommandSource = CommandSource.commandPalette) => selectAndRunTestFile(cmdSource)));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Select_And_Run_File, (_, cmdSource: CommandSource = CommandSource.commandPalette) => selectAndRunTestFile(cmdSource)));
// tslint:disable-next-line:no-unnecessary-callback-wrapper
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Run_Current_File, (cmdSource: CommandSource = CommandSource.commandPalette) => runCurrentTestFile(cmdSource)));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Run_Current_File, (_, cmdSource: CommandSource = CommandSource.commandPalette) => runCurrentTestFile(cmdSource)));

return disposables;
}
Expand Down

0 comments on commit e2b9037

Please sign in to comment.