From 84826d1404483fb0b028ab44c029d060e2719d0e Mon Sep 17 00:00:00 2001 From: Eleanor Boyd Date: Thu, 28 Sep 2023 09:47:41 -0700 Subject: [PATCH] fix regex split for subtest names (#22107) fixes https://github.com/microsoft/vscode-python/issues/21733. Handles both cases of subtest naming as described here by ChatGPT: When you use self.subTest(i=i), you're explicitly naming the argument i. This causes subTest to use the key=value format for the sub-test's description. Therefore, the sub-test name becomes: `test_subtests.NumbersTest2.test_even2 (i='h i')` However, when you use self.subTest(i), you're passing a positional argument. In this case, subTest doesn't have a key for the argument, so it simply uses the value in square brackets: `test_subtests.NumbersTest2.test_even2 [h i]` --- .../testController/common/resultResolver.ts | 19 +++---- .../testing/testController/common/utils.ts | 17 ++++++ .../testing/testController/utils.unit.test.ts | 56 +++++++++++++++++++ 3 files changed, 82 insertions(+), 10 deletions(-) diff --git a/src/client/testing/testController/common/resultResolver.ts b/src/client/testing/testController/common/resultResolver.ts index 78883bdcf8fbd..79cee6452a8c4 100644 --- a/src/client/testing/testController/common/resultResolver.ts +++ b/src/client/testing/testController/common/resultResolver.ts @@ -11,7 +11,7 @@ import { clearAllChildren, createErrorTestItem, getTestCaseNodes } from './testI import { sendTelemetryEvent } from '../../../telemetry'; import { EventName } from '../../../telemetry/constants'; import { splitLines } from '../../../common/stringUtils'; -import { buildErrorNodeOptions, fixLogLines, populateTestTree } from './utils'; +import { buildErrorNodeOptions, fixLogLines, populateTestTree, splitTestNameWithRegex } from './utils'; import { Deferred } from '../../../common/utils/async'; export class PythonResultResolver implements ITestResultResolver { @@ -216,9 +216,8 @@ export class PythonResultResolver implements ITestResultResolver { }); } } else if (rawTestExecData.result[keyTemp].outcome === 'subtest-failure') { - // split on " " since the subtest ID has the parent test ID in the first part of the ID. - const parentTestCaseId = keyTemp.split(' ')[0]; - const subtestId = keyTemp.split(' ')[1]; + // split on [] or () based on how the subtest is setup. + const [parentTestCaseId, subtestId] = splitTestNameWithRegex(keyTemp); const parentTestItem = this.runIdToTestItem.get(parentTestCaseId); const data = rawTestExecData.result[keyTemp]; // find the subtest's parent test item @@ -227,7 +226,10 @@ export class PythonResultResolver implements ITestResultResolver { if (subtestStats) { subtestStats.failed += 1; } else { - this.subTestStats.set(parentTestCaseId, { failed: 1, passed: 0 }); + this.subTestStats.set(parentTestCaseId, { + failed: 1, + passed: 0, + }); runInstance.appendOutput(fixLogLines(`${parentTestCaseId} [subtests]:\r\n`)); // clear since subtest items don't persist between runs clearAllChildren(parentTestItem); @@ -253,11 +255,8 @@ export class PythonResultResolver implements ITestResultResolver { throw new Error('Parent test item not found'); } } else if (rawTestExecData.result[keyTemp].outcome === 'subtest-success') { - // split only on first " [" since the subtest ID has the parent test ID in the first part of the ID. - const index = keyTemp.indexOf(' ['); - const parentTestCaseId = keyTemp.substring(0, index); - // add one to index to remove the space from the start of the subtest ID - const subtestId = keyTemp.substring(index + 1, keyTemp.length); + // split on [] or () based on how the subtest is setup. + const [parentTestCaseId, subtestId] = splitTestNameWithRegex(keyTemp); const parentTestItem = this.runIdToTestItem.get(parentTestCaseId); // find the subtest's parent test item diff --git a/src/client/testing/testController/common/utils.ts b/src/client/testing/testController/common/utils.ts index 1272ff37fb5db..c58850050590e 100644 --- a/src/client/testing/testController/common/utils.ts +++ b/src/client/testing/testController/common/utils.ts @@ -320,3 +320,20 @@ export function createEOTPayload(executionBool: boolean): EOTTestPayload { eot: true, } as EOTTestPayload; } + +/** + * Splits a test name into its parent test name and subtest unique section. + * + * @param testName The full test name string. + * @returns A tuple where the first item is the parent test name and the second item is the subtest section or `testName` if no subtest section exists. + */ +export function splitTestNameWithRegex(testName: string): [string, string] { + // If a match is found, return the parent test name and the subtest (whichever was captured between parenthesis or square brackets). + // Otherwise, return the entire testName for the parent and entire testName for the subtest. + const regex = /^(.*?) ([\[(].*[\])])$/; + const match = testName.match(regex); + if (match) { + return [match[1].trim(), match[2] || match[3] || testName]; + } + return [testName, testName]; +} diff --git a/src/test/testing/testController/utils.unit.test.ts b/src/test/testing/testController/utils.unit.test.ts index 9168abc7041fa..12100252d1a94 100644 --- a/src/test/testing/testController/utils.unit.test.ts +++ b/src/test/testing/testController/utils.unit.test.ts @@ -8,6 +8,7 @@ import { JSONRPC_UUID_HEADER, ExtractJsonRPCData, parseJsonRPCHeadersAndData, + splitTestNameWithRegex, } from '../../../client/testing/testController/common/utils'; suite('Test Controller Utils: JSON RPC', () => { @@ -65,3 +66,58 @@ suite('Test Controller Utils: JSON RPC', () => { assert.deepStrictEqual(rpcContent.remainingRawData, rawDataString); }); }); + +suite('Test Controller Utils: Other', () => { + interface TestCase { + name: string; + input: string; + expectedParent: string; + expectedSubtest: string; + } + + const testCases: Array = [ + { + name: 'Single parameter, named', + input: 'test_package.ClassName.test_method (param=value)', + expectedParent: 'test_package.ClassName.test_method', + expectedSubtest: '(param=value)', + }, + { + name: 'Single parameter, unnamed', + input: 'test_package.ClassName.test_method [value]', + expectedParent: 'test_package.ClassName.test_method', + expectedSubtest: '[value]', + }, + { + name: 'Multiple parameters, named', + input: 'test_package.ClassName.test_method (param1=value1, param2=value2)', + expectedParent: 'test_package.ClassName.test_method', + expectedSubtest: '(param1=value1, param2=value2)', + }, + { + name: 'Multiple parameters, unnamed', + input: 'test_package.ClassName.test_method [value1, value2]', + expectedParent: 'test_package.ClassName.test_method', + expectedSubtest: '[value1, value2]', + }, + { + name: 'Names with special characters', + input: 'test_package.ClassName.test_method (param1=value/1, param2=value+2)', + expectedParent: 'test_package.ClassName.test_method', + expectedSubtest: '(param1=value/1, param2=value+2)', + }, + { + name: 'Names with spaces', + input: 'test_package.ClassName.test_method ["a b c d"]', + expectedParent: 'test_package.ClassName.test_method', + expectedSubtest: '["a b c d"]', + }, + ]; + + testCases.forEach((testCase) => { + test(`splitTestNameWithRegex: ${testCase.name}`, () => { + const splitResult = splitTestNameWithRegex(testCase.input); + assert.deepStrictEqual(splitResult, [testCase.expectedParent, testCase.expectedSubtest]); + }); + }); +});