From b15fb23a01cab8bf3f2a72947e64a6e94c46f107 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 4 Mar 2024 10:00:28 -0800 Subject: [PATCH 1/2] fixes bug which stops duplicate pytest args --- .../testing/testController/common/utils.ts | 44 +++++++++++++---- .../testing/testController/utils.unit.test.ts | 49 +++++++++++-------- 2 files changed, 62 insertions(+), 31 deletions(-) diff --git a/src/client/testing/testController/common/utils.ts b/src/client/testing/testController/common/utils.ts index 0e81154a899c..d2a14a40cb0d 100644 --- a/src/client/testing/testController/common/utils.ts +++ b/src/client/testing/testController/common/utils.ts @@ -359,14 +359,26 @@ export function splitTestNameWithRegex(testName: string): [string, string] { * @param args - Readonly array of strings to be converted to a map. * @returns A map representation of the input strings. */ -export const argsToMap = (args: ReadonlyArray): { [key: string]: string | null | undefined } => { - const map: { [key: string]: string | null } = {}; +export const argsToMap = (args: ReadonlyArray): { [key: string]: Array | null | undefined } => { + const map: { [key: string]: Array | null } = {}; for (const arg of args) { const delimiter = arg.indexOf('='); if (delimiter === -1) { + // If no delimiter is found, the entire string becomes a key with a value of null. map[arg] = null; } else { - map[arg.slice(0, delimiter)] = arg.slice(delimiter + 1); + const key = arg.slice(0, delimiter); + const value = arg.slice(delimiter + 1); + if (map[key]) { + // add to the array + const arr = map[key] as string[]; + arr.push(value); + // do I need to resave? + map[key] = arr; + } else { + // create a new array + map[key] = [value]; + } } } @@ -383,7 +395,7 @@ export const argsToMap = (args: ReadonlyArray): { [key: string]: string * @param map - The map to be converted to an array of strings. * @returns An array of strings representation of the input map. */ -export const mapToArgs = (map: { [key: string]: string | null | undefined }): string[] => { +export const mapToArgs = (map: { [key: string]: Array | null | undefined }): string[] => { const out: string[] = []; for (const key of Object.keys(map)) { const value = map[key]; @@ -391,8 +403,15 @@ export const mapToArgs = (map: { [key: string]: string | null | undefined }): st // eslint-disable-next-line no-continue continue; } - - out.push(value === null ? key : `${key}=${value}`); + if (value === null) { + out.push(key); + } else { + const values = Array.isArray(value) ? (value as string[]) : [value]; + // what happens if this is just "value" and not "values"? + for (const v of values) { + out.push(`${key}=${v}`); + } + } } return out; @@ -407,13 +426,18 @@ export const mapToArgs = (map: { [key: string]: string | null | undefined }): st * @returns The updated map. */ export function addArgIfNotExist( - map: { [key: string]: string | null | undefined }, + map: { [key: string]: Array | null | undefined }, argKey: string, argValue: string | null, -): { [key: string]: string | null | undefined } { +): { [key: string]: Array | null | undefined } { // Only add the argument if it doesn't exist in the map. if (map[argKey] === undefined) { - map[argKey] = argValue; + // if null then set to null, otherwise set to an array with the value + if (argValue === null) { + map[argKey] = null; + } else { + map[argKey] = [argValue]; + } } return map; @@ -426,6 +450,6 @@ export function addArgIfNotExist( * @param argKey - The argument key to be checked. * @returns True if the argument key exists in the map, false otherwise. */ -export function argKeyExists(map: { [key: string]: string | null | undefined }, argKey: string): boolean { +export function argKeyExists(map: { [key: string]: Array | null | undefined }, argKey: string): boolean { return map[argKey] !== undefined; } diff --git a/src/test/testing/testController/utils.unit.test.ts b/src/test/testing/testController/utils.unit.test.ts index 5bcf8dfa10c8..36ee42ab022e 100644 --- a/src/test/testing/testController/utils.unit.test.ts +++ b/src/test/testing/testController/utils.unit.test.ts @@ -165,10 +165,10 @@ ${data}${secondPayload}`; suite('Test Controller Utils: Args Mapping', () => { test('Converts map with mixed values to array of strings', async () => { const inputMap = { - key1: 'value1', + key1: ['value1'], key2: null, key3: undefined, - key4: 'value4', + key4: ['value4'], }; const expectedOutput = ['key1=value1', 'key2', 'key4=value4']; @@ -209,6 +209,17 @@ ${data}${secondPayload}`; assert.deepStrictEqual(result, expectedOutput); }); + test('Handles mapToArgs for a key with multiple values', async () => { + const inputMap = { + key1: null, + key2: ['value1', 'value2'], + }; + const expectedOutput = ['key1', 'key2=value1', 'key2=value2']; + + const result = mapToArgs(inputMap); + + assert.deepStrictEqual(result, expectedOutput); + }); test('Adds new argument if it does not exist', () => { const map = {}; const argKey = 'newKey'; @@ -216,17 +227,17 @@ ${data}${secondPayload}`; const updatedMap = addArgIfNotExist(map, argKey, argValue); - assert.deepStrictEqual(updatedMap, { [argKey]: argValue }); + assert.deepStrictEqual(updatedMap, { [argKey]: [argValue] }); }); test('Does not overwrite existing argument', () => { - const map = { existingKey: 'existingValue' }; + const map = { existingKey: ['existingValue'] }; const argKey = 'existingKey'; const argValue = 'newValue'; const updatedMap = addArgIfNotExist(map, argKey, argValue); - assert.deepStrictEqual(updatedMap, { [argKey]: 'existingValue' }); + assert.deepStrictEqual(updatedMap, { [argKey]: ['existingValue'] }); }); test('Handles null value for new key', () => { @@ -249,21 +260,9 @@ ${data}${secondPayload}`; assert.deepStrictEqual(updatedMap, { [argKey]: null }); }); - test('Accepts addition if key exists with undefined value', () => { - const map = { undefinedKey: undefined }; - const argKey = 'undefinedKey'; - const argValue = 'newValue'; - - // Attempting to add a key that is explicitly set to undefined - const updatedMap = addArgIfNotExist(map, argKey, argValue); - - // Expect the map to remain unchanged because the key exists as undefined - assert.strictEqual(map[argKey], argValue); - assert.deepStrictEqual(updatedMap, { [argKey]: argValue }); - }); test('Complex test for argKeyExists with various key types', () => { const map = { - stringKey: 'stringValue', + stringKey: ['stringValue'], nullKey: null, // Note: not adding an 'undefinedKey' explicitly since it's not present and hence undefined by default }; @@ -289,7 +288,15 @@ ${data}${secondPayload}`; }); test('Converts array of strings with "=" into a map', () => { const args = ['key1=value1', 'key2=value2']; - const expectedMap = { key1: 'value1', key2: 'value2' }; + const expectedMap = { key1: ['value1'], key2: ['value2'] }; + + const resultMap = argsToMap(args); + + assert.deepStrictEqual(resultMap, expectedMap); + }); + test('Handles argsToMap for multiple values for the same key', () => { + const args = ['key1=value1', 'key1=value2']; + const expectedMap = { key1: ['value1', 'value2'] }; const resultMap = argsToMap(args); @@ -307,7 +314,7 @@ ${data}${secondPayload}`; test('Handles mixed keys with and without "="', () => { const args = ['key1=value1', 'key2']; - const expectedMap = { key1: 'value1', key2: null }; + const expectedMap = { key1: ['value1'], key2: null }; const resultMap = argsToMap(args); @@ -316,7 +323,7 @@ ${data}${secondPayload}`; test('Handles strings with multiple "=" characters', () => { const args = ['key1=part1=part2']; - const expectedMap = { key1: 'part1=part2' }; + const expectedMap = { key1: ['part1=part2'] }; const resultMap = argsToMap(args); From 63f184e7d3845a000118a2fd8543f4689508ee97 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 4 Mar 2024 10:02:48 -0800 Subject: [PATCH 2/2] fix a comment --- src/client/testing/testController/common/utils.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/client/testing/testController/common/utils.ts b/src/client/testing/testController/common/utils.ts index d2a14a40cb0d..e98fa99b9bd2 100644 --- a/src/client/testing/testController/common/utils.ts +++ b/src/client/testing/testController/common/utils.ts @@ -373,7 +373,6 @@ export const argsToMap = (args: ReadonlyArray): { [key: string]: Array | null | undefined out.push(key); } else { const values = Array.isArray(value) ? (value as string[]) : [value]; - // what happens if this is just "value" and not "values"? for (const v of values) { out.push(`${key}=${v}`); }