diff --git a/src/client/testing/testController/common/server.ts b/src/client/testing/testController/common/server.ts index 8797a861fb4a..d0a225ae5712 100644 --- a/src/client/testing/testController/common/server.ts +++ b/src/client/testing/testController/common/server.ts @@ -15,7 +15,7 @@ import { traceError, traceInfo, traceLog } from '../../../logging'; import { DataReceivedEvent, ITestServer, TestCommandOptions } from './types'; import { ITestDebugLauncher, LaunchOptions } from '../../common/types'; import { UNITTEST_PROVIDER } from '../../common/constants'; -import { jsonRPCHeaders, jsonRPCContent, JSONRPC_UUID_HEADER } from './utils'; +import { jsonRPCHeaders, jsonRPCContent, JSONRPC_UUID_HEADER, createExecutionErrorPayload } from './utils'; import { createDeferred } from '../../../common/utils/async'; export class PythonTestServer implements ITestServer, Disposable { @@ -133,6 +133,10 @@ export class PythonTestServer implements ITestServer, Disposable { return this._onDiscoveryDataReceived.event; } + public triggerRunDataReceivedEvent(payload: DataReceivedEvent): void { + this._onRunDataReceived.fire(payload); + } + public dispose(): void { this.server.close(); this._onDataReceived.dispose(); @@ -146,6 +150,7 @@ export class PythonTestServer implements ITestServer, Disposable { options: TestCommandOptions, runTestIdPort?: string, runInstance?: TestRun, + testIds?: string[], callback?: () => void, ): Promise { const { uuid } = options; @@ -218,8 +223,15 @@ export class PythonTestServer implements ITestServer, Disposable { result?.proc?.stderr?.on('data', (data) => { spawnOptions?.outputChannel?.append(data.toString()); }); - result?.proc?.on('exit', () => { - traceLog('Exec server closed.', uuid); + result?.proc?.on('exit', (code, signal) => { + // if the child has testIds then this is a run request + if (code !== 0 && testIds) { + // if the child process exited with a non-zero exit code, then we need to send the error payload. + this._onRunDataReceived.fire({ + uuid, + data: JSON.stringify(createExecutionErrorPayload(code, signal, testIds, options.cwd)), + }); + } deferred.resolve({ stdout: '', stderr: '' }); callback?.(); }); diff --git a/src/client/testing/testController/common/types.ts b/src/client/testing/testController/common/types.ts index 16c0bd0e3cee..610c3d76d17b 100644 --- a/src/client/testing/testController/common/types.ts +++ b/src/client/testing/testController/common/types.ts @@ -178,12 +178,14 @@ export interface ITestServer { options: TestCommandOptions, runTestIdsPort?: string, runInstance?: TestRun, + testIds?: string[], callback?: () => void, ): Promise; serverReady(): Promise; getPort(): number; createUUID(cwd: string): string; deleteUUID(uuid: string): void; + triggerRunDataReceivedEvent(data: DataReceivedEvent): void; } export interface ITestResultResolver { runIdToVSid: Map; diff --git a/src/client/testing/testController/common/utils.ts b/src/client/testing/testController/common/utils.ts index f98550d3e72b..dd1b51551a45 100644 --- a/src/client/testing/testController/common/utils.ts +++ b/src/client/testing/testController/common/utils.ts @@ -9,7 +9,7 @@ import { EnableTestAdapterRewrite } from '../../../common/experiments/groups'; import { IExperimentService } from '../../../common/types'; import { IServiceContainer } from '../../../ioc/types'; import { DebugTestTag, ErrorTestItemOptions, RunTestTag } from './testItemUtilities'; -import { DiscoveredTestItem, DiscoveredTestNode, ITestResultResolver } from './types'; +import { DiscoveredTestItem, DiscoveredTestNode, ExecutionTestPayload, ITestResultResolver } from './types'; export function fixLogLines(content: string): string { const lines = content.split(/\r?\n/g); @@ -188,3 +188,27 @@ export function populateTestTree( function isTestItem(test: DiscoveredTestNode | DiscoveredTestItem): test is DiscoveredTestItem { return test.type_ === 'test'; } + +export function createExecutionErrorPayload( + code: number | null, + signal: NodeJS.Signals | null, + testIds: string[], + cwd: string, +): ExecutionTestPayload { + const etp: ExecutionTestPayload = { + cwd, + status: 'error', + error: 'Test run failed, the python test process was terminated before it could exit on its own.', + result: {}, + }; + // add error result for each attempted test. + for (let i = 0; i < testIds.length; i = i + 1) { + const test = testIds[i]; + etp.result![test] = { + test, + outcome: 'error', + message: ` \n The python test process was terminated before it could exit on its own, the process errored with: Code: ${code}, Signal: ${signal}`, + }; + } + return etp; +} diff --git a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts index 96d53db22c1c..9705374d74af 100644 --- a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts +++ b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts @@ -5,7 +5,7 @@ import { TestRun, Uri } from 'vscode'; import * as path from 'path'; import { IConfigurationService, ITestOutputChannel } from '../../../common/types'; import { createDeferred } from '../../../common/utils/async'; -import { traceError, traceInfo, traceLog, traceVerbose } from '../../../logging'; +import { traceError, traceInfo, traceVerbose } from '../../../logging'; import { DataReceivedEvent, ExecutionTestPayload, @@ -130,7 +130,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { if (debugBool && !testArgs.some((a) => a.startsWith('--capture') || a === '-s')) { testArgs.push('--capture', 'no'); } - traceLog(`Running PYTEST execution for the following test ids: ${testIds}`); const pytestRunTestIdsPort = await utils.startTestIdServer(testIds); if (spawnOptions.extraVariables) @@ -175,7 +174,15 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { this.outputChannel?.append(data.toString()); }); - result?.proc?.on('exit', () => { + result?.proc?.on('exit', (code, signal) => { + // if the child has testIds then this is a run request + if (code !== 0 && testIds) { + // if the child process exited with a non-zero exit code, then we need to send the error payload. + this.testServer.triggerRunDataReceivedEvent({ + uuid, + data: JSON.stringify(utils.createExecutionErrorPayload(code, signal, testIds, cwd)), + }); + } deferredExec.resolve({ stdout: '', stderr: '' }); deferred.resolve(); disposeDataReceiver?.(this.testServer); diff --git a/src/client/testing/testController/unittest/testDiscoveryAdapter.ts b/src/client/testing/testController/unittest/testDiscoveryAdapter.ts index 1cbad7ef65ef..9820aa89626c 100644 --- a/src/client/testing/testController/unittest/testDiscoveryAdapter.ts +++ b/src/client/testing/testController/unittest/testDiscoveryAdapter.ts @@ -64,7 +64,7 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter { } private async callSendCommand(options: TestCommandOptions, callback: () => void): Promise { - await this.testServer.sendCommand(options, undefined, undefined, callback); + await this.testServer.sendCommand(options, undefined, undefined, [], callback); const discoveryPayload: DiscoveredTestPayload = { cwd: '', status: 'success' }; return discoveryPayload; } diff --git a/src/client/testing/testController/unittest/testExecutionAdapter.ts b/src/client/testing/testController/unittest/testExecutionAdapter.ts index 9af9e593c246..bc5e41d19d9d 100644 --- a/src/client/testing/testController/unittest/testExecutionAdapter.ts +++ b/src/client/testing/testController/unittest/testExecutionAdapter.ts @@ -83,7 +83,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { const runTestIdsPort = await startTestIdServer(testIds); - await this.testServer.sendCommand(options, runTestIdsPort.toString(), runInstance, () => { + await this.testServer.sendCommand(options, runTestIdsPort.toString(), runInstance, testIds, () => { deferred.resolve(); disposeDataReceiver?.(this.testServer); }); diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index 1334085e4cea..5af7e59f6a46 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -13,7 +13,7 @@ import { ITestDebugLauncher } from '../../../client/testing/common/types'; import { IConfigurationService, ITestOutputChannel } from '../../../client/common/types'; import { IServiceContainer } from '../../../client/ioc/types'; import { EXTENSION_ROOT_DIR_FOR_TESTS, initialize } from '../../initialize'; -import { traceError, traceLog } from '../../../client/logging'; +import { traceLog } from '../../../client/logging'; import { PytestTestExecutionAdapter } from '../../../client/testing/testController/pytest/pytestExecutionAdapter'; import { UnittestTestDiscoveryAdapter } from '../../../client/testing/testController/unittest/testDiscoveryAdapter'; import { UnittestTestExecutionAdapter } from '../../../client/testing/testController/unittest/testExecutionAdapter'; @@ -39,6 +39,12 @@ suite('End to End Tests: test adapters', () => { 'testTestingRootWkspc', 'largeWorkspace', ); + const rootPathErrorWorkspace = path.join( + EXTENSION_ROOT_DIR_FOR_TESTS, + 'src', + 'testTestingRootWkspc', + 'errorWorkspace', + ); suiteSetup(async () => { serviceContainer = (await initialize()).serviceContainer; }); @@ -86,10 +92,21 @@ suite('End to End Tests: test adapters', () => { await discoveryAdapter.discoverTests(workspaceUri).finally(() => { // verification after discovery is complete - resultResolver.verify( - (x) => x.resolveDiscovery(typeMoq.It.isAny(), typeMoq.It.isAny()), - typeMoq.Times.once(), - ); + // resultResolver.verify( + // (x) => x.resolveDiscovery(typeMoq.It.isAny(), typeMoq.It.isAny()), + // typeMoq.Times.once(), + // ); + + // 1. Check the status is "success" + assert.strictEqual(actualData.status, 'success', "Expected status to be 'success'"); + // 2. Confirm no errors + assert.strictEqual(actualData.error, undefined, "Expected no errors in 'error' field"); + // 3. Confirm tests are found + assert.ok(actualData.tests, 'Expected tests to be present'); + }); + + await discoveryAdapter.discoverTests(Uri.parse(rootPathErrorWorkspace)).finally(() => { + // verification after discovery is complete // 1. Check the status is "success" assert.strictEqual(actualData.status, 'success', "Expected status to be 'success'"); @@ -191,7 +208,6 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveDiscovery(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - traceLog(`resolveDiscovery ${data}`); actualData = data; return Promise.resolve(); }); @@ -231,7 +247,6 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - traceLog(`resolveExecution ${data}`); actualData = data; return Promise.resolve(); }); @@ -263,7 +278,6 @@ suite('End to End Tests: test adapters', () => { (x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny()), typeMoq.Times.once(), ); - // 1. Check the status is "success" assert.strictEqual(actualData.status, 'success', "Expected status to be 'success'"); // 2. Confirm tests are found @@ -275,7 +289,6 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - traceError(`resolveExecution ${data}`); traceLog(`resolveExecution ${data}`); // do the following asserts for each time resolveExecution is called, should be called once per test. // 1. Check the status, can be subtest success or failure @@ -328,7 +341,6 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - traceLog(`resolveExecution ${data}`); actualData = data; return Promise.resolve(); }); @@ -366,7 +378,6 @@ suite('End to End Tests: test adapters', () => { (x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny()), typeMoq.Times.once(), ); - // 1. Check the status is "success" assert.strictEqual(actualData.status, 'success', "Expected status to be 'success'"); // 2. Confirm no errors @@ -379,7 +390,6 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - traceLog(`resolveExecution ${data}`); // do the following asserts for each time resolveExecution is called, should be called once per test. // 1. Check the status is "success" assert.strictEqual(data.status, 'success', "Expected status to be 'success'"); @@ -424,4 +434,100 @@ suite('End to End Tests: test adapters', () => { ); }); }); + test('unittest execution adapter seg fault error handling', async () => { + const testId = `test_seg_fault.TestSegmentationFault.test_segfault`; + const testIds: string[] = [testId]; + resultResolver + .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) + .returns((data) => { + // do the following asserts for each time resolveExecution is called, should be called once per test. + // 1. Check the status is "success" + assert.strictEqual(data.status, 'error', "Expected status to be 'error'"); + // 2. Confirm no errors + assert.ok(data.error, "Expected errors in 'error' field"); + // 3. Confirm tests are found + assert.ok(data.result, 'Expected results to be present'); + // 4. make sure the testID is found in the results + assert.notDeepEqual( + JSON.stringify(data).search('test_seg_fault.TestSegmentationFault.test_segfault'), + -1, + 'Expected testId to be present', + ); + return Promise.resolve(); + }); + + // set workspace to test workspace folder + workspaceUri = Uri.parse(rootPathErrorWorkspace); + + // run pytest execution + const executionAdapter = new UnittestTestExecutionAdapter( + pythonTestServer, + configService, + testOutputChannel, + resultResolver.object, + ); + const testRun = typeMoq.Mock.ofType(); + testRun + .setup((t) => t.token) + .returns( + () => + ({ + onCancellationRequested: () => undefined, + } as any), + ); + await executionAdapter.runTests(workspaceUri, testIds, false, testRun.object).finally(() => { + resultResolver.verify( + (x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny()), + typeMoq.Times.exactly(1), + ); + }); + }); + test('pytest execution adapter seg fault error handling', async () => { + const testId = `${rootPathErrorWorkspace}/test_seg_fault.py::TestSegmentationFault::test_segfault`; + const testIds: string[] = [testId]; + resultResolver + .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) + .returns((data) => { + // do the following asserts for each time resolveExecution is called, should be called once per test. + // 1. Check the status is "success" + assert.strictEqual(data.status, 'error', "Expected status to be 'error'"); + // 2. Confirm no errors + assert.ok(data.error, "Expected errors in 'error' field"); + // 3. Confirm tests are found + assert.ok(data.result, 'Expected results to be present'); + // 4. make sure the testID is found in the results + assert.notDeepEqual( + JSON.stringify(data).search('test_seg_fault.py::TestSegmentationFault::test_segfault'), + -1, + 'Expected testId to be present', + ); + return Promise.resolve(); + }); + + // set workspace to test workspace folder + workspaceUri = Uri.parse(rootPathErrorWorkspace); + + // run pytest execution + const executionAdapter = new PytestTestExecutionAdapter( + pythonTestServer, + configService, + testOutputChannel, + resultResolver.object, + ); + const testRun = typeMoq.Mock.ofType(); + testRun + .setup((t) => t.token) + .returns( + () => + ({ + onCancellationRequested: () => undefined, + } as any), + ); + await executionAdapter.runTests(workspaceUri, testIds, false, testRun.object, pythonExecFactory).finally(() => { + resultResolver.verify( + (x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny()), + typeMoq.Times.exactly(1), + ); + }); + }); }); diff --git a/src/testTestingRootWkspc/errorWorkspace/test_seg_fault.py b/src/testTestingRootWkspc/errorWorkspace/test_seg_fault.py new file mode 100644 index 000000000000..bad7ff8fcbbd --- /dev/null +++ b/src/testTestingRootWkspc/errorWorkspace/test_seg_fault.py @@ -0,0 +1,18 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +import unittest +import ctypes + + +class TestSegmentationFault(unittest.TestCase): + def cause_segfault(self): + ctypes.string_at(0) # Dereference a NULL pointer + + def test_segfault(self): + assert True + self.cause_segfault() + + +if __name__ == "__main__": + unittest.main() diff --git a/src/testTestingRootWkspc/largeWorkspace/test_parameterized_subtest.py b/src/testTestingRootWkspc/largeWorkspace/test_parameterized_subtest.py index 3e84df0a2d9f..8c6a29adf495 100644 --- a/src/testTestingRootWkspc/largeWorkspace/test_parameterized_subtest.py +++ b/src/testTestingRootWkspc/largeWorkspace/test_parameterized_subtest.py @@ -6,7 +6,7 @@ @pytest.mark.parametrize("num", range(0, 200)) def test_odd_even(num): - return num % 2 == 0 + assert num % 2 == 0 class NumbersTest(unittest.TestCase):