Skip to content

Commit

Permalink
fix debugging with new pytest run script (#21299)
Browse files Browse the repository at this point in the history
fix debugging for run_pytest_script.py setup
  • Loading branch information
eleanorjboyd authored May 25, 2023
1 parent b916981 commit 4109228
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 18 deletions.
7 changes: 7 additions & 0 deletions src/client/common/process/internal/scripts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,13 @@ export function testlauncher(testArgs: string[]): string[] {
return [script, ...testArgs];
}

// run_pytest_script.py
export function pytestlauncher(testArgs: string[]): string[] {
const script = path.join(SCRIPTS_DIR, 'vscode_pytest', 'run_pytest_script.py');
// There is no output to parse, so we do not return a function.
return [script, ...testArgs];
}

// visualstudio_py_testlauncher.py

// eslint-disable-next-line camelcase
Expand Down
13 changes: 6 additions & 7 deletions src/client/testing/common/debugLauncher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,7 @@ export class DebugLauncher implements ITestDebugLauncher {
const args = script(testArgs);
const [program] = args;
configArgs.program = program;
// if the test provider is pytest, then use the pytest module instead of using a program
const rewriteTestingEnabled = process.env.ENABLE_PYTHON_TESTING_REWRITE;
if (options.testProvider === 'pytest' && rewriteTestingEnabled) {
configArgs.module = 'pytest';
configArgs.program = undefined;
}

configArgs.args = args.slice(1);
// We leave configArgs.request as "test" so it will be sent in telemetry.

Expand All @@ -204,12 +199,16 @@ 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.pytestPort && options.pytestUUID) {
launchArgs.env = {
...launchArgs.env,
TEST_PORT: options.pytestPort,
TEST_UUID: options.pytestUUID,
RUN_TEST_IDS_PORT: options.pytestRunTestIdsPort,
};
} else {
throw Error(
Expand Down Expand Up @@ -238,7 +237,7 @@ export class DebugLauncher implements ITestDebugLauncher {
}
case 'pytest': {
if (rewriteTestingEnabled) {
return (testArgs: string[]) => testArgs;
return internalScripts.pytestlauncher; // this is the new way to run pytest execution, debugger
}
return internalScripts.testlauncher; // old way pytest execution, debugger
}
Expand Down
1 change: 1 addition & 0 deletions src/client/testing/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export type LaunchOptions = {
outChannel?: OutputChannel;
pytestPort?: string;
pytestUUID?: string;
pytestRunTestIdsPort?: string;
};

export type ParserOptions = TestDiscoveryOptions;
Expand Down
26 changes: 15 additions & 11 deletions src/client/testing/testController/pytest/pytestExecutionAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as path from 'path';
import * as net from 'net';
import { IConfigurationService, ITestOutputChannel } from '../../../common/types';
import { createDeferred, Deferred } from '../../../common/utils/async';
import { traceLog, traceVerbose } from '../../../logging';
import { traceError, traceLog, traceVerbose } from '../../../logging';
import { DataReceivedEvent, ExecutionTestPayload, ITestExecutionAdapter, ITestServer } from '../common/types';
import {
ExecutionFactoryCreateWithEnvironmentOptions,
Expand Down Expand Up @@ -112,18 +112,16 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
testArgs.splice(0, 0, '--rootdir', uri.fsPath);
}

// why is this needed?
if (debugBool && !testArgs.some((a) => a.startsWith('--capture') || a === '-s')) {
testArgs.push('--capture', 'no');
}
const pluginArgs = ['-p', 'vscode_pytest'].concat(testArgs).concat(testIds);
const scriptPath = path.join(fullPluginPath, 'vscode_pytest', 'run_pytest_script.py');
const runArgs = [scriptPath, ...testArgs];

// create payload with testIds to send to run pytest script
const testData = JSON.stringify(testIds);
const headers = [`Content-Length: ${Buffer.byteLength(testData)}`, 'Content-Type: application/json'];
const payload = `${headers.join('\r\n')}\r\n\r\n${testData}`;

let pytestRunTestIdsPort: string | undefined;
const startServer = (): Promise<number> =>
new Promise((resolve, reject) => {
const server = net.createServer((socket: net.Socket) => {
Expand Down Expand Up @@ -151,34 +149,40 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
await startServer()
.then((assignedPort) => {
traceLog(`Server started and listening on port ${assignedPort}`);
pytestRunTestIdsPort = assignedPort.toString();
if (spawnOptions.extraVariables)
spawnOptions.extraVariables.RUN_TEST_IDS_PORT = assignedPort.toString();
spawnOptions.extraVariables.RUN_TEST_IDS_PORT = pytestRunTestIdsPort;
})
.catch((error) => {
console.error('Error starting server:', error);
traceError('Error starting server:', error);
});

if (debugBool) {
const pytestPort = this.testServer.getPort().toString();
const pytestUUID = uuid.toString();
const launchOptions: LaunchOptions = {
cwd: uri.fsPath,
args: pluginArgs,
args: testArgs,
token: spawnOptions.token,
testProvider: PYTEST_PROVIDER,
pytestPort,
pytestUUID,
pytestRunTestIdsPort,
};
console.debug(`Running debug test with arguments: ${pluginArgs.join(' ')}\r\n`);
traceVerbose(`Running debug test with arguments: ${testArgs.join(' ')}\r\n`);
await debugLauncher!.launchDebugger(launchOptions);
} else {
// combine path to run script with run args
const scriptPath = path.join(fullPluginPath, 'vscode_pytest', 'run_pytest_script.py');
const runArgs = [scriptPath, ...testArgs];

await execService?.exec(runArgs, spawnOptions).catch((ex) => {
console.debug(`Error while running tests: ${testIds}\r\n${ex}\r\n\r\n`);
traceError(`Error while running tests: ${testIds}\r\n${ex}\r\n\r\n`);
return Promise.reject(ex);
});
}
} catch (ex) {
console.debug(`Error while running tests: ${testIds}\r\n${ex}\r\n\r\n`);
traceVerbose(`Error while running tests: ${testIds}\r\n${ex}\r\n\r\n`);
return Promise.reject(ex);
}

Expand Down

0 comments on commit 4109228

Please sign in to comment.