diff --git a/extensions/positron-python/ThirdPartyNotices-Repository.txt b/extensions/positron-python/ThirdPartyNotices-Repository.txt index bbb00d523f9..9e7e822af1b 100644 --- a/extensions/positron-python/ThirdPartyNotices-Repository.txt +++ b/extensions/positron-python/ThirdPartyNotices-Repository.txt @@ -17,7 +17,6 @@ Microsoft Python extension for Visual Studio Code incorporates third party mater 11. vscode-cpptools (https://github.com/microsoft/vscode-cpptools) 12. mocha (https://github.com/mochajs/mocha) 13. get-pip (https://github.com/pypa/get-pip) -14. vscode-js-debug (https://github.com/microsoft/vscode-js-debug) %% Go for Visual Studio Code NOTICES, INFORMATION, AND LICENSE BEGIN HERE @@ -1033,31 +1032,3 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. ========================================= END OF get-pip NOTICES, INFORMATION, AND LICENSE - - -%% vscode-js-debug NOTICES, INFORMATION, AND LICENSE BEGIN HERE -========================================= - -MIT License - -Copyright (c) Microsoft Corporation. All rights reserved. - -Permission is hereby granted, free of charge, to any person obtaining a copy -of this software and associated documentation files (the "Software"), to deal -in the Software without restriction, including without limitation the rights -to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -copies of the Software, and to permit persons to whom the Software is -furnished to do so, subject to the following conditions: - -The above copyright notice and this permission notice shall be included in all -copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -SOFTWARE -========================================= -END OF vscode-js-debug NOTICES, INFORMATION, AND LICENSE diff --git a/extensions/positron-python/src/client/testing/testController/common/utils.ts b/extensions/positron-python/src/client/testing/testController/common/utils.ts index e98fa99b9bd..be1cf8b2ca3 100644 --- a/extensions/positron-python/src/client/testing/testController/common/utils.ts +++ b/extensions/positron-python/src/client/testing/testController/common/utils.ts @@ -3,7 +3,7 @@ import * as net from 'net'; import * as path from 'path'; import { CancellationToken, Position, TestController, TestItem, Uri, Range } from 'vscode'; -import { traceError, traceLog, traceVerbose } from '../../../logging'; +import { traceError, traceInfo, traceLog, traceVerbose } from '../../../logging'; import { EnableTestAdapterRewrite } from '../../../common/experiments/groups'; import { IExperimentService } from '../../../common/types'; @@ -351,103 +351,39 @@ export function splitTestNameWithRegex(testName: string): [string, string] { } /** - * Converts an array of strings (with or without '=') into a map. - * If a string contains '=', it is split into a key-value pair, with the portion - * before the '=' as the key and the portion after the '=' as the value. - * If no '=' is found in the string, the entire string becomes a key with a value of null. - * - * @param args - Readonly array of strings to be converted to a map. - * @returns A map representation of the input strings. + * Takes a list of arguments and adds an key-value pair to the list if the key doesn't already exist. Searches each element + * in the array for the key to see if it is contained within the element. + * @param args list of arguments to search + * @param argToAdd argument to add if it doesn't already exist + * @returns the list of arguments with the key-value pair added if it didn't already exist */ -export const argsToMap = (args: ReadonlyArray): { [key: string]: Array | null | undefined } => { - const map: { [key: string]: Array | null } = {}; +export function addValueIfKeyNotExist(args: string[], key: string, value: string | null): string[] { 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 { - 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]; - } + if (arg.includes(key)) { + traceInfo(`arg: ${key} already exists in args, not adding.`); + return args; } } - - return map; -}; - -/** - * Converts a map into an array of strings. - * Each key-value pair in the map is transformed into a string. - * If the value is null, only the key is represented in the string. - * If the value is defined (and not null), the string is in the format "key=value". - * If a value is undefined, the key-value pair is skipped. - * - * @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]: Array | 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; - } - 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}`); - } - } + if (value) { + args.push(`${key}=${value}`); + } else { + args.push(`${key}`); } - - return out; -}; + return args; +} /** - * Adds an argument to the map only if it doesn't already exist. - * - * @param map - The map of arguments. - * @param argKey - The argument key to be checked and added. - * @param argValue - The value to set for the argument if it's not already in the map. - * @returns The updated map. + * Checks if a key exists in a list of arguments. Searches each element in the array + * for the key to see if it is contained within the element. + * @param args list of arguments to search + * @param key string to search for + * @returns true if the key exists in the list of arguments, false otherwise */ -export function addArgIfNotExist( - map: { [key: string]: Array | null | undefined }, - argKey: string, - argValue: string | null, -): { [key: string]: Array | null | undefined } { - // Only add the argument if it doesn't exist in the map. - if (map[argKey] === undefined) { - // if null then set to null, otherwise set to an array with the value - if (argValue === null) { - map[argKey] = null; - } else { - map[argKey] = [argValue]; +export function argKeyExists(args: string[], key: string): boolean { + for (const arg of args) { + if (arg.includes(key)) { + return true; } } - - return map; -} - -/** - * Checks if an argument key exists in the map. - * - * @param map - The map of arguments. - * @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]: Array | null | undefined }, argKey: string): boolean { - return map[argKey] !== undefined; + return false; } diff --git a/extensions/positron-python/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts b/extensions/positron-python/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts index 2d0dab76508..f29a428f3d3 100644 --- a/extensions/positron-python/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts +++ b/extensions/positron-python/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts @@ -25,9 +25,7 @@ import { createEOTPayload, createTestingDeferred, fixLogLinesNoTrailing, - argsToMap, - addArgIfNotExist, - mapToArgs, + addValueIfKeyNotExist, } from '../common/utils'; import { IEnvironmentVariablesProvider } from '../../../common/variables/types'; @@ -70,16 +68,14 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { const relativePathToPytest = 'python_files'; const fullPluginPath = path.join(EXTENSION_ROOT_DIR, relativePathToPytest); const settings = this.configSettings.getSettings(uri); - let pytestArgsMap = argsToMap(settings.testing.pytestArgs); + let { pytestArgs } = settings.testing; const cwd = settings.testing.cwd && settings.testing.cwd.length > 0 ? settings.testing.cwd : uri.fsPath; // check for symbolic path const stats = fs.lstatSync(cwd); if (stats.isSymbolicLink()) { - traceWarn( - "The cwd is a symbolic link, adding '--rootdir' to pytestArgsMap only if it doesn't already exist.", - ); - pytestArgsMap = addArgIfNotExist(pytestArgsMap, '--rootdir', cwd); + traceWarn("The cwd is a symbolic link, adding '--rootdir' to pytestArgs only if it doesn't already exist."); + pytestArgs = addValueIfKeyNotExist(pytestArgs, '--rootdir', cwd); } // get and edit env vars @@ -111,7 +107,7 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { }; const execService = await executionFactory?.createActivatedEnvironment(creationOptions); // delete UUID following entire discovery finishing. - const execArgs = ['-m', 'pytest', '-p', 'vscode_pytest', '--collect-only'].concat(mapToArgs(pytestArgsMap)); + const execArgs = ['-m', 'pytest', '-p', 'vscode_pytest', '--collect-only'].concat(pytestArgs); traceVerbose(`Running pytest discovery with command: ${execArgs.join(' ')} for workspace ${uri.fsPath}.`); const deferredTillExecClose: Deferred = createTestingDeferred(); diff --git a/extensions/positron-python/src/client/testing/testController/pytest/pytestExecutionAdapter.ts b/extensions/positron-python/src/client/testing/testController/pytest/pytestExecutionAdapter.ts index de519548d68..2f41d486ba3 100644 --- a/extensions/positron-python/src/client/testing/testController/pytest/pytestExecutionAdapter.ts +++ b/extensions/positron-python/src/client/testing/testController/pytest/pytestExecutionAdapter.ts @@ -128,17 +128,16 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { const execService = await executionFactory?.createActivatedEnvironment(creationOptions); try { // Remove positional test folders and files, we will add as needed per node - const testArgs = removePositionalFoldersAndFiles(pytestArgs); - let testArgsMap = utils.argsToMap(testArgs); + let testArgs = removePositionalFoldersAndFiles(pytestArgs); // if user has provided `--rootdir` then use that, otherwise add `cwd` // root dir is required so pytest can find the relative paths and for symlinks - utils.addArgIfNotExist(testArgsMap, '--rootdir', cwd); + utils.addValueIfKeyNotExist(testArgs, '--rootdir', cwd); // -s and --capture are both command line options that control how pytest captures output. // if neither are set, then set --capture=no to prevent pytest from capturing output. - if (debugBool && !utils.argKeyExists(testArgsMap, '-s')) { - testArgsMap = utils.addArgIfNotExist(testArgsMap, '--capture', 'no'); + if (debugBool && !utils.argKeyExists(testArgs, '-s')) { + testArgs = utils.addValueIfKeyNotExist(testArgs, '--capture', 'no'); } // add port with run test ids to env vars @@ -163,18 +162,14 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { const pytestUUID = uuid.toString(); const launchOptions: LaunchOptions = { cwd, - args: utils.mapToArgs(testArgsMap), + args: testArgs, token: spawnOptions.token, testProvider: PYTEST_PROVIDER, pytestPort, pytestUUID, runTestIdsPort: pytestRunTestIdsPort.toString(), }; - traceInfo( - `Running DEBUG pytest with arguments: ${utils.mapToArgs(testArgsMap).join(' ')} for workspace ${ - uri.fsPath - } \r\n`, - ); + traceInfo(`Running DEBUG pytest with arguments: ${testArgs} for workspace ${uri.fsPath} \r\n`); await debugLauncher!.launchDebugger(launchOptions, () => { deferredTillEOT?.resolve(); }); @@ -183,7 +178,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { const deferredTillExecClose: Deferred = utils.createTestingDeferred(); // combine path to run script with run args const scriptPath = path.join(fullPluginPath, 'vscode_pytest', 'run_pytest_script.py'); - const runArgs = [scriptPath, ...utils.mapToArgs(testArgsMap)]; + const runArgs = [scriptPath, ...testArgs]; traceInfo(`Running pytest with arguments: ${runArgs.join(' ')} for workspace ${uri.fsPath} \r\n`); let resultProc: ChildProcess | undefined; diff --git a/extensions/positron-python/src/test/testing/common/testingAdapter.test.ts b/extensions/positron-python/src/test/testing/common/testingAdapter.test.ts index 8e2f6200003..c52c4c8ba6a 100644 --- a/extensions/positron-python/src/test/testing/common/testingAdapter.test.ts +++ b/extensions/positron-python/src/test/testing/common/testingAdapter.test.ts @@ -72,13 +72,17 @@ suite('End to End Tests: test adapters', () => { // create symlink for specific symlink test const target = rootPathSmallWorkspace; const dest = rootPathDiscoverySymlink; - fs.symlink(target, dest, 'dir', (err) => { - if (err) { - console.error(err); - } else { - console.log('Symlink created successfully for end to end tests.'); - } - }); + try { + fs.symlink(target, dest, 'dir', (err) => { + if (err) { + console.error(err); + } else { + console.log('Symlink created successfully for end to end tests.'); + } + }); + } catch (err) { + console.error(err); + } }); setup(async () => { @@ -117,13 +121,17 @@ suite('End to End Tests: test adapters', () => { suiteTeardown(async () => { // remove symlink const dest = rootPathDiscoverySymlink; - fs.unlink(dest, (err) => { - if (err) { - console.error(err); - } else { - console.log('Symlink removed successfully after tests.'); - } - }); + if (fs.existsSync(dest)) { + fs.unlink(dest, (err) => { + if (err) { + console.error(err); + } else { + console.log('Symlink removed successfully after tests.'); + } + }); + } else { + console.log('Symlink was not found to remove after tests, exiting successfully'); + } }); test('unittest discovery adapter small workspace', async () => { // result resolver and saved data for assertions @@ -293,6 +301,7 @@ suite('End to End Tests: test adapters', () => { resultResolver, envVarsService, ); + configService.getSettings(workspaceUri).testing.pytestArgs = []; await discoveryAdapter.discoverTests(workspaceUri, pythonExecFactory).finally(() => { // verification after discovery is complete @@ -372,6 +381,7 @@ suite('End to End Tests: test adapters', () => { // set workspace to test workspace folder workspaceUri = Uri.parse(rootPathLargeWorkspace); + configService.getSettings(workspaceUri).testing.pytestArgs = []; await discoveryAdapter.discoverTests(workspaceUri, pythonExecFactory).finally(() => { // verification after discovery is complete @@ -558,6 +568,7 @@ suite('End to End Tests: test adapters', () => { }; // set workspace to test workspace folder workspaceUri = Uri.parse(rootPathSmallWorkspace); + configService.getSettings(workspaceUri).testing.pytestArgs = []; // run pytest execution const executionAdapter = new PytestTestExecutionAdapter( @@ -648,6 +659,7 @@ suite('End to End Tests: test adapters', () => { // set workspace to test workspace folder workspaceUri = Uri.parse(rootPathLargeWorkspace); + configService.getSettings(workspaceUri).testing.pytestArgs = []; // generate list of test_ids const testIds: string[] = []; @@ -728,6 +740,7 @@ suite('End to End Tests: test adapters', () => { // set workspace to test workspace folder workspaceUri = Uri.parse(rootPathDiscoveryErrorWorkspace); + configService.getSettings(workspaceUri).testing.unittestArgs = ['-s', '.', '-p', '*test*.py']; const discoveryAdapter = new UnittestTestDiscoveryAdapter( pythonTestServer, @@ -799,6 +812,8 @@ suite('End to End Tests: test adapters', () => { // set workspace to test workspace folder workspaceUri = Uri.parse(rootPathDiscoveryErrorWorkspace); + configService.getSettings(workspaceUri).testing.pytestArgs = []; + await discoveryAdapter.discoverTests(workspaceUri, pythonExecFactory).finally(() => { // verification after discovery is complete assert.ok( @@ -860,6 +875,7 @@ suite('End to End Tests: test adapters', () => { // set workspace to test workspace folder workspaceUri = Uri.parse(rootPathErrorWorkspace); + configService.getSettings(workspaceUri).testing.unittestArgs = ['-s', '.', '-p', '*test*.py']; // run pytest execution const executionAdapter = new UnittestTestExecutionAdapter( @@ -921,6 +937,7 @@ suite('End to End Tests: test adapters', () => { // set workspace to test workspace folder workspaceUri = Uri.parse(rootPathErrorWorkspace); + configService.getSettings(workspaceUri).testing.pytestArgs = []; // run pytest execution const executionAdapter = new PytestTestExecutionAdapter( diff --git a/extensions/positron-python/src/test/testing/testController/utils.unit.test.ts b/extensions/positron-python/src/test/testing/testController/utils.unit.test.ts index 36ee42ab022..dbf8b8249b9 100644 --- a/extensions/positron-python/src/test/testing/testController/utils.unit.test.ts +++ b/extensions/positron-python/src/test/testing/testController/utils.unit.test.ts @@ -9,10 +9,8 @@ import { ExtractJsonRPCData, parseJsonRPCHeadersAndData, splitTestNameWithRegex, - mapToArgs, - addArgIfNotExist, argKeyExists, - argsToMap, + addValueIfKeyNotExist, } from '../../../client/testing/testController/common/utils'; suite('Test Controller Utils: JSON RPC', () => { @@ -163,180 +161,42 @@ ${data}${secondPayload}`; }); }); suite('Test Controller Utils: Args Mapping', () => { - test('Converts map with mixed values to array of strings', async () => { - const inputMap = { - key1: ['value1'], - key2: null, - key3: undefined, - key4: ['value4'], - }; - const expectedOutput = ['key1=value1', 'key2', 'key4=value4']; - - const result = mapToArgs(inputMap); - - assert.deepStrictEqual(result, expectedOutput); - }); - - test('Returns an empty array for an empty map', async () => { - const inputMap = {}; - const expectedOutput: unknown[] = []; - - const result = mapToArgs(inputMap); - - assert.deepStrictEqual(result, expectedOutput); - }); - - test('Skips undefined values', async () => { - const inputMap = { - key1: undefined, - key2: undefined, - }; - const expectedOutput: unknown[] = []; - - const result = mapToArgs(inputMap); - - assert.deepStrictEqual(result, expectedOutput); - }); - - test('Handles null values correctly', async () => { - const inputMap = { - key1: null, - key2: null, - }; - const expectedOutput = ['key1', 'key2']; - - const result = mapToArgs(inputMap); - - 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] }); - }); - - test('Does not overwrite existing argument', () => { - const map = { existingKey: ['existingValue'] }; - const argKey = 'existingKey'; - const argValue = 'newValue'; - - const updatedMap = addArgIfNotExist(map, argKey, argValue); - - assert.deepStrictEqual(updatedMap, { [argKey]: ['existingValue'] }); - }); - - test('Handles null value for new key', () => { - const map = {}; - const argKey = 'nullKey'; - const argValue = null; - - const updatedMap = addArgIfNotExist(map, argKey, argValue); - - assert.deepStrictEqual(updatedMap, { [argKey]: argValue }); - }); - - test('Ignores addition if key exists with null value', () => { - const map = { nullKey: null }; - const argKey = 'nullKey'; - const argValue = 'newValue'; - - const updatedMap = addArgIfNotExist(map, argKey, argValue); - - assert.deepStrictEqual(updatedMap, { [argKey]: null }); - }); - - test('Complex test for argKeyExists with various key types', () => { - const map = { - stringKey: ['stringValue'], - nullKey: null, - // Note: not adding an 'undefinedKey' explicitly since it's not present and hence undefined by default - }; - - // Should return true for keys that are present, even with a null value - assert.strictEqual( - argKeyExists(map, 'stringKey'), - true, - "Failed to recognize 'stringKey' which has a string value.", - ); - assert.strictEqual( - argKeyExists(map, 'nullKey'), - true, - "Failed to recognize 'nullKey' which has a null value.", - ); - - // Should return false for keys that are not present - assert.strictEqual( - argKeyExists(map, 'undefinedKey'), - false, - "Incorrectly recognized 'undefinedKey' as existing.", - ); - }); - test('Converts array of strings with "=" into a map', () => { - const args = ['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); - - assert.deepStrictEqual(resultMap, expectedMap); - }); - - test('Assigns null to keys without "="', () => { - const args = ['key1', 'key2']; - const expectedMap = { key1: null, key2: null }; - - const resultMap = argsToMap(args); - - assert.deepStrictEqual(resultMap, expectedMap); - }); - - test('Handles mixed keys with and without "="', () => { - const args = ['key1=value1', 'key2']; - const expectedMap = { key1: ['value1'], key2: null }; - - const resultMap = argsToMap(args); - - assert.deepStrictEqual(resultMap, expectedMap); - }); - - test('Handles strings with multiple "=" characters', () => { - const args = ['key1=part1=part2']; - const expectedMap = { key1: ['part1=part2'] }; - - const resultMap = argsToMap(args); + suite('addValueIfKeyNotExist', () => { + test('should add key-value pair if key does not exist', () => { + const args = ['key1=value1', 'key2=value2']; + const result = addValueIfKeyNotExist(args, 'key3', 'value3'); + assert.deepEqual(result, ['key1=value1', 'key2=value2', 'key3=value3']); + }); - assert.deepStrictEqual(resultMap, expectedMap); + test('should not add key-value pair if key already exists', () => { + const args = ['key1=value1', 'key2=value2']; + const result = addValueIfKeyNotExist(args, 'key1', 'value3'); + assert.deepEqual(result, ['key1=value1', 'key2=value2']); + }); + test('should not add key-value pair if key exists as a solo element', () => { + const args = ['key1=value1', 'key2']; + const result = addValueIfKeyNotExist(args, 'key2', 'yellow'); + assert.deepEqual(result, ['key1=value1', 'key2']); + }); + test('add just key if value is null', () => { + const args = ['key1=value1', 'key2']; + const result = addValueIfKeyNotExist(args, 'key3', null); + assert.deepEqual(result, ['key1=value1', 'key2', 'key3']); + }); }); - test('Returns an empty map for an empty input array', () => { - const args: ReadonlyArray = []; - const expectedMap = {}; - - const resultMap = argsToMap(args); + suite('argKeyExists', () => { + test('should return true if key exists', () => { + const args = ['key1=value1', 'key2=value2']; + const result = argKeyExists(args, 'key1'); + assert.deepEqual(result, true); + }); - assert.deepStrictEqual(resultMap, expectedMap); + test('should return false if key does not exist', () => { + const args = ['key1=value1', 'key2=value2']; + const result = argKeyExists(args, 'key3'); + assert.deepEqual(result, false); + }); }); }); });