Skip to content

fixes bug which stops duplicate pytest args #23024

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 4, 2024
Merged

fixes bug which stops duplicate pytest args #23024

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b15fb23
fixes bug which stops duplicate pytest args
eleanorjboyd Mar 4, 2024
63f184e
fix a comment
eleanorjboyd Mar 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 32 additions & 10 deletions src/client/testing/testController/common/utils.ts
View file
Open in desktop
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
Open in desktop
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