Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Set up testing rewrite experiment #21258

Merged
merged 8 commits into from
May 25, 2023
Merged

Set up testing rewrite experiment #21258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b0d8472
switch rewrite to be behind an experiment
eleanorjboyd May 16, 2023
e03d8e7
fix placement of rewrite experiment check
eleanorjboyd May 16, 2023
188b461
switch to bool
eleanorjboyd May 16, 2023
95dd4a8
fix tests
eleanorjboyd May 16, 2023
1faad0a
add run failure telemetry
eleanorjboyd May 16, 2023
fe1efab
Merge branch 'main' into testing-rewrite-experiment
eleanorjboyd May 25, 2023
64b4353
Merge branch 'main' into testing-rewrite-experiment
eleanorjboyd May 25, 2023
0b23a0a
format fix
eleanorjboyd May 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/client/common/experiments/groups.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,7 @@ export enum TerminalEnvVarActivation {
export enum ShowFormatterExtensionPrompt {
experiment = 'pythonPromptNewFormatterExt',
}
// Experiment to enable the new testing rewrite.
export enum EnableTestAdapterRewrite {
experiment = 'pythonTestAdapter',
}
17 changes: 8 additions & 9 deletions src/client/testing/common/debugLauncher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { getConfigurationsForWorkspace } from '../../debugger/extension/configur
import { getWorkspaceFolder, getWorkspaceFolders } from '../../common/vscodeApis/workspaceApis';
import { showErrorMessage } from '../../common/vscodeApis/windowApis';
import { createDeferred } from '../../common/utils/async';
import { pythonTestAdapterRewriteEnabled } from '../testController/common/utils';

@injectable()
export class DebugLauncher implements ITestDebugLauncher {
Expand Down Expand Up @@ -87,6 +88,7 @@ export class DebugLauncher implements ITestDebugLauncher {
path: path.join(EXTENSION_ROOT_DIR, 'pythonFiles'),
include: false,
});

DebugLauncher.applyDefaults(debugConfig!, workspaceFolder, configSettings);

return this.convertConfigToArgs(debugConfig!, workspaceFolder, options);
Expand Down Expand Up @@ -171,10 +173,11 @@ export class DebugLauncher implements ITestDebugLauncher {
workspaceFolder: WorkspaceFolder,
options: LaunchOptions,
): Promise<LaunchRequestArguments> {
const pythonTestAdapterRewriteExperiment = pythonTestAdapterRewriteEnabled(this.serviceContainer);
const configArgs = debugConfig as LaunchRequestArguments;
const testArgs =
options.testProvider === 'unittest' ? options.args.filter((item) => item !== '--debug') : options.args;
const script = DebugLauncher.getTestLauncherScript(options.testProvider);
const script = DebugLauncher.getTestLauncherScript(options.testProvider, pythonTestAdapterRewriteExperiment);
const args = script(testArgs);
const [program] = args;
configArgs.program = program;
Expand All @@ -199,10 +202,7 @@ export class DebugLauncher implements ITestDebugLauncher {
throw Error(`Invalid debug config "${debugConfig.name}"`);
}
launchArgs.request = 'launch';

// If we are in the pytest rewrite then we must send additional environment variables.
const rewriteTestingEnabled = process.env.ENABLE_PYTHON_TESTING_REWRITE;
if (options.testProvider === 'pytest' && rewriteTestingEnabled) {
if (options.testProvider === 'pytest' && pythonTestAdapterRewriteExperiment) {
if (options.pytestPort && options.pytestUUID) {
launchArgs.env = {
...launchArgs.env,
Expand All @@ -226,17 +226,16 @@ export class DebugLauncher implements ITestDebugLauncher {
return launchArgs;
}

private static getTestLauncherScript(testProvider: TestProvider) {
const rewriteTestingEnabled = process.env.ENABLE_PYTHON_TESTING_REWRITE;
private static getTestLauncherScript(testProvider: TestProvider, pythonTestAdapterRewriteExperiment?: boolean) {
switch (testProvider) {
case 'unittest': {
if (rewriteTestingEnabled) {
if (pythonTestAdapterRewriteExperiment) {
return internalScripts.execution_py_testlauncher; // this is the new way to run unittest execution, debugger
}
return internalScripts.visualstudio_py_testlauncher; // old way unittest execution, debugger
}
case 'pytest': {
if (rewriteTestingEnabled) {
if (pythonTestAdapterRewriteExperiment) {
return internalScripts.pytestlauncher; // this is the new way to run pytest execution, debugger
}
return internalScripts.testlauncher; // old way pytest execution, debugger
Expand Down
10 changes: 10 additions & 0 deletions src/client/testing/testController/common/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
import * as net from 'net';
import { traceLog } from '../../../logging';

import { EnableTestAdapterRewrite } from '../../../common/experiments/groups';
import { IExperimentService } from '../../../common/types';
import { IServiceContainer } from '../../../ioc/types';

export function fixLogLines(content: string): string {
const lines = content.split(/\r?\n/g);
return `${lines.join('\r\n')}\r\n`;
Expand Down Expand Up @@ -52,6 +56,12 @@ export function jsonRPCContent(headers: Map<string, string>, rawData: string): I
remainingRawData,
};
}

export function pythonTestAdapterRewriteEnabled(serviceContainer: IServiceContainer): boolean {
const experiment = serviceContainer.get<IExperimentService>(IExperimentService);
return experiment.inExperimentSync(EnableTestAdapterRewrite.experiment);
}

export const startServer = (testIds: string): Promise<number> =>
new Promise((resolve, reject) => {
const server = net.createServer((socket: net.Socket) => {
Expand Down
21 changes: 11 additions & 10 deletions src/client/testing/testController/controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ import { PytestTestDiscoveryAdapter } from './pytest/pytestDiscoveryAdapter';
import { PytestTestExecutionAdapter } from './pytest/pytestExecutionAdapter';
import { WorkspaceTestAdapter } from './workspaceTestAdapter';
import { ITestDebugLauncher } from '../common/types';
import { pythonTestAdapterRewriteEnabled } from './common/utils';
import { IServiceContainer } from '../../ioc/types';

// Types gymnastics to make sure that sendTriggerTelemetry only accepts the correct types.
type EventPropertyType = IEventNamePropertyMapping[EventName.UNITTEST_DISCOVERY_TRIGGER];
Expand Down Expand Up @@ -93,6 +95,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
@inject(IPythonExecutionFactory) private readonly pythonExecFactory: IPythonExecutionFactory,
@inject(ITestDebugLauncher) private readonly debugLauncher: ITestDebugLauncher,
@inject(ITestOutputChannel) private readonly testOutputChannel: ITestOutputChannel,
@inject(IServiceContainer) private readonly serviceContainer: IServiceContainer,
) {
this.refreshCancellation = new CancellationTokenSource();

Expand Down Expand Up @@ -241,12 +244,11 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
if (uri) {
const settings = this.configSettings.getSettings(uri);
traceVerbose(`Testing: Refreshing test data for ${uri.fsPath}`);
const rewriteTestingEnabled = process.env.ENABLE_PYTHON_TESTING_REWRITE;
if (settings.testing.pytestEnabled) {
// Ensure we send test telemetry if it gets disabled again
this.sendTestDisabledTelemetry = true;
if (rewriteTestingEnabled) {
// ** rewriteTestingEnabled set to true to use NEW test discovery mechanism
// ** experiment to roll out NEW test discovery mechanism
if (pythonTestAdapterRewriteEnabled(this.serviceContainer)) {
const workspace = this.workspaceService.getWorkspaceFolder(uri);
traceVerbose(`Discover tests for workspace name: ${workspace?.name} - uri: ${uri.fsPath}`);
const testAdapter =
Expand All @@ -263,8 +265,8 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
} else if (settings.testing.unittestEnabled) {
// ** Ensure we send test telemetry if it gets disabled again
this.sendTestDisabledTelemetry = true;
if (rewriteTestingEnabled) {
// ** rewriteTestingEnabled set to true to use NEW test discovery mechanism
// ** experiment to roll out NEW test discovery mechanism
if (pythonTestAdapterRewriteEnabled(this.serviceContainer)) {
const workspace = this.workspaceService.getWorkspaceFolder(uri);
traceVerbose(`Discover tests for workspace name: ${workspace?.name} - uri: ${uri.fsPath}`);
const testAdapter =
Expand Down Expand Up @@ -388,14 +390,13 @@ export class PythonTestController implements ITestController, IExtensionSingleAc

const settings = this.configSettings.getSettings(workspace.uri);
if (testItems.length > 0) {
const rewriteTestingEnabled = process.env.ENABLE_PYTHON_TESTING_REWRITE;
if (settings.testing.pytestEnabled) {
sendTelemetryEvent(EventName.UNITTEST_RUN, undefined, {
tool: 'pytest',
debugging: request.profile?.kind === TestRunProfileKind.Debug,
});
// ** rewriteTestingEnabled set to true to use NEW test discovery mechanism
if (rewriteTestingEnabled) {
// ** experiment to roll out NEW test discovery mechanism
if (pythonTestAdapterRewriteEnabled(this.serviceContainer)) {
const testAdapter =
this.testAdapters.get(workspace.uri) ||
(this.testAdapters.values().next().value as WorkspaceTestAdapter);
Expand Down Expand Up @@ -425,8 +426,8 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
tool: 'unittest',
debugging: request.profile?.kind === TestRunProfileKind.Debug,
});
// ** rewriteTestingEnabled set to true to use NEW test discovery mechanism
if (rewriteTestingEnabled) {
// ** experiment to roll out NEW test discovery mechanism
if (pythonTestAdapterRewriteEnabled(this.serviceContainer)) {
const testAdapter =
this.testAdapters.get(workspace.uri) ||
(this.testAdapters.values().next().value as WorkspaceTestAdapter);
Expand Down
31 changes: 18 additions & 13 deletions src/client/testing/testController/pytest/pytestController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,19 +285,24 @@ export class PytestController implements ITestFrameworkController {

public runTests(testRun: ITestRun, workspace: WorkspaceFolder, token: CancellationToken): Promise<void> {
const settings = this.configService.getSettings(workspace.uri);
return this.runner.runTests(
testRun,
{
workspaceFolder: workspace.uri,
cwd:
settings.testing.cwd && settings.testing.cwd.length > 0
? settings.testing.cwd
: workspace.uri.fsPath,
token,
args: settings.testing.pytestArgs,
},
this.idToRawData,
);
try {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@karthiknadig thoughts on adding this to gather infrastructure exceptions?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a good idea for me.

return this.runner.runTests(
testRun,
{
workspaceFolder: workspace.uri,
cwd:
settings.testing.cwd && settings.testing.cwd.length > 0
? settings.testing.cwd
: workspace.uri.fsPath,
token,
args: settings.testing.pytestArgs,
},
this.idToRawData,
);
} catch (ex) {
sendTelemetryEvent(EventName.UNITTEST_RUN_ALL_FAILED, undefined);
throw new Error(`Failed to run tests: ${ex}`);
}
}
}

Expand Down
33 changes: 19 additions & 14 deletions src/client/testing/testController/unittest/unittestController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,20 +251,25 @@ export class UnittestController implements ITestFrameworkController {
testController?: TestController,
): Promise<void> {
const settings = this.configService.getSettings(workspace.uri);
return this.runner.runTests(
testRun,
{
workspaceFolder: workspace.uri,
cwd:
settings.testing.cwd && settings.testing.cwd.length > 0
? settings.testing.cwd
: workspace.uri.fsPath,
token,
args: settings.testing.unittestArgs,
},
this.idToRawData,
testController,
);
try {
return this.runner.runTests(
testRun,
{
workspaceFolder: workspace.uri,
cwd:
settings.testing.cwd && settings.testing.cwd.length > 0
? settings.testing.cwd
: workspace.uri.fsPath,
token,
args: settings.testing.unittestArgs,
},
this.idToRawData,
testController,
);
} catch (ex) {
sendTelemetryEvent(EventName.UNITTEST_RUN_ALL_FAILED, undefined);
throw new Error(`Failed to run tests: ${ex}`);
}
}
}

Expand Down
29 changes: 18 additions & 11 deletions src/test/testing/common/debugLauncher.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { ITestingSettings } from '../../../client/testing/configuration/types';
import { TestProvider } from '../../../client/testing/types';
import { isOs, OSType } from '../../common';
import { IEnvironmentActivationService } from '../../../client/interpreter/activation/types';
import * as util from '../../../client/testing/testController/common/utils';

use(chaiAsPromised);

Expand All @@ -45,6 +46,7 @@ suite('Unit Tests - Debug Launcher', () => {
let getWorkspaceFoldersStub: sinon.SinonStub;
let pathExistsStub: sinon.SinonStub;
let readFileStub: sinon.SinonStub;
let pythonTestAdapterRewriteEnabledStub: sinon.SinonStub;
const envVars = { FOO: 'BAR' };

setup(async () => {
Expand All @@ -65,6 +67,8 @@ suite('Unit Tests - Debug Launcher', () => {
getWorkspaceFoldersStub = sinon.stub(workspaceApis, 'getWorkspaceFolders');
pathExistsStub = sinon.stub(fs, 'pathExists');
readFileStub = sinon.stub(fs, 'readFile');
pythonTestAdapterRewriteEnabledStub = sinon.stub(util, 'pythonTestAdapterRewriteEnabled');
pythonTestAdapterRewriteEnabledStub.returns(false);

const appShell = TypeMoq.Mock.ofType<IApplicationShell>(undefined, TypeMoq.MockBehavior.Strict);
appShell.setup((a) => a.showErrorMessage(TypeMoq.It.isAny())).returns(() => Promise.resolve(undefined));
Expand Down Expand Up @@ -143,19 +147,22 @@ suite('Unit Tests - Debug Launcher', () => {
uri: Uri.file(folderPath),
};
}
function getTestLauncherScript(testProvider: TestProvider) {
switch (testProvider) {
case 'unittest': {
return path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'visualstudio_py_testlauncher.py');
}
case 'pytest': {
return path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'testlauncher.py');
}
default: {
throw new Error(`Unknown test provider '${testProvider}'`);
function getTestLauncherScript(testProvider: TestProvider, pythonTestAdapterRewriteExperiment?: boolean) {
if (!pythonTestAdapterRewriteExperiment) {
switch (testProvider) {
case 'unittest': {
return path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'visualstudio_py_testlauncher.py');
}
case 'pytest': {
return path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'testlauncher.py');
}
default: {
throw new Error(`Unknown test provider '${testProvider}'`);
}
}
}
}

function getDefaultDebugConfig(): DebugConfiguration {
return {
name: 'Debug Unit Test',
Expand All @@ -178,7 +185,7 @@ suite('Unit Tests - Debug Launcher', () => {
expected?: DebugConfiguration,
debugConfigs?: string | DebugConfiguration[],
) {
const testLaunchScript = getTestLauncherScript(testProvider);
const testLaunchScript = getTestLauncherScript(testProvider, false);

const workspaceFolders = [createWorkspaceFolder(options.cwd), createWorkspaceFolder('five/six/seven')];
getWorkspaceFoldersStub.returns(workspaceFolders);
Expand Down