From 72f7ef8113536d35fa000c8a75f7aae598575990 Mon Sep 17 00:00:00 2001 From: Eleanor Boyd Date: Thu, 25 May 2023 13:41:55 -0700 Subject: [PATCH] Set up testing rewrite experiment (#21258) is the beginning of this issue: https://github.com/microsoft/vscode-python/issues/21150, in that it will start the process of implementing the setting in the extension --- src/client/common/experiments/groups.ts | 4 +++ src/client/testing/common/debugLauncher.ts | 17 +++++----- .../testing/testController/common/utils.ts | 10 ++++++ .../testing/testController/controller.ts | 21 ++++++------ .../testController/pytest/pytestController.ts | 31 +++++++++-------- .../unittest/unittestController.ts | 33 +++++++++++-------- .../testing/common/debugLauncher.unit.test.ts | 29 +++++++++------- 7 files changed, 88 insertions(+), 57 deletions(-) diff --git a/src/client/common/experiments/groups.ts b/src/client/common/experiments/groups.ts index 5884aafd122d..1ee06469095c 100644 --- a/src/client/common/experiments/groups.ts +++ b/src/client/common/experiments/groups.ts @@ -14,3 +14,7 @@ export enum TerminalEnvVarActivation { export enum ShowFormatterExtensionPrompt { experiment = 'pythonPromptNewFormatterExt', } +// Experiment to enable the new testing rewrite. +export enum EnableTestAdapterRewrite { + experiment = 'pythonTestAdapter', +} diff --git a/src/client/testing/common/debugLauncher.ts b/src/client/testing/common/debugLauncher.ts index af233132768c..0c13671b6e0d 100644 --- a/src/client/testing/common/debugLauncher.ts +++ b/src/client/testing/common/debugLauncher.ts @@ -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 { @@ -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); @@ -171,10 +173,11 @@ export class DebugLauncher implements ITestDebugLauncher { workspaceFolder: WorkspaceFolder, options: LaunchOptions, ): Promise { + 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; @@ -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, @@ -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 diff --git a/src/client/testing/testController/common/utils.ts b/src/client/testing/testController/common/utils.ts index 88e3450d35dc..1bf31e80e11a 100644 --- a/src/client/testing/testController/common/utils.ts +++ b/src/client/testing/testController/common/utils.ts @@ -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`; @@ -52,6 +56,12 @@ export function jsonRPCContent(headers: Map, rawData: string): I remainingRawData, }; } + +export function pythonTestAdapterRewriteEnabled(serviceContainer: IServiceContainer): boolean { + const experiment = serviceContainer.get(IExperimentService); + return experiment.inExperimentSync(EnableTestAdapterRewrite.experiment); +} + export const startServer = (testIds: string): Promise => new Promise((resolve, reject) => { const server = net.createServer((socket: net.Socket) => { diff --git a/src/client/testing/testController/controller.ts b/src/client/testing/testController/controller.ts index 20fcfa49bb69..6c6b9f409e5f 100644 --- a/src/client/testing/testController/controller.ts +++ b/src/client/testing/testController/controller.ts @@ -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]; @@ -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(); @@ -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 = @@ -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 = @@ -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); @@ -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); diff --git a/src/client/testing/testController/pytest/pytestController.ts b/src/client/testing/testController/pytest/pytestController.ts index 793170231210..997e3e29b7ec 100644 --- a/src/client/testing/testController/pytest/pytestController.ts +++ b/src/client/testing/testController/pytest/pytestController.ts @@ -285,19 +285,24 @@ export class PytestController implements ITestFrameworkController { public runTests(testRun: ITestRun, workspace: WorkspaceFolder, token: CancellationToken): Promise { 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 { + 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}`); + } } } diff --git a/src/client/testing/testController/unittest/unittestController.ts b/src/client/testing/testController/unittest/unittestController.ts index ee79103c4e3e..a795620f3ca0 100644 --- a/src/client/testing/testController/unittest/unittestController.ts +++ b/src/client/testing/testController/unittest/unittestController.ts @@ -251,20 +251,25 @@ export class UnittestController implements ITestFrameworkController { testController?: TestController, ): Promise { 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}`); + } } } diff --git a/src/test/testing/common/debugLauncher.unit.test.ts b/src/test/testing/common/debugLauncher.unit.test.ts index b8b7d5c55130..41b95c66040e 100644 --- a/src/test/testing/common/debugLauncher.unit.test.ts +++ b/src/test/testing/common/debugLauncher.unit.test.ts @@ -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); @@ -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 () => { @@ -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(undefined, TypeMoq.MockBehavior.Strict); appShell.setup((a) => a.showErrorMessage(TypeMoq.It.isAny())).returns(() => Promise.resolve(undefined)); @@ -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', @@ -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);