From c3f13a77e306c4a04ab2aace6835b738b3d2b42b Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Thu, 30 Sep 2021 14:42:33 -0700 Subject: [PATCH 1/4] Support for subtest run results. --- pythonFiles/visualstudio_py_testlauncher.py | 25 ++++-- .../common/testItemUtilities.ts | 20 ++++- .../testing/testController/common/types.ts | 14 ++- .../testing/testController/controller.ts | 12 ++- .../testing/testController/unittest/runner.ts | 89 +++++++++++++++++-- .../unittest/unittestController.ts | 8 +- types/vscode.proposed.d.ts | 51 ++++++++--- 7 files changed, 190 insertions(+), 29 deletions(-) diff --git a/pythonFiles/visualstudio_py_testlauncher.py b/pythonFiles/visualstudio_py_testlauncher.py index c22c0d5d2eda..7f80dfa3ba88 100644 --- a/pythonFiles/visualstudio_py_testlauncher.py +++ b/pythonFiles/visualstudio_py_testlauncher.py @@ -168,7 +168,13 @@ def addUnexpectedSuccess(self, test): super(VsTestResult, self).addUnexpectedSuccess(test) self.sendResult(test, "passed-unexpected") - def sendResult(self, test, outcome, trace=None): + def addSubTest(self, test, subtest, err): + super(VsTestResult, self).addSubTest(test, subtest, err) + self.sendResult( + test, "subtest-passed" if err is None else "subtest-failed", err, subtest + ) + + def sendResult(self, test, outcome, trace=None, subtest=None): if _channel is not None: tb = None message = None @@ -179,13 +185,16 @@ def sendResult(self, test, outcome, trace=None): formatted = formatted[1:] tb = "".join(formatted) message = str(trace[1]) - _channel.send_event( - name="result", - outcome=outcome, - traceback=tb, - message=message, - test=test.id(), - ) + + result = { + "outcome": outcome, + "traceback": tb, + "message": message, + "test": test.id(), + } + if subtest is not None: + result["subtest"] = subtest.id() + _channel.send_event("result", **result) def stopTests(): diff --git a/src/client/testing/testController/common/testItemUtilities.ts b/src/client/testing/testController/common/testItemUtilities.ts index 392e73de7406..2f27fe8d64d7 100644 --- a/src/client/testing/testController/common/testItemUtilities.ts +++ b/src/client/testing/testController/common/testItemUtilities.ts @@ -11,6 +11,7 @@ import { TestRunResult, TestResultState, TestResultSnapshot, + TestTag, } from 'vscode'; import { CancellationToken } from 'vscode-jsonrpc'; import { traceError, traceVerbose } from '../../../common/logger'; @@ -25,6 +26,9 @@ import { TestDataKinds, } from './types'; +export const RunTestTag = new TestTag('python-run'); +export const DebugTestTag = new TestTag('python-debug'); + export function removeItemByIdFromChildren( idToRawData: Map, item: TestItem, @@ -43,6 +47,7 @@ export function createErrorTestItem( const testItem = testController.createTestItem(options.id, options.label); testItem.canResolveChildren = false; testItem.error = options.error; + testItem.tags = [RunTestTag, DebugTestTag]; return testItem; } @@ -58,6 +63,7 @@ export function createWorkspaceRootTestItem( rawId: options.rawId ?? options.id, kind: TestDataKinds.Workspace, }); + testItem.tags = [RunTestTag, DebugTestTag]; return testItem; } @@ -125,6 +131,7 @@ function createFolderOrFileTestItem( kind: TestDataKinds.FolderOrFile, parentId, }); + testItem.tags = [RunTestTag, DebugTestTag]; return testItem; } @@ -151,6 +158,7 @@ function updateFolderOrFileTestItem( kind: TestDataKinds.FolderOrFile, parentId, }); + item.tags = [RunTestTag, DebugTestTag]; } function createCollectionTestItem( @@ -184,6 +192,7 @@ function createCollectionTestItem( kind: TestDataKinds.Collection, parentId, }); + testItem.tags = [RunTestTag, DebugTestTag]; return testItem; } @@ -213,6 +222,7 @@ function updateCollectionTestItem( kind: TestDataKinds.Collection, parentId, }); + item.tags = [RunTestTag, DebugTestTag]; } function createTestCaseItem( @@ -249,6 +259,7 @@ function createTestCaseItem( kind: TestDataKinds.Case, parentId, }); + testItem.tags = [RunTestTag, DebugTestTag]; return testItem; } @@ -279,6 +290,7 @@ function updateTestCaseItem( kind: TestDataKinds.Case, parentId, }); + item.tags = [RunTestTag, DebugTestTag]; } function updateTestItemFromRawDataInternal( @@ -475,7 +487,7 @@ export function getUri(node: TestItem): Uri | undefined { } export function getTestCaseNodes(testNode: TestItem, collection: TestItem[] = []): TestItem[] { - if (!testNode.canResolveChildren) { + if (!testNode.canResolveChildren && testNode.tags.length > 0) { collection.push(testNode); } @@ -551,3 +563,9 @@ export function checkForFailedTests(resultMap: Map): bo ) !== undefined ); } + +export function clearAllChildren(testNode: TestItem): void { + const ids: string[] = []; + testNode.children.forEach((c) => ids.push(c.id)); + ids.forEach(testNode.children.delete); +} diff --git a/src/client/testing/testController/common/types.ts b/src/client/testing/testController/common/types.ts index 1480086f3a40..709625049a46 100644 --- a/src/client/testing/testController/common/types.ts +++ b/src/client/testing/testController/common/types.ts @@ -61,12 +61,22 @@ export const ITestFrameworkController = Symbol('ITestFrameworkController'); export interface ITestFrameworkController { resolveChildren(testController: TestController, item: TestItem, token?: CancellationToken): Promise; refreshTestData(testController: TestController, resource?: Uri, token?: CancellationToken): Promise; - runTests(testRun: ITestRun, workspace: WorkspaceFolder, token: CancellationToken): Promise; + runTests( + testRun: ITestRun, + workspace: WorkspaceFolder, + token: CancellationToken, + testController?: TestController, + ): Promise; } export const ITestsRunner = Symbol('ITestsRunner'); export interface ITestsRunner { - runTests(testRun: ITestRun, options: TestRunOptions, idToRawData: Map): Promise; + runTests( + testRun: ITestRun, + options: TestRunOptions, + idToRawData: Map, + testController?: TestController, + ): Promise; } export type TestRunOptions = { diff --git a/src/client/testing/testController/controller.ts b/src/client/testing/testController/controller.ts index bcd61391a8d3..3c2551154d7f 100644 --- a/src/client/testing/testController/controller.ts +++ b/src/client/testing/testController/controller.ts @@ -23,7 +23,7 @@ import { DelayedTrigger, IDelayedTrigger } from '../../common/utils/delayTrigger import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { PYTEST_PROVIDER, UNITTEST_PROVIDER } from '../common/constants'; -import { getNodeByUri } from './common/testItemUtilities'; +import { DebugTestTag, getNodeByUri, RunTestTag } from './common/testItemUtilities'; import { ITestController, ITestFrameworkController, TestRefreshOptions } from './common/types'; @injectable() @@ -74,12 +74,19 @@ export class PythonTestController implements ITestController { this.refreshData = delayTrigger; this.disposables.push( - this.testController.createRunProfile('Run Tests', TestRunProfileKind.Run, this.runTests.bind(this), true), + this.testController.createRunProfile( + 'Run Tests', + TestRunProfileKind.Run, + this.runTests.bind(this), + true, + RunTestTag, + ), this.testController.createRunProfile( 'Debug Tests', TestRunProfileKind.Debug, this.runTests.bind(this), true, + DebugTestTag, ), ); this.testController.resolveHandler = this.resolveChildren.bind(this); @@ -244,6 +251,7 @@ export class PythonTestController implements ITestController { }, workspace, token, + this.testController, ); } } diff --git a/src/client/testing/testController/unittest/runner.ts b/src/client/testing/testController/unittest/runner.ts index 52d7c4ad774c..fb1482c2e9b4 100644 --- a/src/client/testing/testController/unittest/runner.ts +++ b/src/client/testing/testController/unittest/runner.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { injectable, inject, named } from 'inversify'; -import { Location, TestItem, TestMessage, TestRun, TestRunProfileKind } from 'vscode'; +import { Location, TestController, TestItem, TestMessage, TestRun, TestRunProfileKind } from 'vscode'; import { traceError, traceInfo } from '../../../common/logger'; import * as internalScripts from '../../../common/process/internal/scripts'; import { IOutputChannel } from '../../../common/types'; @@ -10,7 +10,7 @@ import { noop } from '../../../common/utils/misc'; import { UNITTEST_PROVIDER } from '../../common/constants'; import { ITestRunner, ITestDebugLauncher, IUnitTestSocketServer, LaunchOptions, Options } from '../../common/types'; import { TEST_OUTPUT_CHANNEL } from '../../constants'; -import { getTestCaseNodes } from '../common/testItemUtilities'; +import { clearAllChildren, getTestCaseNodes } from '../common/testItemUtilities'; import { ITestRun, ITestsRunner, TestData, TestRunInstanceOptions, TestRunOptions } from '../common/types'; import { fixLogLines } from '../common/utils'; import { getTestRunArgs } from './arguments'; @@ -20,6 +20,7 @@ interface ITestData { message: string; outcome: string; traceback: string; + subtest?: string; } @injectable() @@ -35,6 +36,7 @@ export class UnittestRunner implements ITestsRunner { testRun: ITestRun, options: TestRunOptions, idToRawData: Map, + testController?: TestController, ): Promise { const runOptions: TestRunInstanceOptions = { ...options, @@ -43,7 +45,7 @@ export class UnittestRunner implements ITestsRunner { }; try { - await this.runTest(testRun.includes, testRun.runInstance, runOptions, idToRawData); + await this.runTest(testRun.includes, testRun.runInstance, runOptions, idToRawData, testController); } catch (ex) { testRun.runInstance.appendOutput(`Error while running tests:\r\n${ex}\r\n\r\n`); } @@ -54,6 +56,7 @@ export class UnittestRunner implements ITestsRunner { runInstance: TestRun, options: TestRunInstanceOptions, idToRawData: Map, + testController?: TestController, ): Promise { runInstance.appendOutput(`Running tests (unittest): ${testNodes.map((t) => t.id).join(' ; ')}\r\n`); const testCaseNodes: TestItem[] = []; @@ -77,12 +80,13 @@ export class UnittestRunner implements ITestsRunner { const tested: string[] = []; const counts = { - total: testCaseNodes.length, + total: 0, passed: 0, skipped: 0, errored: 0, failed: 0, }; + const subTestStats: Map = new Map(); let failFast = false; let stopTesting = false; @@ -98,6 +102,7 @@ export class UnittestRunner implements ITestsRunner { const testCase = testCaseNodes.find((node) => idToRawData.get(node.id)?.runId === data.test); const rawTestCase = idToRawData.get(testCase?.id ?? ''); if (testCase && rawTestCase) { + counts.total += 1; tested.push(rawTestCase.runId); if (data.outcome === 'passed' || data.outcome === 'failed-expected') { @@ -147,6 +152,70 @@ export class UnittestRunner implements ITestsRunner { runInstance.skipped(testCase); runInstance.appendOutput(fixLogLines(text)); counts.skipped += 1; + } else if (data.outcome === 'subtest-passed') { + const sub = subTestStats.get(data.test); + if (sub) { + sub.passed += 1; + } else { + counts.passed += 1; + subTestStats.set(data.test, { passed: 1, failed: 0 }); + runInstance.appendOutput(fixLogLines(`${rawTestCase.rawId} [subtests]:\r\n`)); + + // We are seeing the first subtest for this node. Clear all other nodes under it + // because we have no way to detect these at discovery, they can always be different + // for each run. + clearAllChildren(testCase); + } + if (data.subtest) { + runInstance.appendOutput(fixLogLines(`${data.subtest} Passed\r\n`)); + + // This is a runtime only node for unittest subtest, since they can only be detected + // at runtime. So, create a fresh one for each result. + const subtest = testController?.createTestItem(data.subtest, data.subtest); + if (subtest) { + testCase.children.add(subtest); + runInstance.started(subtest); + runInstance.passed(subtest); + } + } + } else if (data.outcome === 'subtest-failed') { + const sub = subTestStats.get(data.test); + if (sub) { + sub.failed += 1; + } else { + counts.failed += 1; + subTestStats.set(data.test, { passed: 0, failed: 1 }); + + runInstance.appendOutput(fixLogLines(`${rawTestCase.rawId} [subtests]:\r\n`)); + + // We are seeing the first subtest for this node. Clear all other nodes under it + // because we have no way to detect these at discovery, they can always be different + // for each run. + clearAllChildren(testCase); + } + + if (data.subtest) { + runInstance.appendOutput(fixLogLines(`${data.subtest} Failed\r\n`)); + const traceback = data.traceback + ? data.traceback.splitLines({ trim: false, removeEmptyEntries: true }).join('\r\n') + : ''; + const text = `${data.subtest} Failed: ${data.message ?? data.outcome}\r\n${traceback}\r\n`; + runInstance.appendOutput(fixLogLines(text)); + + // This is a runtime only node for unittest subtest, since they can only be detected + // at runtime. So, create a fresh one for each result. + const subtest = testController?.createTestItem(data.subtest, data.subtest); + if (subtest) { + testCase.children.add(subtest); + runInstance.started(subtest); + const message = new TestMessage(text); + if (testCase.uri && testCase.range) { + message.location = new Location(testCase.uri, testCase.range); + } + + runInstance.failed(subtest, message); + } + } } else { const text = `Unknown outcome type for test ${rawTestCase.rawId}: ${data.outcome}`; runInstance.appendOutput(fixLogLines(text)); @@ -233,7 +302,17 @@ export class UnittestRunner implements ITestsRunner { runInstance.appendOutput(`Total number of tests passed: ${counts.passed}\r\n`); runInstance.appendOutput(`Total number of tests failed: ${counts.failed}\r\n`); runInstance.appendOutput(`Total number of tests failed with errors: ${counts.errored}\r\n`); - runInstance.appendOutput(`Total number of tests skipped: ${counts.skipped}\r\n`); + runInstance.appendOutput(`Total number of tests skipped: ${counts.skipped}\r\n\r\n`); + + if (subTestStats.size > 0) { + runInstance.appendOutput('Sub-test stats: \r\n'); + } + + subTestStats.forEach((v, k) => { + runInstance.appendOutput( + `Sub-tests for [${k}]: Total=${v.passed + v.failed} Passed=${v.passed} Failed=${v.failed}\r\n\r\n`, + ); + }); if (failFast) { runInstance.appendOutput( diff --git a/src/client/testing/testController/unittest/unittestController.ts b/src/client/testing/testController/unittest/unittestController.ts index 05e39218e41c..f05f9833599e 100644 --- a/src/client/testing/testController/unittest/unittestController.ts +++ b/src/client/testing/testController/unittest/unittestController.ts @@ -240,7 +240,12 @@ export class UnittestController implements ITestFrameworkController { return Promise.resolve(); } - public runTests(testRun: ITestRun, workspace: WorkspaceFolder, token: CancellationToken): Promise { + public runTests( + testRun: ITestRun, + workspace: WorkspaceFolder, + token: CancellationToken, + testController?: TestController, + ): Promise { const settings = this.configService.getSettings(workspace.uri); return this.runner.runTests( testRun, @@ -254,6 +259,7 @@ export class UnittestController implements ITestFrameworkController { args: settings.testing.unittestArgs, }, this.idToRawData, + testController, ); } } diff --git a/types/vscode.proposed.d.ts b/types/vscode.proposed.d.ts index 3550b5c667e9..3f817992a0ef 100644 --- a/types/vscode.proposed.d.ts +++ b/types/vscode.proposed.d.ts @@ -751,16 +751,16 @@ declare module 'vscode' { } export interface QuickPick extends QuickInput { - /** - * An optional flag to sort the final results by index of first query match in label. Defaults to true. - */ - sortByLabel: boolean; + /** + * An optional flag to sort the final results by index of first query match in label. Defaults to true. + */ + sortByLabel: boolean; - /* - * An optional flag that can be set to true to maintain the scroll position of the quick pick when the quick pick items are updated. Defaults to false. - */ - keepScrollPosition?: boolean; - } + /* + * An optional flag that can be set to true to maintain the scroll position of the quick pick when the quick pick items are updated. Defaults to false. + */ + keepScrollPosition?: boolean; + } export enum NotebookCellExecutionState { Idle = 1, @@ -1090,7 +1090,11 @@ declare module 'vscode' { * user can configure this. */ isDefault: boolean; - + /** + * Associated tag for the profile. If this is set, only {@link TestItem} + * instances with the same tag will be eligible to execute in this profile. + */ + tag?: TestTag; /** * If this method is present, a configuration gear will be present in the * UI, and this method will be invoked when it's clicked. When called, @@ -1158,6 +1162,7 @@ declare module 'vscode' { * @param kind Configures what kind of execution this profile manages. * @param runHandler Function called to start a test run. * @param isDefault Whether this is the default action for its kind. + * @param tag Profile test tag. * @returns An instance of a {@link TestRunProfile}, which is automatically * associated with this controller. */ @@ -1166,6 +1171,7 @@ declare module 'vscode' { kind: TestRunProfileKind, runHandler: (request: TestRunRequest, token: CancellationToken) => Thenable | void, isDefault?: boolean, + tag?: TestTag, ): TestRunProfile; /** @@ -1467,6 +1473,12 @@ declare module 'vscode' { * discovery, such as syntax errors. */ error?: string | MarkdownString; + + /** + * Tags associated with this test item. May be used in combination with + * {@link TestRunProfile.tags}, or simply as an organizational feature. + */ + tags: readonly TestTag[]; } /** @@ -1535,6 +1547,25 @@ declare module 'vscode' { readonly results: ReadonlyArray>; } + /** + * Tags can be associated with {@link TestItem TestItems} and + * {@link TestRunProfile TestRunProfiles}. A profile with a tag can only + * execute tests that include that tag in their {@link TestItem.tags} array. + */ + export class TestTag { + /** + * ID of the test tag. `TestTag` instances with the same ID are considered + * to be identical. + */ + readonly id: string; + + /** + * Creates a new TestTag instance. + * @param id ID of the test tag. + */ + constructor(id: string); + } + /** * A {@link TestItem}-like interface with an associated result, which appear * or can be provided in {@link TestResult} interfaces. From 008eb731795e4be88657bc60874cc6a86d5e75cd Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Thu, 30 Sep 2021 15:40:28 -0700 Subject: [PATCH 2/4] Add news --- news/2 Fixes/17561.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/2 Fixes/17561.md diff --git a/news/2 Fixes/17561.md b/news/2 Fixes/17561.md new file mode 100644 index 000000000000..79e6e8b47446 --- /dev/null +++ b/news/2 Fixes/17561.md @@ -0,0 +1 @@ +Ensures test node is updated when `unittest` sub-tests are used. From 90b14a1707a4fc77b223af33569a44f4a584bf65 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 5 Oct 2021 10:03:43 -0700 Subject: [PATCH 3/4] Ensure engine version supports testtags --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6c64757e71de..94ed3c9614ab 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ "theme": "dark" }, "engines": { - "vscode": "^1.59.0" + "vscode": "^1.61.0" }, "keywords": [ "python", From fa91367c771b9ae6836b6a78ac594e81c5a72ce8 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Fri, 8 Oct 2021 12:06:40 -0700 Subject: [PATCH 4/4] Don't use proposed constructor --- .../testing/testController/common/testItemUtilities.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/client/testing/testController/common/testItemUtilities.ts b/src/client/testing/testController/common/testItemUtilities.ts index 2f27fe8d64d7..86c535d389a8 100644 --- a/src/client/testing/testController/common/testItemUtilities.ts +++ b/src/client/testing/testController/common/testItemUtilities.ts @@ -11,7 +11,6 @@ import { TestRunResult, TestResultState, TestResultSnapshot, - TestTag, } from 'vscode'; import { CancellationToken } from 'vscode-jsonrpc'; import { traceError, traceVerbose } from '../../../common/logger'; @@ -26,8 +25,9 @@ import { TestDataKinds, } from './types'; -export const RunTestTag = new TestTag('python-run'); -export const DebugTestTag = new TestTag('python-debug'); +// Todo: Use `TestTag` when the proposed API gets into stable. +export const RunTestTag = { id: 'python-run' }; +export const DebugTestTag = { id: 'python-debug' }; export function removeItemByIdFromChildren( idToRawData: Map,