Skip to content

Commit e45fe13

Browse files
eleanorjboydeleanorjboyd
authored
fixes bug which stops duplicate pytest args (#23024)
fixes #22999 (comment)
1 parent 27783ce commit e45fe13

File tree

2 files changed

+60
-31
lines changed

2 files changed

+60
-31
lines changed

‎src/client/testing/testController/common/utils.ts

+32-10
Original file line numberDiff line numberDiff line change
@@ -359,14 +359,25 @@ export function splitTestNameWithRegex(testName: string): [string, string] {
359359
* @param args - Readonly array of strings to be converted to a map.
360360
* @returns A map representation of the input strings.
361361
*/
362-
export const argsToMap = (args: ReadonlyArray<string>): { [key: string]: string | null | undefined } => {
363-
const map: { [key: string]: string | null } = {};
362+
export const argsToMap = (args: ReadonlyArray<string>): { [key: string]: Array<string> | null | undefined } => {
363+
const map: { [key: string]: Array<string> | null } = {};
364364
for (const arg of args) {
365365
const delimiter = arg.indexOf('=');
366366
if (delimiter === -1) {
367+
// If no delimiter is found, the entire string becomes a key with a value of null.
367368
map[arg] = null;
368369
} else {
369-
map[arg.slice(0, delimiter)] = arg.slice(delimiter + 1);
370+
const key = arg.slice(0, delimiter);
371+
const value = arg.slice(delimiter + 1);
372+
if (map[key]) {
373+
// add to the array
374+
const arr = map[key] as string[];
375+
arr.push(value);
376+
map[key] = arr;
377+
} else {
378+
// create a new array
379+
map[key] = [value];
380+
}
370381
}
371382
}
372383

@@ -383,16 +394,22 @@ export const argsToMap = (args: ReadonlyArray<string>): { [key: string]: string
383394
* @param map - The map to be converted to an array of strings.
384395
* @returns An array of strings representation of the input map.
385396
*/
386-
export const mapToArgs = (map: { [key: string]: string | null | undefined }): string[] => {
397+
export const mapToArgs = (map: { [key: string]: Array<string> | null | undefined }): string[] => {
387398
const out: string[] = [];
388399
for (const key of Object.keys(map)) {
389400
const value = map[key];
390401
if (value === undefined) {
391402
// eslint-disable-next-line no-continue
392403
continue;
393404
}
394-
395-
out.push(value === null ? key : `${key}=${value}`);
405+
if (value === null) {
406+
out.push(key);
407+
} else {
408+
const values = Array.isArray(value) ? (value as string[]) : [value];
409+
for (const v of values) {
410+
out.push(`${key}=${v}`);
411+
}
412+
}
396413
}
397414

398415
return out;
@@ -407,13 +424,18 @@ export const mapToArgs = (map: { [key: string]: string | null | undefined }): st
407424
* @returns The updated map.
408425
*/
409426
export function addArgIfNotExist(
410-
map: { [key: string]: string | null | undefined },
427+
map: { [key: string]: Array<string> | null | undefined },
411428
argKey: string,
412429
argValue: string | null,
413-
): { [key: string]: string | null | undefined } {
430+
): { [key: string]: Array<string> | null | undefined } {
414431
// Only add the argument if it doesn't exist in the map.
415432
if (map[argKey] === undefined) {
416-
map[argKey] = argValue;
433+
// if null then set to null, otherwise set to an array with the value
434+
if (argValue === null) {
435+
map[argKey] = null;
436+
} else {
437+
map[argKey] = [argValue];
438+
}
417439
}
418440

419441
return map;
@@ -426,6 +448,6 @@ export function addArgIfNotExist(
426448
* @param argKey - The argument key to be checked.
427449
* @returns True if the argument key exists in the map, false otherwise.
428450
*/
429-
export function argKeyExists(map: { [key: string]: string | null | undefined }, argKey: string): boolean {
451+
export function argKeyExists(map: { [key: string]: Array<string> | null | undefined }, argKey: string): boolean {
430452
return map[argKey] !== undefined;
431453
}

‎src/test/testing/testController/utils.unit.test.ts

+28-21
Original file line numberDiff line numberDiff line change
@@ -165,10 +165,10 @@ ${data}${secondPayload}`;
165165
suite('Test Controller Utils: Args Mapping', () => {
166166
test('Converts map with mixed values to array of strings', async () => {
167167
const inputMap = {
168-
key1: 'value1',
168+
key1: ['value1'],
169169
key2: null,
170170
key3: undefined,
171-
key4: 'value4',
171+
key4: ['value4'],
172172
};
173173
const expectedOutput = ['key1=value1', 'key2', 'key4=value4'];
174174

@@ -209,24 +209,35 @@ ${data}${secondPayload}`;
209209

210210
assert.deepStrictEqual(result, expectedOutput);
211211
});
212+
test('Handles mapToArgs for a key with multiple values', async () => {
213+
const inputMap = {
214+
key1: null,
215+
key2: ['value1', 'value2'],
216+
};
217+
const expectedOutput = ['key1', 'key2=value1', 'key2=value2'];
218+
219+
const result = mapToArgs(inputMap);
220+
221+
assert.deepStrictEqual(result, expectedOutput);
222+
});
212223
test('Adds new argument if it does not exist', () => {
213224
const map = {};
214225
const argKey = 'newKey';
215226
const argValue = 'newValue';
216227

217228
const updatedMap = addArgIfNotExist(map, argKey, argValue);
218229

219-
assert.deepStrictEqual(updatedMap, { [argKey]: argValue });
230+
assert.deepStrictEqual(updatedMap, { [argKey]: [argValue] });
220231
});
221232

222233
test('Does not overwrite existing argument', () => {
223-
const map = { existingKey: 'existingValue' };
234+
const map = { existingKey: ['existingValue'] };
224235
const argKey = 'existingKey';
225236
const argValue = 'newValue';
226237

227238
const updatedMap = addArgIfNotExist(map, argKey, argValue);
228239

229-
assert.deepStrictEqual(updatedMap, { [argKey]: 'existingValue' });
240+
assert.deepStrictEqual(updatedMap, { [argKey]: ['existingValue'] });
230241
});
231242

232243
test('Handles null value for new key', () => {
@@ -249,21 +260,9 @@ ${data}${secondPayload}`;
249260
assert.deepStrictEqual(updatedMap, { [argKey]: null });
250261
});
251262

252-
test('Accepts addition if key exists with undefined value', () => {
253-
const map = { undefinedKey: undefined };
254-
const argKey = 'undefinedKey';
255-
const argValue = 'newValue';
256-
257-
// Attempting to add a key that is explicitly set to undefined
258-
const updatedMap = addArgIfNotExist(map, argKey, argValue);
259-
260-
// Expect the map to remain unchanged because the key exists as undefined
261-
assert.strictEqual(map[argKey], argValue);
262-
assert.deepStrictEqual(updatedMap, { [argKey]: argValue });
263-
});
264263
test('Complex test for argKeyExists with various key types', () => {
265264
const map = {
266-
stringKey: 'stringValue',
265+
stringKey: ['stringValue'],
267266
nullKey: null,
268267
// Note: not adding an 'undefinedKey' explicitly since it's not present and hence undefined by default
269268
};
@@ -289,7 +288,15 @@ ${data}${secondPayload}`;
289288
});
290289
test('Converts array of strings with "=" into a map', () => {
291290
const args = ['key1=value1', 'key2=value2'];
292-
const expectedMap = { key1: 'value1', key2: 'value2' };
291+
const expectedMap = { key1: ['value1'], key2: ['value2'] };
292+
293+
const resultMap = argsToMap(args);
294+
295+
assert.deepStrictEqual(resultMap, expectedMap);
296+
});
297+
test('Handles argsToMap for multiple values for the same key', () => {
298+
const args = ['key1=value1', 'key1=value2'];
299+
const expectedMap = { key1: ['value1', 'value2'] };
293300

294301
const resultMap = argsToMap(args);
295302

@@ -307,7 +314,7 @@ ${data}${secondPayload}`;
307314

308315
test('Handles mixed keys with and without "="', () => {
309316
const args = ['key1=value1', 'key2'];
310-
const expectedMap = { key1: 'value1', key2: null };
317+
const expectedMap = { key1: ['value1'], key2: null };
311318

312319
const resultMap = argsToMap(args);
313320

@@ -316,7 +323,7 @@ ${data}${secondPayload}`;
316323

317324
test('Handles strings with multiple "=" characters', () => {
318325
const args = ['key1=part1=part2'];
319-
const expectedMap = { key1: 'part1=part2' };
326+
const expectedMap = { key1: ['part1=part2'] };
320327

321328
const resultMap = argsToMap(args);
322329

0 commit comments

Comments
 (0)