Skip to content

Commit

Permalink
fixes bug which stops duplicate pytest args (#23024)
Browse files Browse the repository at this point in the history 
fixes
#22999 (comment)
  • Loading branch information
@eleanorjboyd
eleanorjboyd authored Mar 4, 2024
1 parent 27783ce commit e45fe13
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 31 deletions.
42 changes: 32 additions & 10 deletions src/client/testing/testController/common/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,14 +359,25 @@ 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<string>): { [key: string]: string | null | undefined } => {
const map: { [key: string]: string | null } = {};
export const argsToMap = (args: ReadonlyArray<string>): { [key: string]: Array<string> | null | undefined } => {
const map: { [key: string]: Array<string> | 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);
map[key] = arr;
} else {
// create a new array
map[key] = [value];
}
}
}

Expand All @@ -383,16 +394,22 @@ export const argsToMap = (args: ReadonlyArray<string>): { [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<string> | null | undefined }): string[] => {
const out: string[] = [];
for (const key of Object.keys(map)) {
const value = map[key];
if (value === undefined) {
// 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];
for (const v of values) {
out.push(`${key}=${v}`);
}
}
}

return out;
Expand All @@ -407,13 +424,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<string> | null | undefined },
argKey: string,
argValue: string | null,
): { [key: string]: string | null | undefined } {
): { [key: string]: Array<string> | 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;
Expand All @@ -426,6 +448,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<string> | null | undefined }, argKey: string): boolean {
return map[argKey] !== undefined;
}
49 changes: 28 additions & 21 deletions src/test/testing/testController/utils.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'];

Expand Down Expand Up @@ -209,24 +209,35 @@ ${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';
const argValue = 'newValue';

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', () => {
Expand All @@ -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
};
Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand Down

0 comments on commit e45fe13

Please sign in to comment.