From 610507cacb74effbb73e77e70d5c5b614d021ee9 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Wed, 27 Sep 2023 17:50:44 -0700 Subject: [PATCH 1/2] fix regex split for subtest names --- .../testController/common/resultResolver.ts | 19 +++---- .../testing/testController/common/utils.ts | 25 +++++++++ .../testing/testController/utils.unit.test.ts | 56 +++++++++++++++++++ 3 files changed, 90 insertions(+), 10 deletions(-) diff --git a/src/client/testing/testController/common/resultResolver.ts b/src/client/testing/testController/common/resultResolver.ts index 78883bdcf8fb..79cee6452a8c 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 1272ff37fb5d..4a0c7078667a 100644 --- a/src/client/testing/testController/common/utils.ts +++ b/src/client/testing/testController/common/utils.ts @@ -320,3 +320,28 @@ 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] { + // The regex pattern has three main components: + // 1. ^(.*?): Matches the beginning of the string and captures everything until the last opening bracket or parenthesis. This captures the parent test name. + // 2. (?:...|...): A non-capturing group containing two patterns separated by an OR (|). + // - \(([^)]+)\): Matches an opening parenthesis, captures everything inside it until the closing parenthesis. This captures the subtest inside parenthesis. + // - \[([^]]+)\]: Matches an opening square bracket, captures everything inside it until the closing square bracket. This captures the subtest inside square brackets. + // 3. ?$: The question mark indicates the preceding non-capturing group is optional. The dollar sign matches the end of the string. + const regex = /^(.*?) ([\[(].*[\])])$/; + const match = testName.match(regex); + // const m2 = regex.exec(testName); + // console.log('m2', m2); + // 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. + 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 9168abc7041f..12100252d1a9 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]); + }); + }); +}); From 04682008428f3dfebf1213b5513b601a0cd990fb Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Thu, 28 Sep 2023 08:36:29 -0700 Subject: [PATCH 2/2] fixing commenting --- src/client/testing/testController/common/utils.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/client/testing/testController/common/utils.ts b/src/client/testing/testController/common/utils.ts index 4a0c7078667a..c58850050590 100644 --- a/src/client/testing/testController/common/utils.ts +++ b/src/client/testing/testController/common/utils.ts @@ -328,18 +328,10 @@ export function createEOTPayload(executionBool: boolean): EOTTestPayload { * @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] { - // The regex pattern has three main components: - // 1. ^(.*?): Matches the beginning of the string and captures everything until the last opening bracket or parenthesis. This captures the parent test name. - // 2. (?:...|...): A non-capturing group containing two patterns separated by an OR (|). - // - \(([^)]+)\): Matches an opening parenthesis, captures everything inside it until the closing parenthesis. This captures the subtest inside parenthesis. - // - \[([^]]+)\]: Matches an opening square bracket, captures everything inside it until the closing square bracket. This captures the subtest inside square brackets. - // 3. ?$: The question mark indicates the preceding non-capturing group is optional. The dollar sign matches the end of the string. - const regex = /^(.*?) ([\[(].*[\])])$/; - const match = testName.match(regex); - // const m2 = regex.exec(testName); - // console.log('m2', m2); // 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]; }