From 716fc44f3c572252a3a6fd68612d1b9f2cff385b Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 11 Sep 2023 11:38:27 -0700 Subject: [PATCH 01/14] handle subprocess segfaults for testAdapters --- .../testing/testController/common/server.ts | 22 +++- .../testing/testController/common/types.ts | 3 + .../testing/testController/common/utils.ts | 26 ++++- .../pytest/pytestExecutionAdapter.ts | 10 +- .../unittest/testDiscoveryAdapter.ts | 2 +- .../unittest/testExecutionAdapter.ts | 2 +- .../testing/common/testingAdapter.test.ts | 102 ++++++++++++++++++ .../errorWorkspace/test_seg_fault.py | 18 ++++ 8 files changed, 178 insertions(+), 7 deletions(-) create mode 100644 src/testTestingRootWkspc/errorWorkspace/test_seg_fault.py diff --git a/src/client/testing/testController/common/server.ts b/src/client/testing/testController/common/server.ts index 8797a861fb4a..fb6cf950f908 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,14 @@ export class PythonTestServer implements ITestServer, Disposable { return this._onDiscoveryDataReceived.event; } + public triggerDataReceivedEvent(payload: DataReceivedEvent): void { + this._onDataReceived.fire(payload); + } + + public triggerRunDataReceivedEvent(payload: DataReceivedEvent): void { + this._onRunDataReceived.fire(payload); + } + public dispose(): void { this.server.close(); this._onDataReceived.dispose(); @@ -146,6 +154,7 @@ export class PythonTestServer implements ITestServer, Disposable { options: TestCommandOptions, runTestIdPort?: string, runInstance?: TestRun, + testIds?: string[], callback?: () => void, ): Promise { const { uuid } = options; @@ -218,8 +227,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..a17102c231b6 100644 --- a/src/client/testing/testController/common/types.ts +++ b/src/client/testing/testController/common/types.ts @@ -178,12 +178,15 @@ 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; + triggerDataReceivedEvent(data: DataReceivedEvent): 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..33d87b9077a5 100644 --- a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts +++ b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts @@ -175,7 +175,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..43640f64922a 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -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; }); @@ -424,4 +430,100 @@ suite('End to End Tests: test adapters', () => { ); }); }); + test('unittest execution adapter seg fault error handling', async () => { + // generate list of test_ids + const testId = `test_seg_fault.TestSegmentationFault.test_segfault`; + const testIds: string[] = [testId]; + let foundId = ''; + 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, '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'); + [foundId] = Object.keys(data.result); + 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(() => { + // resolve execution should be called 200 times since there are 200 tests run. + resultResolver.verify( + (x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny()), + typeMoq.Times.exactly(1), + ); + assert.strictEqual(foundId, testId, 'Expected testId to be present'); + }); + }); + test('pytest execution adapter seg fault error handling', async () => { + // generate list of test_ids + const testId = `${rootPathErrorWorkspace}/test_seg_fault.py::TestSegmentationFault::test_segfault`; + const testIds: string[] = [testId]; + let foundId = ''; + 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, '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'); + [foundId] = Object.keys(data.result); + 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(() => { + // resolve execution should be called 200 times since there are 200 tests run. + resultResolver.verify( + (x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny()), + typeMoq.Times.exactly(1), + ); + assert.strictEqual(foundId, testId, 'Expected testId to be present'); + }); + }); }); 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() From 660f691a2df8480445ec17ea68caba69c4f1007f Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 11 Sep 2023 11:40:57 -0700 Subject: [PATCH 02/14] remove general dataReceived event getter --- src/client/testing/testController/common/server.ts | 4 ---- src/client/testing/testController/common/types.ts | 1 - 2 files changed, 5 deletions(-) diff --git a/src/client/testing/testController/common/server.ts b/src/client/testing/testController/common/server.ts index fb6cf950f908..d0a225ae5712 100644 --- a/src/client/testing/testController/common/server.ts +++ b/src/client/testing/testController/common/server.ts @@ -133,10 +133,6 @@ export class PythonTestServer implements ITestServer, Disposable { return this._onDiscoveryDataReceived.event; } - public triggerDataReceivedEvent(payload: DataReceivedEvent): void { - this._onDataReceived.fire(payload); - } - public triggerRunDataReceivedEvent(payload: DataReceivedEvent): void { this._onRunDataReceived.fire(payload); } diff --git a/src/client/testing/testController/common/types.ts b/src/client/testing/testController/common/types.ts index a17102c231b6..610c3d76d17b 100644 --- a/src/client/testing/testController/common/types.ts +++ b/src/client/testing/testController/common/types.ts @@ -185,7 +185,6 @@ export interface ITestServer { getPort(): number; createUUID(cwd: string): string; deleteUUID(uuid: string): void; - triggerDataReceivedEvent(data: DataReceivedEvent): void; triggerRunDataReceivedEvent(data: DataReceivedEvent): void; } export interface ITestResultResolver { From 93e94e626c8078254bdfb7cf0ba4765db4749e33 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 11 Sep 2023 13:12:40 -0700 Subject: [PATCH 03/14] add better assert note --- src/test/testing/common/testingAdapter.test.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index 43640f64922a..6c90bc46f40d 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -475,7 +475,11 @@ suite('End to End Tests: test adapters', () => { (x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny()), typeMoq.Times.exactly(1), ); - assert.strictEqual(foundId, testId, 'Expected testId to be present'); + assert.strictEqual( + foundId, + testId, + `Expected testId to be present, expected: ${testId} actual: ${foundId}`, + ); }); }); test('pytest execution adapter seg fault error handling', async () => { @@ -523,7 +527,11 @@ suite('End to End Tests: test adapters', () => { (x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny()), typeMoq.Times.exactly(1), ); - assert.strictEqual(foundId, testId, 'Expected testId to be present'); + assert.strictEqual( + foundId, + testId, + `Expected testId to be present, expected: ${testId} actual: ${foundId}`, + ); }); }); }); From 1ba5b823d64b69b73132ce0fbf2fda73f2be2609 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 11 Sep 2023 13:36:40 -0700 Subject: [PATCH 04/14] add logging --- .../testing/common/testingAdapter.test.ts | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index 6c90bc46f40d..dbd47f7ae1b4 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -92,16 +92,28 @@ 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'"); // 2. Confirm no errors assert.strictEqual(actualData.error, undefined, "Expected no errors in 'error' field"); // 3. Confirm tests are found + console.log(actualData.tests); assert.ok(actualData.tests, 'Expected tests to be present'); }); }); @@ -269,7 +281,7 @@ suite('End to End Tests: test adapters', () => { (x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny()), typeMoq.Times.once(), ); - + console.log('for testing, data result', JSON.stringify(actualData)); // 1. Check the status is "success" assert.strictEqual(actualData.status, 'success', "Expected status to be 'success'"); // 2. Confirm tests are found @@ -372,7 +384,7 @@ suite('End to End Tests: test adapters', () => { (x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny()), typeMoq.Times.once(), ); - + console.log('for testing, data result', JSON.stringify(actualData)); // 1. Check the status is "success" assert.strictEqual(actualData.status, 'success', "Expected status to be 'success'"); // 2. Confirm no errors @@ -431,7 +443,6 @@ suite('End to End Tests: test adapters', () => { }); }); test('unittest execution adapter seg fault error handling', async () => { - // generate list of test_ids const testId = `test_seg_fault.TestSegmentationFault.test_segfault`; const testIds: string[] = [testId]; let foundId = ''; @@ -447,6 +458,7 @@ suite('End to End Tests: test adapters', () => { // 3. Confirm tests are found assert.ok(data.result, 'Expected results to be present'); [foundId] = Object.keys(data.result); + console.log('for testing, data result', JSON.stringify(data)); return Promise.resolve(); }); @@ -470,7 +482,6 @@ suite('End to End Tests: test adapters', () => { } as any), ); await executionAdapter.runTests(workspaceUri, testIds, false, testRun.object).finally(() => { - // resolve execution should be called 200 times since there are 200 tests run. resultResolver.verify( (x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny()), typeMoq.Times.exactly(1), @@ -483,7 +494,6 @@ suite('End to End Tests: test adapters', () => { }); }); test('pytest execution adapter seg fault error handling', async () => { - // generate list of test_ids const testId = `${rootPathErrorWorkspace}/test_seg_fault.py::TestSegmentationFault::test_segfault`; const testIds: string[] = [testId]; let foundId = ''; @@ -522,7 +532,6 @@ suite('End to End Tests: test adapters', () => { } as any), ); await executionAdapter.runTests(workspaceUri, testIds, false, testRun.object, pythonExecFactory).finally(() => { - // resolve execution should be called 200 times since there are 200 tests run. resultResolver.verify( (x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny()), typeMoq.Times.exactly(1), From ca5d1e0b4d30c65fbba9aa464caae4dad6ba7fa4 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 11 Sep 2023 13:57:04 -0700 Subject: [PATCH 05/14] add another log --- src/client/testing/testController/common/resultResolver.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/client/testing/testController/common/resultResolver.ts b/src/client/testing/testController/common/resultResolver.ts index aa9f2a541f51..b1f46a200ce2 100644 --- a/src/client/testing/testController/common/resultResolver.ts +++ b/src/client/testing/testController/common/resultResolver.ts @@ -157,6 +157,9 @@ export class PythonResultResolver implements ITestResultResolver { ) { const grabTestItem = this.runIdToTestItem.get(keyTemp); const grabVSid = this.runIdToVSid.get(keyTemp); + console.log('grabTestItem', grabTestItem); + console.log('grabVSid', grabVSid); + console.log('key', keyTemp); if (grabTestItem !== undefined) { testCases.forEach((indiItem) => { if (indiItem.id === grabVSid) { From cc18eae75b42957627035132308d13eb209fc731 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 11 Sep 2023 14:27:34 -0700 Subject: [PATCH 06/14] logs --- src/test/testing/common/testingAdapter.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index dbd47f7ae1b4..fee72c8ec708 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -113,7 +113,7 @@ suite('End to End Tests: test adapters', () => { // 2. Confirm no errors assert.strictEqual(actualData.error, undefined, "Expected no errors in 'error' field"); // 3. Confirm tests are found - console.log(actualData.tests); + console.log('actual data', actualData.tests); assert.ok(actualData.tests, 'Expected tests to be present'); }); }); From 0a7339d98fd94e1901d2fb0a3a06085d56ed7dad Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 11 Sep 2023 15:25:38 -0700 Subject: [PATCH 07/14] logging --- src/client/testing/testController/common/server.ts | 2 ++ .../testing/testController/pytest/pytestExecutionAdapter.ts | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/client/testing/testController/common/server.ts b/src/client/testing/testController/common/server.ts index d0a225ae5712..80e796f8b356 100644 --- a/src/client/testing/testController/common/server.ts +++ b/src/client/testing/testController/common/server.ts @@ -219,9 +219,11 @@ export class PythonTestServer implements ITestServer, Disposable { // Displays output to user and ensure the subprocess doesn't run into buffer overflow. result?.proc?.stdout?.on('data', (data) => { spawnOptions?.outputChannel?.append(data.toString()); + console.log(data.toString()); }); result?.proc?.stderr?.on('data', (data) => { spawnOptions?.outputChannel?.append(data.toString()); + console.log(data.toString()); }); result?.proc?.on('exit', (code, signal) => { // if the child has testIds then this is a run request diff --git a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts index 33d87b9077a5..2912e9c202b8 100644 --- a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts +++ b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts @@ -130,7 +130,7 @@ 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}`); + console.log(`Running PYTEST execution for the following test ids: ${testIds}`); const pytestRunTestIdsPort = await utils.startTestIdServer(testIds); if (spawnOptions.extraVariables) @@ -170,9 +170,11 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { // Displays output to user and ensure the subprocess doesn't run into buffer overflow. result?.proc?.stdout?.on('data', (data) => { this.outputChannel?.append(data.toString()); + console.log(data.toString()); }); result?.proc?.stderr?.on('data', (data) => { this.outputChannel?.append(data.toString()); + console.log(data.toString()); }); result?.proc?.on('exit', (code, signal) => { From c480c7ea8f4228ad2b1fd12e2073ee250797f776 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 11 Sep 2023 15:27:23 -0700 Subject: [PATCH 08/14] fix --- src/test/testing/common/testingAdapter.test.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index fee72c8ec708..26f433b7d3da 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -209,7 +209,7 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveDiscovery(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - traceLog(`resolveDiscovery ${data}`); + console.log(`resolveDiscovery ${data}`); actualData = data; return Promise.resolve(); }); @@ -249,7 +249,7 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - traceLog(`resolveExecution ${data}`); + console.log(`resolveExecution ${data}`); actualData = data; return Promise.resolve(); }); @@ -293,7 +293,7 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - traceError(`resolveExecution ${data}`); + console.log(`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 @@ -346,7 +346,7 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - traceLog(`resolveExecution ${data}`); + console.log(`resolveExecution ${data}`); actualData = data; return Promise.resolve(); }); @@ -397,7 +397,7 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - traceLog(`resolveExecution ${data}`); + console.log(`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'"); @@ -449,7 +449,7 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - traceLog(`resolveExecution ${data}`); + console.log(`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, 'error', "Expected status to be 'error'"); @@ -500,7 +500,7 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - traceLog(`resolveExecution ${data}`); + console.log(`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, 'error', "Expected status to be 'error'"); From 00482bb3dcdac4db26d5236754e1077e345799b7 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 11 Sep 2023 15:33:48 -0700 Subject: [PATCH 09/14] linting --- .../testing/testController/pytest/pytestExecutionAdapter.ts | 2 +- src/test/testing/common/testingAdapter.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts index 2912e9c202b8..5b47e879f3f1 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, diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index 26f433b7d3da..42069492d572 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'; From 8b78140985176c616a90b8746c0415b3d1bb4427 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 11 Sep 2023 15:51:17 -0700 Subject: [PATCH 10/14] fix assert --- src/test/testing/common/testingAdapter.test.ts | 12 ++++++------ .../largeWorkspace/test_parameterized_subtest.py | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index 42069492d572..d9ee512984da 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -249,7 +249,7 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - console.log(`resolveExecution ${data}`); + console.log(`resolveExecution ${JSON.stringify(data)}`); actualData = data; return Promise.resolve(); }); @@ -293,7 +293,7 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - console.log(`resolveExecution ${data}`); + console.log(`resolveExecution ${JSON.stringify(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 @@ -346,7 +346,7 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - console.log(`resolveExecution ${data}`); + console.log(`resolveExecution ${JSON.stringify(data)}`); actualData = data; return Promise.resolve(); }); @@ -397,7 +397,7 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - console.log(`resolveExecution ${data}`); + console.log(`resolveExecution ${JSON.stringify(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'"); @@ -449,7 +449,7 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - console.log(`resolveExecution ${data}`); + console.log(`resolveExecution ${JSON.stringify(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'"); @@ -500,7 +500,7 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - console.log(`resolveExecution ${data}`); + console.log(`resolveExecution ${JSON.stringify(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'"); 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): From 1ddf5426f1aa19c48ff20e5f5e6c3c43254f0713 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Tue, 12 Sep 2023 08:55:16 -0700 Subject: [PATCH 11/14] change way of assert --- .../testing/common/testingAdapter.test.ts | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index d9ee512984da..d24b5be5d6f9 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -445,7 +445,6 @@ 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]; - let foundId = ''; resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { @@ -457,8 +456,12 @@ suite('End to End Tests: test adapters', () => { assert.ok(data.error, "Expected errors in 'error' field"); // 3. Confirm tests are found assert.ok(data.result, 'Expected results to be present'); - [foundId] = Object.keys(data.result); - console.log('for testing, data result', JSON.stringify(data)); + // 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(); }); @@ -486,17 +489,11 @@ suite('End to End Tests: test adapters', () => { (x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny()), typeMoq.Times.exactly(1), ); - assert.strictEqual( - foundId, - testId, - `Expected testId to be present, expected: ${testId} actual: ${foundId}`, - ); }); }); test('pytest execution adapter seg fault error handling', async () => { const testId = `${rootPathErrorWorkspace}/test_seg_fault.py::TestSegmentationFault::test_segfault`; const testIds: string[] = [testId]; - let foundId = ''; resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { @@ -508,7 +505,12 @@ suite('End to End Tests: test adapters', () => { assert.ok(data.error, "Expected errors in 'error' field"); // 3. Confirm tests are found assert.ok(data.result, 'Expected results to be present'); - [foundId] = Object.keys(data.result); + // 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(); }); @@ -536,11 +538,6 @@ suite('End to End Tests: test adapters', () => { (x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny()), typeMoq.Times.exactly(1), ); - assert.strictEqual( - foundId, - testId, - `Expected testId to be present, expected: ${testId} actual: ${foundId}`, - ); }); }); }); From 7bfbf45d55082969747f5e10238097c660e92158 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Tue, 12 Sep 2023 09:47:09 -0700 Subject: [PATCH 12/14] remove logging --- src/client/testing/testController/common/resultResolver.ts | 3 --- .../testController/pytest/pytestExecutionAdapter.ts | 3 --- src/test/testing/common/testingAdapter.test.ts | 7 ------- 3 files changed, 13 deletions(-) diff --git a/src/client/testing/testController/common/resultResolver.ts b/src/client/testing/testController/common/resultResolver.ts index b1f46a200ce2..aa9f2a541f51 100644 --- a/src/client/testing/testController/common/resultResolver.ts +++ b/src/client/testing/testController/common/resultResolver.ts @@ -157,9 +157,6 @@ export class PythonResultResolver implements ITestResultResolver { ) { const grabTestItem = this.runIdToTestItem.get(keyTemp); const grabVSid = this.runIdToVSid.get(keyTemp); - console.log('grabTestItem', grabTestItem); - console.log('grabVSid', grabVSid); - console.log('key', keyTemp); if (grabTestItem !== undefined) { testCases.forEach((indiItem) => { if (indiItem.id === grabVSid) { diff --git a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts index 5b47e879f3f1..9705374d74af 100644 --- a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts +++ b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts @@ -130,7 +130,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { if (debugBool && !testArgs.some((a) => a.startsWith('--capture') || a === '-s')) { testArgs.push('--capture', 'no'); } - console.log(`Running PYTEST execution for the following test ids: ${testIds}`); const pytestRunTestIdsPort = await utils.startTestIdServer(testIds); if (spawnOptions.extraVariables) @@ -170,11 +169,9 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { // Displays output to user and ensure the subprocess doesn't run into buffer overflow. result?.proc?.stdout?.on('data', (data) => { this.outputChannel?.append(data.toString()); - console.log(data.toString()); }); result?.proc?.stderr?.on('data', (data) => { this.outputChannel?.append(data.toString()); - console.log(data.toString()); }); result?.proc?.on('exit', (code, signal) => { diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index d24b5be5d6f9..9d6519a8c54d 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -113,7 +113,6 @@ suite('End to End Tests: test adapters', () => { // 2. Confirm no errors assert.strictEqual(actualData.error, undefined, "Expected no errors in 'error' field"); // 3. Confirm tests are found - console.log('actual data', actualData.tests); assert.ok(actualData.tests, 'Expected tests to be present'); }); }); @@ -249,7 +248,6 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - console.log(`resolveExecution ${JSON.stringify(data)}`); actualData = data; return Promise.resolve(); }); @@ -293,7 +291,6 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - console.log(`resolveExecution ${JSON.stringify(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 @@ -346,7 +343,6 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - console.log(`resolveExecution ${JSON.stringify(data)}`); actualData = data; return Promise.resolve(); }); @@ -397,7 +393,6 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - console.log(`resolveExecution ${JSON.stringify(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'"); @@ -448,7 +443,6 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - console.log(`resolveExecution ${JSON.stringify(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'"); @@ -497,7 +491,6 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - console.log(`resolveExecution ${JSON.stringify(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'"); From 01c035c7bb9d5a7914acd6ee29902d3c592c3b85 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Tue, 12 Sep 2023 09:47:58 -0700 Subject: [PATCH 13/14] remove console log --- src/client/testing/testController/common/server.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/client/testing/testController/common/server.ts b/src/client/testing/testController/common/server.ts index 80e796f8b356..d0a225ae5712 100644 --- a/src/client/testing/testController/common/server.ts +++ b/src/client/testing/testController/common/server.ts @@ -219,11 +219,9 @@ export class PythonTestServer implements ITestServer, Disposable { // Displays output to user and ensure the subprocess doesn't run into buffer overflow. result?.proc?.stdout?.on('data', (data) => { spawnOptions?.outputChannel?.append(data.toString()); - console.log(data.toString()); }); result?.proc?.stderr?.on('data', (data) => { spawnOptions?.outputChannel?.append(data.toString()); - console.log(data.toString()); }); result?.proc?.on('exit', (code, signal) => { // if the child has testIds then this is a run request From 8cf937cdae52dc9e8bcacc46b535e02dea1fbe94 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Tue, 12 Sep 2023 11:00:54 -0700 Subject: [PATCH 14/14] forgotten logs --- src/test/testing/common/testingAdapter.test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index 9d6519a8c54d..5af7e59f6a46 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -208,7 +208,6 @@ suite('End to End Tests: test adapters', () => { resultResolver .setup((x) => x.resolveDiscovery(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns((data) => { - console.log(`resolveDiscovery ${data}`); actualData = data; return Promise.resolve(); }); @@ -279,7 +278,6 @@ suite('End to End Tests: test adapters', () => { (x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny()), typeMoq.Times.once(), ); - console.log('for testing, data result', JSON.stringify(actualData)); // 1. Check the status is "success" assert.strictEqual(actualData.status, 'success', "Expected status to be 'success'"); // 2. Confirm tests are found @@ -380,7 +378,6 @@ suite('End to End Tests: test adapters', () => { (x) => x.resolveExecution(typeMoq.It.isAny(), typeMoq.It.isAny()), typeMoq.Times.once(), ); - console.log('for testing, data result', JSON.stringify(actualData)); // 1. Check the status is "success" assert.strictEqual(actualData.status, 'success', "Expected status to be 'success'"); // 2. Confirm no errors