From 638eb39989d661ff06381ce3b18fa9e39d191a0a Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Fri, 2 Feb 2024 12:59:04 -0800 Subject: [PATCH 01/31] symlink solution for pytest --- .vscode/launch.json | 2 +- .../expected_discovery_test_output.py | 49 +++++ .../tests/pytestadapter/test_discovery.py | 37 ++++ pythonFiles/vscode_pytest/__init__.py | 45 +++- .../pytest/pytestDiscoveryAdapter.ts | 27 +++ .../testing/common/testingAdapter.test.ts | 208 ++++++++++-------- 6 files changed, 273 insertions(+), 95 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 82981a93305d..e448dd15b13b 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -101,7 +101,7 @@ "--extensionTestsPath=${workspaceFolder}/out/test" ], "env": { - "VSC_PYTHON_CI_TEST_GREP": "" // Modify this to run a subset of the single workspace tests + "VSC_PYTHON_CI_TEST_GREP": "End to End Tests: test adapters" // Modify this to run a subset of the single workspace tests }, "stopOnEntry": false, "sourceMaps": true, diff --git a/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py b/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py index d4e91f56b5fe..aef1bf3a557c 100644 --- a/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py +++ b/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py @@ -994,3 +994,52 @@ ], "id_": str(TEST_DATA_PATH), } + +symlink_expected_discovery_output = { + "name": "symlink_root", + "path": "--symlink-path-insert-here--", + "type_": "folder", + "children": [ + { + "name": "tests", + "path": "--symlink-path-insert-here--/tests", + "type_": "folder", + "id_": "--symlink-path-insert-here--/tests", + "children": [ + { + "name": "test_a.py", + "path": "--symlink-path-insert-here--/tests/test_a.py", + "type_": "file", + "id_": "--symlink-path-insert-here--/tests/test_a.py", + "children": [ + { + "name": "test_a_function", + "path": "--symlink-path-insert-here--/tests/test_a.py", + "lineno": "5", + "type_": "test", + "id_": "--symlink-path-insert-here--/tests/test_a.py::test_a_function", + "runID": "--symlink-path-insert-here--/tests/test_a.py::test_a_function", + } + ], + }, + { + "name": "test_b.py", + "path": "--symlink-path-insert-here--/tests/test_b.py", + "type_": "file", + "id_": "--symlink-path-insert-here--/tests/test_b.py", + "children": [ + { + "name": "test_b_function", + "path": "--symlink-path-insert-here--/tests/test_b.py", + "lineno": "5", + "type_": "test", + "id_": "--symlink-path-insert-here--/tests/test_b.py::test_b_function", + "runID": "--symlink-path-insert-here--/tests/test_b.py::test_b_function", + } + ], + }, + ], + } + ], + "id_": "--symlink-path-insert-here--", +} diff --git a/pythonFiles/tests/pytestadapter/test_discovery.py b/pythonFiles/tests/pytestadapter/test_discovery.py index 2630ddef68b0..e5848db5ca0b 100644 --- a/pythonFiles/tests/pytestadapter/test_discovery.py +++ b/pythonFiles/tests/pytestadapter/test_discovery.py @@ -1,7 +1,9 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +import json import os import shutil +import string from typing import Any, Dict, List, Optional import pytest @@ -198,6 +200,41 @@ def test_pytest_collect(file, expected_const): assert actual_item.get("tests") == expected_const +def test_symlink_root_dir(tmp_path): + """ + Test to test pytest discovery with the command line arg --rootdir specified as a symlink path. + Discovery should succeed and testids should be relative to the symlinked root directory. + Keyword arguments: + tmp_path -- pytest fixture that creates a temporary directory. + """ + # create symlink + source = TEST_DATA_PATH / "root" + destination = tmp_path / "symlink_root" + os.symlink(source, destination) + + "--symlink-path-insert-here--" + # Run pytest with the cwd being the resolved symlink path (as it will be when we run the subprocess from node). + actual = runner_with_cwd(["--collect-only", f"--rootdir={destination}"], source) + + # now assert the output is the same, but sub in the tmp_path for a placeholder in the expected output object. + json_expected = json.dumps( + expected_discovery_test_output.symlink_expected_discovery_output + ) + json_expected_new = json_expected.replace( + "--symlink-path-insert-here--", str(destination) + ) + expected = json.loads(json_expected_new) + assert actual + actual_list: List[Dict[str, Any]] = actual + if actual_list is not None: + assert actual_list.pop(-1).get("eot") + actual_item = actual_list.pop(0) + assert all(item in actual_item.keys() for item in ("status", "cwd", "error")) + assert actual_item.get("status") == "success" + assert actual_item.get("cwd") == os.fspath(source) + assert actual_item.get("tests") == expected + + def test_pytest_root_dir(): """ Test to test pytest discovery with the command line arg --rootdir specified to be a subfolder diff --git a/pythonFiles/vscode_pytest/__init__.py b/pythonFiles/vscode_pytest/__init__.py index a565fbf930a6..8acf20c7181d 100644 --- a/pythonFiles/vscode_pytest/__init__.py +++ b/pythonFiles/vscode_pytest/__init__.py @@ -57,6 +57,7 @@ def __init__(self, message): collected_tests_so_far = list() TEST_PORT = os.getenv("TEST_PORT") TEST_UUID = os.getenv("TEST_UUID") +SYMLINK_PATH = None def pytest_load_initial_conftests(early_config, parser, args): @@ -75,6 +76,26 @@ def pytest_load_initial_conftests(early_config, parser, args): global IS_DISCOVERY IS_DISCOVERY = True + # check if --rootdir is in the args + for arg in args: + if "--rootdir=" in arg: + rootdir = arg.split("--rootdir=")[1] + print("--rootdir= argument found:", rootdir) + if not os.path.exists(rootdir): + raise VSCodePytestError( + f"The path set in the argument --rootdir={rootdir} does not exist." + ) + if ( + os.path.islink(rootdir) + and pathlib.Path(os.path.realpath(rootdir)) == pathlib.Path.cwd() + ): + print( + f"rootdir argument, {rootdir}, is identified as a symlink to the cwd, {pathlib.Path.cwd()}. + Therefore setting symlink path to rootdir argument." + ) + global SYMLINK_PATH + SYMLINK_PATH = pathlib.Path(rootdir) + def pytest_internalerror(excrepr, excinfo): """A pytest hook that is called when an internal error occurs. @@ -388,6 +409,12 @@ def build_test_tree(session: pytest.Session) -> TestNode: class_nodes_dict: Dict[str, TestNode] = {} function_nodes_dict: Dict[str, TestNode] = {} + # Check to see if the global variable for symlink path is set + global SYMLINK_PATH + if SYMLINK_PATH: + session_node["path"] = SYMLINK_PATH + session_node["id_"] = os.fspath(SYMLINK_PATH) + for test_case in session.items: test_node = create_test_node(test_case) if isinstance(test_case.parent, pytest.Class): @@ -645,13 +672,29 @@ class EOTPayloadDict(TypedDict): def get_node_path(node: Any) -> pathlib.Path: - """A function that returns the path of a node given the switch to pathlib.Path.""" + """ + A function that returns the path of a node given the switch to pathlib.Path. + It also evaluates if the node is a symlink and returns the equivalent path. + """ path = getattr(node, "path", None) or pathlib.Path(node.fspath) if not path: raise VSCodePytestError( f"Unable to find path for node: {node}, node.path: {node.path}, node.fspath: {node.fspath}" ) + + global SYMLINK_PATH + # Check for the session node since it has the symlink already. + if SYMLINK_PATH and not isinstance(node, pytest.Session): + # Get relative between the cwd (resolved path) and the node path. + try: + rel_path = os.path.relpath(path, pathlib.Path.cwd()) + # Calculate the new node path by making it relative to the symlink path. + sym_path = pathlib.Path(os.path.join(SYMLINK_PATH, rel_path)) + return sym_path + except Exception as e: + raise VSCodePytestError(f"Error occurred while calculating symlink equivalent from node path: {e} \n", + "SYMLINK_PATH: {SYMLINK_PATH}, \n node path: {path}, \n cwd: {{pathlib.Path.cwd()}}") return path diff --git a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts index 39dc87ed12c1..f0ad3ef640c5 100644 --- a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts +++ b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import * as path from 'path'; import { Uri } from 'vscode'; +import * as fs from 'fs'; import { ExecutionFactoryCreateWithEnvironmentOptions, IPythonExecutionFactory, @@ -69,6 +70,19 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { const { pytestArgs } = settings.testing; const cwd = settings.testing.cwd && settings.testing.cwd.length > 0 ? settings.testing.cwd : uri.fsPath; + const stats = fs.lstatSync(cwd); + + if (stats.isSymbolicLink()) { + console.log('The path is a symbolic link.'); + const rootDir = getRootDirValue(pytestArgs); + if (rootDir === null) { + pytestArgs.push(`--rootdir=${cwd}`); + console.log(`The --rootdir argument is set to ${cwd} since it is a symlink.`); + } + } else { + console.log('The path is not a symbolic link.'); + } + // get and edit env vars const mutableEnv = { ...(await this.envVarsService?.getEnvironmentVariables(uri)), @@ -150,3 +164,16 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { await deferredTillExecClose.promise; } } + +function getRootDirValue(pytestArgs: string[]): string | null { + // Find the argument that contains '--rootdir=' + const rootdirArg = pytestArgs.find((arg) => arg.startsWith('--rootdir=')); + + if (rootdirArg) { + // Extract the value after '--rootdir=' + return rootdirArg.split('=')[1]; + } + + // Return null if '--rootdir=' is not found + return null; +} diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index a9ed25194fa9..3220a6a93acd 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -5,6 +5,7 @@ import { TestController, TestRun, Uri } from 'vscode'; import * as typeMoq from 'typemoq'; import * as path from 'path'; import * as assert from 'assert'; +import * as fs from 'fs'; import { PytestTestDiscoveryAdapter } from '../../../client/testing/testController/pytest/pytestDiscoveryAdapter'; import { ITestController, ITestResultResolver } from '../../../client/testing/testController/common/types'; import { PythonTestServer } from '../../../client/testing/testController/common/server'; @@ -96,7 +97,100 @@ suite('End to End Tests: test adapters', () => { teardown(async () => { pythonTestServer.dispose(); }); - test('unittest discovery adapter small workspace', async () => { + // test('unittest discovery adapter small workspace', async () => { + // // result resolver and saved data for assertions + // let actualData: { + // cwd: string; + // tests?: unknown; + // status: 'success' | 'error'; + // error?: string[]; + // }; + // workspaceUri = Uri.parse(rootPathSmallWorkspace); + // resultResolver = new PythonResultResolver(testController, unittestProvider, workspaceUri); + // let callCount = 0; + // resultResolver._resolveDiscovery = async (payload, _token?) => { + // traceLog(`resolveDiscovery ${payload}`); + // callCount = callCount + 1; + // actualData = payload; + // return Promise.resolve(); + // }; + + // // set workspace to test workspace folder and set up settings + + // configService.getSettings(workspaceUri).testing.unittestArgs = ['-s', '.', '-p', '*test*.py']; + + // // run unittest discovery + // const discoveryAdapter = new UnittestTestDiscoveryAdapter( + // pythonTestServer, + // configService, + // testOutputChannel.object, + // resultResolver, + // envVarsService, + // ); + + // await discoveryAdapter.discoverTests(workspaceUri).finally(() => { + // // verification after discovery is complete + + // // 1. Check the status is "success" + // assert.strictEqual( + // actualData.status, + // 'success', + // `Expected status to be 'success' instead status is ${actualData.status}`, + // ); + // // 2. Confirm no errors + // assert.strictEqual(actualData.error, undefined, "Expected no errors in 'error' field"); + // // 3. Confirm tests are found + // assert.ok(actualData.tests, 'Expected tests to be present'); + + // assert.strictEqual(callCount, 1, 'Expected _resolveDiscovery to be called once'); + // }); + // }); + + // test('unittest discovery adapter large workspace', async () => { + // // result resolver and saved data for assertions + // let actualData: { + // cwd: string; + // tests?: unknown; + // status: 'success' | 'error'; + // error?: string[]; + // }; + // resultResolver = new PythonResultResolver(testController, unittestProvider, workspaceUri); + // let callCount = 0; + // resultResolver._resolveDiscovery = async (payload, _token?) => { + // traceLog(`resolveDiscovery ${payload}`); + // callCount = callCount + 1; + // actualData = payload; + // return Promise.resolve(); + // }; + + // // set settings to work for the given workspace + // workspaceUri = Uri.parse(rootPathLargeWorkspace); + // configService.getSettings(workspaceUri).testing.unittestArgs = ['-s', '.', '-p', '*test*.py']; + // // run discovery + // const discoveryAdapter = new UnittestTestDiscoveryAdapter( + // pythonTestServer, + // configService, + // testOutputChannel.object, + // resultResolver, + // envVarsService, + // ); + + // await discoveryAdapter.discoverTests(workspaceUri).finally(() => { + // // 1. Check the status is "success" + // assert.strictEqual( + // actualData.status, + // 'success', + // `Expected status to be 'success' instead status is ${actualData.status}`, + // ); + // // 2. Confirm no errors + // assert.strictEqual(actualData.error, undefined, "Expected no errors in 'error' field"); + // // 3. Confirm tests are found + // assert.ok(actualData.tests, 'Expected tests to be present'); + + // assert.strictEqual(callCount, 1, 'Expected _resolveDiscovery to be called once'); + // }); + // }); + test('pytest discovery adapter small workspace', async () => { // result resolver and saved data for assertions let actualData: { cwd: string; @@ -104,99 +198,29 @@ suite('End to End Tests: test adapters', () => { status: 'success' | 'error'; error?: string[]; }; - workspaceUri = Uri.parse(rootPathSmallWorkspace); - resultResolver = new PythonResultResolver(testController, unittestProvider, workspaceUri); - let callCount = 0; - resultResolver._resolveDiscovery = async (payload, _token?) => { - traceLog(`resolveDiscovery ${payload}`); - callCount = callCount + 1; - actualData = payload; - return Promise.resolve(); - }; - - // set workspace to test workspace folder and set up settings - - configService.getSettings(workspaceUri).testing.unittestArgs = ['-s', '.', '-p', '*test*.py']; - - // run unittest discovery - const discoveryAdapter = new UnittestTestDiscoveryAdapter( - pythonTestServer, - configService, - testOutputChannel.object, - resultResolver, - envVarsService, + // set workspace to test workspace folder + const target = rootPathSmallWorkspace; + const rootPathSmallWorkspaceSymlink = path.join( + EXTENSION_ROOT_DIR_FOR_TESTS, + 'src', + 'testTestingRootWkspc', + 'symlinkSmallWorkspace3', ); - - await discoveryAdapter.discoverTests(workspaceUri).finally(() => { - // verification after discovery is complete - - // 1. Check the status is "success" - assert.strictEqual( - actualData.status, - 'success', - `Expected status to be 'success' instead status is ${actualData.status}`, - ); - // 2. Confirm no errors - assert.strictEqual(actualData.error, undefined, "Expected no errors in 'error' field"); - // 3. Confirm tests are found - assert.ok(actualData.tests, 'Expected tests to be present'); - - assert.strictEqual(callCount, 1, 'Expected _resolveDiscovery to be called once'); + fs.symlink(target, rootPathSmallWorkspaceSymlink, 'dir', (err) => { + if (err) { + console.error(err); + } else { + console.log('Directory symlink created'); + } }); - }); + workspaceUri = Uri.parse(rootPathSmallWorkspaceSymlink); + const stats = fs.lstatSync(rootPathSmallWorkspaceSymlink); - test('unittest discovery adapter large workspace', async () => { - // result resolver and saved data for assertions - let actualData: { - cwd: string; - tests?: unknown; - status: 'success' | 'error'; - error?: string[]; - }; - resultResolver = new PythonResultResolver(testController, unittestProvider, workspaceUri); - let callCount = 0; - resultResolver._resolveDiscovery = async (payload, _token?) => { - traceLog(`resolveDiscovery ${payload}`); - callCount = callCount + 1; - actualData = payload; - return Promise.resolve(); - }; - - // set settings to work for the given workspace - workspaceUri = Uri.parse(rootPathLargeWorkspace); - configService.getSettings(workspaceUri).testing.unittestArgs = ['-s', '.', '-p', '*test*.py']; - // run discovery - const discoveryAdapter = new UnittestTestDiscoveryAdapter( - pythonTestServer, - configService, - testOutputChannel.object, - resultResolver, - envVarsService, - ); - - await discoveryAdapter.discoverTests(workspaceUri).finally(() => { - // 1. Check the status is "success" - assert.strictEqual( - actualData.status, - 'success', - `Expected status to be 'success' instead status is ${actualData.status}`, - ); - // 2. Confirm no errors - assert.strictEqual(actualData.error, undefined, "Expected no errors in 'error' field"); - // 3. Confirm tests are found - assert.ok(actualData.tests, 'Expected tests to be present'); - - assert.strictEqual(callCount, 1, 'Expected _resolveDiscovery to be called once'); - }); - }); - test('pytest discovery adapter small workspace', async () => { - // result resolver and saved data for assertions - let actualData: { - cwd: string; - tests?: unknown; - status: 'success' | 'error'; - error?: string[]; - }; + if (stats.isSymbolicLink()) { + console.log('The path is a symbolic link.'); + } else { + console.log('The path is not a symbolic link.'); + } resultResolver = new PythonResultResolver(testController, pytestProvider, workspaceUri); let callCount = 0; resultResolver._resolveDiscovery = async (payload, _token?) => { @@ -214,8 +238,6 @@ suite('End to End Tests: test adapters', () => { envVarsService, ); - // set workspace to test workspace folder - workspaceUri = Uri.parse(rootPathSmallWorkspace); await discoveryAdapter.discoverTests(workspaceUri, pythonExecFactory).finally(() => { // verification after discovery is complete From f52e093aa81609a42ecd0397d9cfd0b3387b79a7 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 5 Feb 2024 14:27:46 -0800 Subject: [PATCH 02/31] add arg mapping to pytest adapters --- .../testing/testController/common/utils.ts | 80 +++++++++++++++++++ .../pytest/pytestDiscoveryAdapter.ts | 29 ++----- .../pytest/pytestExecutionAdapter.ts | 21 ++--- 3 files changed, 100 insertions(+), 30 deletions(-) diff --git a/src/client/testing/testController/common/utils.ts b/src/client/testing/testController/common/utils.ts index 23ee881a405a..0e81154a899c 100644 --- a/src/client/testing/testController/common/utils.ts +++ b/src/client/testing/testController/common/utils.ts @@ -349,3 +349,83 @@ export function splitTestNameWithRegex(testName: string): [string, string] { } return [testName, testName]; } + +/** + * 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. + */ +export const argsToMap = (args: ReadonlyArray): { [key: string]: string | null | undefined } => { + const map: { [key: string]: string | null } = {}; + for (const arg of args) { + const delimiter = arg.indexOf('='); + if (delimiter === -1) { + map[arg] = null; + } else { + map[arg.slice(0, delimiter)] = arg.slice(delimiter + 1); + } + } + + 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]: 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}`); + } + + return out; +}; + +/** + * 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. + */ +export function addArgIfNotExist( + map: { [key: string]: string | null | undefined }, + argKey: string, + argValue: string | null, +): { [key: string]: string | null | undefined } { + // Only add the argument if it doesn't exist in the map. + if (map[argKey] === undefined) { + map[argKey] = argValue; + } + + 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]: string | null | undefined }, argKey: string): boolean { + return map[argKey] !== undefined; +} diff --git a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts index f0ad3ef640c5..c48ac9488936 100644 --- a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts +++ b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts @@ -25,6 +25,9 @@ import { createEOTPayload, createTestingDeferred, fixLogLinesNoTrailing, + argsToMap, + addArgIfNotExist, + mapToArgs, } from '../common/utils'; import { IEnvironmentVariablesProvider } from '../../../common/variables/types'; @@ -67,18 +70,15 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { const relativePathToPytest = 'pythonFiles'; const fullPluginPath = path.join(EXTENSION_ROOT_DIR, relativePathToPytest); const settings = this.configSettings.getSettings(uri); - const { pytestArgs } = settings.testing; + let pytestArgsMap = argsToMap(settings.testing.pytestArgs); + console.log(`pytestArgsMap: ${JSON.stringify(pytestArgsMap)}`); 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()) { console.log('The path is a symbolic link.'); - const rootDir = getRootDirValue(pytestArgs); - if (rootDir === null) { - pytestArgs.push(`--rootdir=${cwd}`); - console.log(`The --rootdir argument is set to ${cwd} since it is a symlink.`); - } + pytestArgsMap = addArgIfNotExist(pytestArgsMap, '--rootdir', cwd); } else { console.log('The path is not a symbolic link.'); } @@ -112,7 +112,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(pytestArgs); + const execArgs = ['-m', 'pytest', '-p', 'vscode_pytest', '--collect-only'].concat(mapToArgs(pytestArgsMap)); traceVerbose(`Running pytest discovery with command: ${execArgs.join(' ')} for workspace ${uri.fsPath}.`); const deferredTillExecClose: Deferred = createTestingDeferred(); @@ -164,16 +164,3 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { await deferredTillExecClose.promise; } } - -function getRootDirValue(pytestArgs: string[]): string | null { - // Find the argument that contains '--rootdir=' - const rootdirArg = pytestArgs.find((arg) => arg.startsWith('--rootdir=')); - - if (rootdirArg) { - // Extract the value after '--rootdir=' - return rootdirArg.split('=')[1]; - } - - // Return null if '--rootdir=' is not found - return null; -} diff --git a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts index 8995a182a774..d366bdfc9718 100644 --- a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts +++ b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts @@ -129,15 +129,16 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { try { // Remove positional test folders and files, we will add as needed per node const testArgs = removePositionalFoldersAndFiles(pytestArgs); + let testArgsMap = utils.argsToMap(testArgs); // if user has provided `--rootdir` then use that, otherwise add `cwd` - if (testArgs.filter((a) => a.startsWith('--rootdir')).length === 0) { - // Make sure root dir is set so pytest can find the relative paths - testArgs.splice(0, 0, '--rootdir', uri.fsPath); - } + // root dir is required so pytest can find the relative paths and for symlinks + utils.addArgIfNotExist(testArgsMap, '--rootdir', cwd); - if (debugBool && !testArgs.some((a) => a.startsWith('--capture') || a === '-s')) { - testArgs.push('--capture', 'no'); + // -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'); } // add port with run test ids to env vars @@ -162,7 +163,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { const pytestUUID = uuid.toString(); const launchOptions: LaunchOptions = { cwd, - args: testArgs, + args: utils.mapToArgs(testArgsMap), token: spawnOptions.token, testProvider: PYTEST_PROVIDER, pytestPort, @@ -170,7 +171,9 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { runTestIdsPort: pytestRunTestIdsPort.toString(), }; traceInfo( - `Running DEBUG pytest with arguments: ${testArgs.join(' ')} for workspace ${uri.fsPath} \r\n`, + `Running DEBUG pytest with arguments: ${utils.mapToArgs(testArgsMap).join(' ')} for workspace ${ + uri.fsPath + } \r\n`, ); await debugLauncher!.launchDebugger(launchOptions, () => { deferredTillEOT?.resolve(); @@ -180,7 +183,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, ...testArgs]; + const runArgs = [scriptPath, ...utils.mapToArgs(testArgsMap)]; traceInfo(`Running pytest with arguments: ${runArgs.join(' ')} for workspace ${uri.fsPath} \r\n`); let resultProc: ChildProcess | undefined; From 99349e6e83a78e72f58985cc9a7916565b11a5ab Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 5 Feb 2024 14:48:45 -0800 Subject: [PATCH 03/31] testing for util functions --- .../testing/testController/utils.unit.test.ts | 174 ++++++++++++++++++ 1 file changed, 174 insertions(+) diff --git a/src/test/testing/testController/utils.unit.test.ts b/src/test/testing/testController/utils.unit.test.ts index 014261a40232..5bcf8dfa10c8 100644 --- a/src/test/testing/testController/utils.unit.test.ts +++ b/src/test/testing/testController/utils.unit.test.ts @@ -9,6 +9,10 @@ import { ExtractJsonRPCData, parseJsonRPCHeadersAndData, splitTestNameWithRegex, + mapToArgs, + addArgIfNotExist, + argKeyExists, + argsToMap, } from '../../../client/testing/testController/common/utils'; suite('Test Controller Utils: JSON RPC', () => { @@ -158,4 +162,174 @@ ${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('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('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', + 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('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); + + assert.deepStrictEqual(resultMap, expectedMap); + }); + + test('Returns an empty map for an empty input array', () => { + const args: ReadonlyArray = []; + const expectedMap = {}; + + const resultMap = argsToMap(args); + + assert.deepStrictEqual(resultMap, expectedMap); + }); + }); }); From 76973c7a7fbdf993e18bcfad73400c8c8e5e0ad2 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 5 Feb 2024 15:07:49 -0800 Subject: [PATCH 04/31] fix existing tests --- .../pytest/pytestDiscoveryAdapter.unit.test.ts | 7 +++++++ .../pytest/pytestExecutionAdapter.unit.test.ts | 7 ++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/test/testing/testController/pytest/pytestDiscoveryAdapter.unit.test.ts b/src/test/testing/testController/pytest/pytestDiscoveryAdapter.unit.test.ts index 7badb5a0350d..683c8017d115 100644 --- a/src/test/testing/testController/pytest/pytestDiscoveryAdapter.unit.test.ts +++ b/src/test/testing/testController/pytest/pytestDiscoveryAdapter.unit.test.ts @@ -6,6 +6,8 @@ import { Uri } from 'vscode'; import * as typeMoq from 'typemoq'; import * as path from 'path'; import { Observable } from 'rxjs/Observable'; +import * as fs from 'fs'; +import * as sinon from 'sinon'; import { IConfigurationService, ITestOutputChannel } from '../../../../client/common/types'; import { PytestTestDiscoveryAdapter } from '../../../../client/testing/testController/pytest/pytestDiscoveryAdapter'; import { ITestServer } from '../../../../client/testing/testController/common/types'; @@ -52,6 +54,8 @@ suite('pytest test discovery adapter', () => { TEST_PORT: portNum.toString(), }; + sinon.stub(fs, 'lstatSync').returns({ isFile: () => true, isSymbolicLink: () => false } as fs.Stats); + // set up test server testServer = typeMoq.Mock.ofType(); testServer.setup((t) => t.getPort()).returns(() => portNum); @@ -94,6 +98,9 @@ suite('pytest test discovery adapter', () => { }; }); }); + teardown(() => { + sinon.restore(); + }); test('Discovery should call exec with correct basic args', async () => { // set up exec mock deferred = createDeferred(); diff --git a/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts b/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts index a097df654360..d2ab19368b2d 100644 --- a/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts +++ b/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts @@ -171,7 +171,8 @@ suite('pytest test execution adapter', () => { const pathToPythonFiles = path.join(EXTENSION_ROOT_DIR, 'pythonFiles'); const pathToPythonScript = path.join(pathToPythonFiles, 'vscode_pytest', 'run_pytest_script.py'); - const expectedArgs = [pathToPythonScript, '--rootdir', myTestPath]; + const rootDirArg = `--rootdir=${myTestPath}`; + const expectedArgs = [pathToPythonScript, rootDirArg]; const expectedExtraVariables = { PYTHONPATH: pathToPythonFiles, TEST_UUID: 'uuid123', @@ -238,7 +239,7 @@ suite('pytest test execution adapter', () => { const pathToPythonFiles = path.join(EXTENSION_ROOT_DIR, 'pythonFiles'); const pathToPythonScript = path.join(pathToPythonFiles, 'vscode_pytest', 'run_pytest_script.py'); - const expectedArgs = [pathToPythonScript, '--rootdir', myTestPath]; + const expectedArgs = [pathToPythonScript, `--rootdir=${newCwd}`]; const expectedExtraVariables = { PYTHONPATH: pathToPythonFiles, TEST_UUID: 'uuid123', @@ -305,7 +306,7 @@ suite('pytest test execution adapter', () => { x.launchDebugger( typeMoq.It.is((launchOptions) => { assert.equal(launchOptions.cwd, uri.fsPath); - assert.deepEqual(launchOptions.args, ['--rootdir', myTestPath, '--capture', 'no']); + assert.deepEqual(launchOptions.args, [`--rootdir=${myTestPath}`, '--capture=no']); assert.equal(launchOptions.testProvider, 'pytest'); assert.equal(launchOptions.pytestPort, '12345'); assert.equal(launchOptions.pytestUUID, 'uuid123'); From e648fdae30f400ea49bd8e482e06e4e64aad4264 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Tue, 6 Feb 2024 10:53:50 -0800 Subject: [PATCH 05/31] minor fixes --- pythonFiles/vscode_pytest/__init__.py | 10 +-- .../pytestDiscoveryAdapter.unit.test.ts | 65 ++++++++++++++++++- 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/pythonFiles/vscode_pytest/__init__.py b/pythonFiles/vscode_pytest/__init__.py index 8acf20c7181d..bfb12dded904 100644 --- a/pythonFiles/vscode_pytest/__init__.py +++ b/pythonFiles/vscode_pytest/__init__.py @@ -90,8 +90,8 @@ def pytest_load_initial_conftests(early_config, parser, args): and pathlib.Path(os.path.realpath(rootdir)) == pathlib.Path.cwd() ): print( - f"rootdir argument, {rootdir}, is identified as a symlink to the cwd, {pathlib.Path.cwd()}. - Therefore setting symlink path to rootdir argument." + f"rootdir argument, {rootdir}, is identified as a symlink to the cwd, {pathlib.Path.cwd()}.", + "Therefore setting symlink path to rootdir argument.", ) global SYMLINK_PATH SYMLINK_PATH = pathlib.Path(rootdir) @@ -693,8 +693,10 @@ def get_node_path(node: Any) -> pathlib.Path: sym_path = pathlib.Path(os.path.join(SYMLINK_PATH, rel_path)) return sym_path except Exception as e: - raise VSCodePytestError(f"Error occurred while calculating symlink equivalent from node path: {e} \n", - "SYMLINK_PATH: {SYMLINK_PATH}, \n node path: {path}, \n cwd: {{pathlib.Path.cwd()}}") + raise VSCodePytestError( + f"Error occurred while calculating symlink equivalent from node path: {e} \n", + "SYMLINK_PATH: {SYMLINK_PATH}, \n node path: {path}, \n cwd: {{pathlib.Path.cwd()}}", + ) return path diff --git a/src/test/testing/testController/pytest/pytestDiscoveryAdapter.unit.test.ts b/src/test/testing/testController/pytest/pytestDiscoveryAdapter.unit.test.ts index 683c8017d115..45f6d4ffa8eb 100644 --- a/src/test/testing/testController/pytest/pytestDiscoveryAdapter.unit.test.ts +++ b/src/test/testing/testController/pytest/pytestDiscoveryAdapter.unit.test.ts @@ -54,8 +54,6 @@ suite('pytest test discovery adapter', () => { TEST_PORT: portNum.toString(), }; - sinon.stub(fs, 'lstatSync').returns({ isFile: () => true, isSymbolicLink: () => false } as fs.Stats); - // set up test server testServer = typeMoq.Mock.ofType(); testServer.setup((t) => t.getPort()).returns(() => portNum); @@ -111,6 +109,7 @@ suite('pytest test discovery adapter', () => { deferred.resolve(); return Promise.resolve(execService.object); }); + sinon.stub(fs, 'lstatSync').returns({ isFile: () => true, isSymbolicLink: () => false } as fs.Stats); adapter = new PytestTestDiscoveryAdapter(testServer.object, configService, outputChannel.object); adapter.discoverTests(uri, execFactory.object); // add in await and trigger @@ -150,6 +149,8 @@ suite('pytest test discovery adapter', () => { }), } as unknown) as IConfigurationService; + sinon.stub(fs, 'lstatSync').returns({ isFile: () => true, isSymbolicLink: () => false } as fs.Stats); + // set up exec mock deferred = createDeferred(); execFactory = typeMoq.Mock.ofType(); @@ -168,6 +169,7 @@ suite('pytest test discovery adapter', () => { mockProc.trigger('close'); // verification + const expectedArgs = ['-m', 'pytest', '-p', 'vscode_pytest', '--collect-only', '.', 'abc', 'xyz']; execService.verify( (x) => @@ -183,4 +185,63 @@ suite('pytest test discovery adapter', () => { typeMoq.Times.once(), ); }); + test('Test discovery adds cwd to pytest args when path is symlink', async () => { + sinon.stub(fs, 'lstatSync').returns({ + isFile: () => true, + isSymbolicLink: () => true, + } as fs.Stats); + + // set up a config service with different pytest args + const configServiceNew: IConfigurationService = ({ + getSettings: () => ({ + testing: { + pytestArgs: ['.', 'abc', 'xyz'], + cwd: expectedPath, + }, + }), + } as unknown) as IConfigurationService; + + // set up exec mock + deferred = createDeferred(); + execFactory = typeMoq.Mock.ofType(); + execFactory + .setup((x) => x.createActivatedEnvironment(typeMoq.It.isAny())) + .returns(() => { + deferred.resolve(); + return Promise.resolve(execService.object); + }); + + adapter = new PytestTestDiscoveryAdapter(testServer.object, configServiceNew, outputChannel.object); + adapter.discoverTests(uri, execFactory.object); + // add in await and trigger + await deferred.promise; + await deferred2.promise; + mockProc.trigger('close'); + + // verification + const expectedArgs = [ + '-m', + 'pytest', + '-p', + 'vscode_pytest', + '--collect-only', + '.', + 'abc', + 'xyz', + `--rootdir=${expectedPath}`, + ]; + execService.verify( + (x) => + x.execObservable( + expectedArgs, + typeMoq.It.is((options) => { + assert.deepEqual(options.env, expectedExtraVariables); + assert.equal(options.cwd, expectedPath); + assert.equal(options.throwOnStdErr, true); + return true; + }), + ), + typeMoq.Times.once(), + ); + }); }); From 8d8d4fdb4c76413f53cf79e56dd51e518d728930 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Tue, 6 Feb 2024 15:09:16 -0800 Subject: [PATCH 06/31] symlink end to end test --- pythonFiles/vscode_pytest/__init__.py | 10 ++- .../testing/common/testingAdapter.test.ts | 74 +++++++++++++++---- .../symlinkWorkspace/smallWorkspace | 1 + 3 files changed, 69 insertions(+), 16 deletions(-) create mode 120000 src/testTestingRootWkspc/symlinkWorkspace/smallWorkspace diff --git a/pythonFiles/vscode_pytest/__init__.py b/pythonFiles/vscode_pytest/__init__.py index bfb12dded904..868ff5b87e19 100644 --- a/pythonFiles/vscode_pytest/__init__.py +++ b/pythonFiles/vscode_pytest/__init__.py @@ -90,7 +90,7 @@ def pytest_load_initial_conftests(early_config, parser, args): and pathlib.Path(os.path.realpath(rootdir)) == pathlib.Path.cwd() ): print( - f"rootdir argument, {rootdir}, is identified as a symlink to the cwd, {pathlib.Path.cwd()}.", + f"Plugin info[vscode-pytest]: rootdir argument, {rootdir}, is identified as a symlink to the cwd, {pathlib.Path.cwd()}.", "Therefore setting symlink path to rootdir argument.", ) global SYMLINK_PATH @@ -347,6 +347,14 @@ def pytest_sessionfinish(session, exitstatus): Exit code 5: No tests were collected """ cwd = pathlib.Path.cwd() + global SYMLINK_PATH + if SYMLINK_PATH: + print("Plugin warning[vscode-pytest]: SYMLINK set, adjusting cwd.") + # Get relative between the cwd (resolved path) and the node path. + rel_path = os.path.relpath(cwd, pathlib.Path.cwd()) + # Calculate the new node path by making it relative to the symlink path. + cwd = pathlib.Path(os.path.join(SYMLINK_PATH, rel_path)) + if IS_DISCOVERY: if not (exitstatus == 0 or exitstatus == 1 or exitstatus == 5): errorNode: TestNode = { diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index 3220a6a93acd..3709c2568a20 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -198,29 +198,65 @@ suite('End to End Tests: test adapters', () => { status: 'success' | 'error'; error?: string[]; }; + resultResolver = new PythonResultResolver(testController, pytestProvider, workspaceUri); + let callCount = 0; + resultResolver._resolveDiscovery = async (payload, _token?) => { + traceLog(`resolveDiscovery ${payload}`); + callCount = callCount + 1; + actualData = payload; + return Promise.resolve(); + }; + // run pytest discovery + const discoveryAdapter = new PytestTestDiscoveryAdapter( + pythonTestServer, + configService, + testOutputChannel.object, + resultResolver, + envVarsService, + ); + // set workspace to test workspace folder - const target = rootPathSmallWorkspace; + workspaceUri = Uri.parse(rootPathSmallWorkspace); + await discoveryAdapter.discoverTests(workspaceUri, pythonExecFactory).finally(() => { + // verification after discovery is complete + + // 1. Check the status is "success" + assert.strictEqual( + actualData.status, + 'success', + `Expected status to be 'success' instead status is ${actualData.status}`, + ); // 2. Confirm no errors + assert.strictEqual(actualData.error?.length, 0, "Expected no errors in 'error' field"); + // 3. Confirm tests are found + assert.ok(actualData.tests, 'Expected tests to be present'); + + assert.strictEqual(callCount, 1, 'Expected _resolveDiscovery to be called once'); + }); + }); + test('pytest discovery adapter small workspace with symlink', async () => { + // result resolver and saved data for assertions + let actualData: { + cwd: string; + tests?: unknown; + status: 'success' | 'error'; + error?: string[]; + }; + // set workspace to test workspace folder + // const target = rootPathSmallWorkspace; const rootPathSmallWorkspaceSymlink = path.join( EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testTestingRootWkspc', - 'symlinkSmallWorkspace3', + 'symlinkWorkspace', + 'smallWorkspace', ); - fs.symlink(target, rootPathSmallWorkspaceSymlink, 'dir', (err) => { - if (err) { - console.error(err); - } else { - console.log('Directory symlink created'); - } - }); + const testSimpleSymlinkPath = path.join(rootPathSmallWorkspaceSymlink, 'test_simple.py'); workspaceUri = Uri.parse(rootPathSmallWorkspaceSymlink); const stats = fs.lstatSync(rootPathSmallWorkspaceSymlink); - if (stats.isSymbolicLink()) { - console.log('The path is a symbolic link.'); - } else { - console.log('The path is not a symbolic link.'); - } + // confirm that the path is a symbolic link + assert.ok(stats.isSymbolicLink(), 'The path is not a symbolic link but must be for this test.'); + resultResolver = new PythonResultResolver(testController, pytestProvider, workspaceUri); let callCount = 0; resultResolver._resolveDiscovery = async (payload, _token?) => { @@ -250,7 +286,15 @@ suite('End to End Tests: test adapters', () => { assert.strictEqual(actualData.error?.length, 0, "Expected no errors in 'error' field"); // 3. Confirm tests are found assert.ok(actualData.tests, 'Expected tests to be present'); - + // 4. Confirm that the cwd returned is the symlink path + assert.strictEqual(actualData.cwd, rootPathSmallWorkspaceSymlink, 'Expected cwd to be the symlink path'); + // 5. Confirm that the test's path is also using the symlink as the root + assert.strictEqual( + (actualData.tests as { children: { path: string }[] }).children[0].path, + testSimpleSymlinkPath, + 'Expected test path to be the symlink path', + ); + // 5. Confirm that resolveDiscovery was called once assert.strictEqual(callCount, 1, 'Expected _resolveDiscovery to be called once'); }); }); diff --git a/src/testTestingRootWkspc/symlinkWorkspace/smallWorkspace b/src/testTestingRootWkspc/symlinkWorkspace/smallWorkspace new file mode 120000 index 000000000000..f5736036f95e --- /dev/null +++ b/src/testTestingRootWkspc/symlinkWorkspace/smallWorkspace @@ -0,0 +1 @@ +/Users/eleanorboyd/vscode-python/src/testTestingRootWkspc/smallWorkspace \ No newline at end of file From 939b4b762491ae16a82c3a14ee2278a36d72a490 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Thu, 8 Feb 2024 15:06:32 -0800 Subject: [PATCH 07/31] touchups --- .vscode/launch.json | 2 +- .../expected_discovery_test_output.py | 10 +- pythonFiles/vscode_pytest/__init__.py | 1 - .../pytest/pytestDiscoveryAdapter.ts | 9 +- .../testing/common/testingAdapter.test.ts | 186 +++++++++--------- 5 files changed, 106 insertions(+), 102 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index e448dd15b13b..82981a93305d 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -101,7 +101,7 @@ "--extensionTestsPath=${workspaceFolder}/out/test" ], "env": { - "VSC_PYTHON_CI_TEST_GREP": "End to End Tests: test adapters" // Modify this to run a subset of the single workspace tests + "VSC_PYTHON_CI_TEST_GREP": "" // Modify this to run a subset of the single workspace tests }, "stopOnEntry": false, "sourceMaps": true, diff --git a/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py b/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py index aef1bf3a557c..3a29cc2ff825 100644 --- a/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py +++ b/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py @@ -1015,7 +1015,10 @@ { "name": "test_a_function", "path": "--symlink-path-insert-here--/tests/test_a.py", - "lineno": "5", + "lineno": find_test_line_number( + "test_a_function", + os.path.join(tests_path, "test_a.py"), + ), "type_": "test", "id_": "--symlink-path-insert-here--/tests/test_a.py::test_a_function", "runID": "--symlink-path-insert-here--/tests/test_a.py::test_a_function", @@ -1031,7 +1034,10 @@ { "name": "test_b_function", "path": "--symlink-path-insert-here--/tests/test_b.py", - "lineno": "5", + "lineno": find_test_line_number( + "test_b_function", + os.path.join(tests_path, "test_b.py"), + ), "type_": "test", "id_": "--symlink-path-insert-here--/tests/test_b.py::test_b_function", "runID": "--symlink-path-insert-here--/tests/test_b.py::test_b_function", diff --git a/pythonFiles/vscode_pytest/__init__.py b/pythonFiles/vscode_pytest/__init__.py index 868ff5b87e19..792da6c7eb40 100644 --- a/pythonFiles/vscode_pytest/__init__.py +++ b/pythonFiles/vscode_pytest/__init__.py @@ -80,7 +80,6 @@ def pytest_load_initial_conftests(early_config, parser, args): for arg in args: if "--rootdir=" in arg: rootdir = arg.split("--rootdir=")[1] - print("--rootdir= argument found:", rootdir) if not os.path.exists(rootdir): raise VSCodePytestError( f"The path set in the argument --rootdir={rootdir} does not exist." diff --git a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts index c48ac9488936..ab44c96821e5 100644 --- a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts +++ b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts @@ -11,7 +11,7 @@ import { import { IConfigurationService, ITestOutputChannel } from '../../../common/types'; import { Deferred, createDeferred } from '../../../common/utils/async'; import { EXTENSION_ROOT_DIR } from '../../../constants'; -import { traceError, traceInfo, traceVerbose } from '../../../logging'; +import { traceError, traceInfo, traceVerbose, traceWarn } from '../../../logging'; import { DataReceivedEvent, DiscoveredTestPayload, @@ -71,16 +71,15 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { const fullPluginPath = path.join(EXTENSION_ROOT_DIR, relativePathToPytest); const settings = this.configSettings.getSettings(uri); let pytestArgsMap = argsToMap(settings.testing.pytestArgs); - console.log(`pytestArgsMap: ${JSON.stringify(pytestArgsMap)}`); 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()) { - console.log('The path is a symbolic link.'); + traceWarn( + "The cwd is a symbolic link, adding '--rootdir' to pytestArgsMap only if it doesn't already exist.", + ); pytestArgsMap = addArgIfNotExist(pytestArgsMap, '--rootdir', cwd); - } else { - console.log('The path is not a symbolic link.'); } // get and edit env vars diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index 3709c2568a20..45253f5ff8fc 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -97,99 +97,99 @@ suite('End to End Tests: test adapters', () => { teardown(async () => { pythonTestServer.dispose(); }); - // test('unittest discovery adapter small workspace', async () => { - // // result resolver and saved data for assertions - // let actualData: { - // cwd: string; - // tests?: unknown; - // status: 'success' | 'error'; - // error?: string[]; - // }; - // workspaceUri = Uri.parse(rootPathSmallWorkspace); - // resultResolver = new PythonResultResolver(testController, unittestProvider, workspaceUri); - // let callCount = 0; - // resultResolver._resolveDiscovery = async (payload, _token?) => { - // traceLog(`resolveDiscovery ${payload}`); - // callCount = callCount + 1; - // actualData = payload; - // return Promise.resolve(); - // }; - - // // set workspace to test workspace folder and set up settings - - // configService.getSettings(workspaceUri).testing.unittestArgs = ['-s', '.', '-p', '*test*.py']; - - // // run unittest discovery - // const discoveryAdapter = new UnittestTestDiscoveryAdapter( - // pythonTestServer, - // configService, - // testOutputChannel.object, - // resultResolver, - // envVarsService, - // ); - - // await discoveryAdapter.discoverTests(workspaceUri).finally(() => { - // // verification after discovery is complete - - // // 1. Check the status is "success" - // assert.strictEqual( - // actualData.status, - // 'success', - // `Expected status to be 'success' instead status is ${actualData.status}`, - // ); - // // 2. Confirm no errors - // assert.strictEqual(actualData.error, undefined, "Expected no errors in 'error' field"); - // // 3. Confirm tests are found - // assert.ok(actualData.tests, 'Expected tests to be present'); - - // assert.strictEqual(callCount, 1, 'Expected _resolveDiscovery to be called once'); - // }); - // }); - - // test('unittest discovery adapter large workspace', async () => { - // // result resolver and saved data for assertions - // let actualData: { - // cwd: string; - // tests?: unknown; - // status: 'success' | 'error'; - // error?: string[]; - // }; - // resultResolver = new PythonResultResolver(testController, unittestProvider, workspaceUri); - // let callCount = 0; - // resultResolver._resolveDiscovery = async (payload, _token?) => { - // traceLog(`resolveDiscovery ${payload}`); - // callCount = callCount + 1; - // actualData = payload; - // return Promise.resolve(); - // }; - - // // set settings to work for the given workspace - // workspaceUri = Uri.parse(rootPathLargeWorkspace); - // configService.getSettings(workspaceUri).testing.unittestArgs = ['-s', '.', '-p', '*test*.py']; - // // run discovery - // const discoveryAdapter = new UnittestTestDiscoveryAdapter( - // pythonTestServer, - // configService, - // testOutputChannel.object, - // resultResolver, - // envVarsService, - // ); - - // await discoveryAdapter.discoverTests(workspaceUri).finally(() => { - // // 1. Check the status is "success" - // assert.strictEqual( - // actualData.status, - // 'success', - // `Expected status to be 'success' instead status is ${actualData.status}`, - // ); - // // 2. Confirm no errors - // assert.strictEqual(actualData.error, undefined, "Expected no errors in 'error' field"); - // // 3. Confirm tests are found - // assert.ok(actualData.tests, 'Expected tests to be present'); - - // assert.strictEqual(callCount, 1, 'Expected _resolveDiscovery to be called once'); - // }); - // }); + test('unittest discovery adapter small workspace', async () => { + // result resolver and saved data for assertions + let actualData: { + cwd: string; + tests?: unknown; + status: 'success' | 'error'; + error?: string[]; + }; + workspaceUri = Uri.parse(rootPathSmallWorkspace); + resultResolver = new PythonResultResolver(testController, unittestProvider, workspaceUri); + let callCount = 0; + resultResolver._resolveDiscovery = async (payload, _token?) => { + traceLog(`resolveDiscovery ${payload}`); + callCount = callCount + 1; + actualData = payload; + return Promise.resolve(); + }; + + // set workspace to test workspace folder and set up settings + + configService.getSettings(workspaceUri).testing.unittestArgs = ['-s', '.', '-p', '*test*.py']; + + // run unittest discovery + const discoveryAdapter = new UnittestTestDiscoveryAdapter( + pythonTestServer, + configService, + testOutputChannel.object, + resultResolver, + envVarsService, + ); + + await discoveryAdapter.discoverTests(workspaceUri).finally(() => { + // verification after discovery is complete + + // 1. Check the status is "success" + assert.strictEqual( + actualData.status, + 'success', + `Expected status to be 'success' instead status is ${actualData.status}`, + ); + // 2. Confirm no errors + assert.strictEqual(actualData.error, undefined, "Expected no errors in 'error' field"); + // 3. Confirm tests are found + assert.ok(actualData.tests, 'Expected tests to be present'); + + assert.strictEqual(callCount, 1, 'Expected _resolveDiscovery to be called once'); + }); + }); + + test('unittest discovery adapter large workspace', async () => { + // result resolver and saved data for assertions + let actualData: { + cwd: string; + tests?: unknown; + status: 'success' | 'error'; + error?: string[]; + }; + resultResolver = new PythonResultResolver(testController, unittestProvider, workspaceUri); + let callCount = 0; + resultResolver._resolveDiscovery = async (payload, _token?) => { + traceLog(`resolveDiscovery ${payload}`); + callCount = callCount + 1; + actualData = payload; + return Promise.resolve(); + }; + + // set settings to work for the given workspace + workspaceUri = Uri.parse(rootPathLargeWorkspace); + configService.getSettings(workspaceUri).testing.unittestArgs = ['-s', '.', '-p', '*test*.py']; + // run discovery + const discoveryAdapter = new UnittestTestDiscoveryAdapter( + pythonTestServer, + configService, + testOutputChannel.object, + resultResolver, + envVarsService, + ); + + await discoveryAdapter.discoverTests(workspaceUri).finally(() => { + // 1. Check the status is "success" + assert.strictEqual( + actualData.status, + 'success', + `Expected status to be 'success' instead status is ${actualData.status}`, + ); + // 2. Confirm no errors + assert.strictEqual(actualData.error, undefined, "Expected no errors in 'error' field"); + // 3. Confirm tests are found + assert.ok(actualData.tests, 'Expected tests to be present'); + + assert.strictEqual(callCount, 1, 'Expected _resolveDiscovery to be called once'); + }); + }); test('pytest discovery adapter small workspace', async () => { // result resolver and saved data for assertions let actualData: { From 220c8217cb1f512d0615f49c10b99b3c3e2f67d3 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Thu, 8 Feb 2024 15:11:34 -0800 Subject: [PATCH 08/31] fix failing test --- pythonFiles/tests/pytestadapter/test_discovery.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pythonFiles/tests/pytestadapter/test_discovery.py b/pythonFiles/tests/pytestadapter/test_discovery.py index e5848db5ca0b..180a3a985d82 100644 --- a/pythonFiles/tests/pytestadapter/test_discovery.py +++ b/pythonFiles/tests/pytestadapter/test_discovery.py @@ -231,7 +231,7 @@ def test_symlink_root_dir(tmp_path): actual_item = actual_list.pop(0) assert all(item in actual_item.keys() for item in ("status", "cwd", "error")) assert actual_item.get("status") == "success" - assert actual_item.get("cwd") == os.fspath(source) + assert actual_item.get("cwd") == os.fspath(destination) assert actual_item.get("tests") == expected From 58ea94dd88482252ff94d05ae6e676e17bbaead2 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Thu, 8 Feb 2024 15:14:32 -0800 Subject: [PATCH 09/31] fix import --- pythonFiles/tests/pytestadapter/test_discovery.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pythonFiles/tests/pytestadapter/test_discovery.py b/pythonFiles/tests/pytestadapter/test_discovery.py index 180a3a985d82..57a4bd8b2cb6 100644 --- a/pythonFiles/tests/pytestadapter/test_discovery.py +++ b/pythonFiles/tests/pytestadapter/test_discovery.py @@ -3,7 +3,6 @@ import json import os import shutil -import string from typing import Any, Dict, List, Optional import pytest From 53347f9ddbbcdc19624891570ea90a3e844cf7e5 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Fri, 9 Feb 2024 09:46:27 -0800 Subject: [PATCH 10/31] switch to using tmpdir --- pythonFiles/tests/pytestadapter/test_discovery.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pythonFiles/tests/pytestadapter/test_discovery.py b/pythonFiles/tests/pytestadapter/test_discovery.py index ccc31cda6c6b..9784ca28fd04 100644 --- a/pythonFiles/tests/pytestadapter/test_discovery.py +++ b/pythonFiles/tests/pytestadapter/test_discovery.py @@ -206,7 +206,7 @@ def test_pytest_collect(file, expected_const): assert is_same_tree(actual_item.get("tests"), expected_const) -def test_symlink_root_dir(tmp_path): +def test_symlink_root_dir(tmpdir): """ Test to test pytest discovery with the command line arg --rootdir specified as a symlink path. Discovery should succeed and testids should be relative to the symlinked root directory. @@ -215,7 +215,7 @@ def test_symlink_root_dir(tmp_path): """ # create symlink source = TEST_DATA_PATH / "root" - destination = tmp_path / "symlink_root" + destination = tmpdir.mkdir("sub") / "symlink_root" os.symlink(source, destination) "--symlink-path-insert-here--" From f71ab3a78f4cc8a005cc19fdd7cdf0b6541260a2 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Fri, 9 Feb 2024 10:04:10 -0800 Subject: [PATCH 11/31] fix paths --- pythonFiles/tests/pytestadapter/test_discovery.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pythonFiles/tests/pytestadapter/test_discovery.py b/pythonFiles/tests/pytestadapter/test_discovery.py index 9784ca28fd04..4e8eef16d89a 100644 --- a/pythonFiles/tests/pytestadapter/test_discovery.py +++ b/pythonFiles/tests/pytestadapter/test_discovery.py @@ -215,19 +215,23 @@ def test_symlink_root_dir(tmpdir): """ # create symlink source = TEST_DATA_PATH / "root" - destination = tmpdir.mkdir("sub") / "symlink_root" + # destination is a path type + sub_dir = tmpdir.mkdir("sub") + destination = sub_dir / "symlink_root" os.symlink(source, destination) "--symlink-path-insert-here--" # Run pytest with the cwd being the resolved symlink path (as it will be when we run the subprocess from node). - actual = runner_with_cwd(["--collect-only", f"--rootdir={destination}"], source) + actual = runner_with_cwd( + ["--collect-only", f"--rootdir={os.fspath(destination)}"], source + ) # now assert the output is the same, but sub in the tmp_path for a placeholder in the expected output object. json_expected = json.dumps( expected_discovery_test_output.symlink_expected_discovery_output ) json_expected_new = json_expected.replace( - "--symlink-path-insert-here--", str(destination) + "--symlink-path-insert-here--", os.fspath(destination) ) expected = json.loads(json_expected_new) assert actual From 35098cea139c58142a694ead6b8269d72ca8c6d1 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Fri, 9 Feb 2024 10:13:46 -0800 Subject: [PATCH 12/31] add 3rd party notice --- ThirdPartyNotices-Repository.txt | 1 + .../tests/pytestadapter/test_discovery.py | 20 ++++++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/ThirdPartyNotices-Repository.txt b/ThirdPartyNotices-Repository.txt index 9e7e822af1bb..cfad56ee0a94 100644 --- a/ThirdPartyNotices-Repository.txt +++ b/ThirdPartyNotices-Repository.txt @@ -17,6 +17,7 @@ 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. Test controller arg mapping utils functions (https://github.com/microsoft/vscode-js-debug) %% Go for Visual Studio Code NOTICES, INFORMATION, AND LICENSE BEGIN HERE diff --git a/pythonFiles/tests/pytestadapter/test_discovery.py b/pythonFiles/tests/pytestadapter/test_discovery.py index 4e8eef16d89a..0e0f799ff589 100644 --- a/pythonFiles/tests/pytestadapter/test_discovery.py +++ b/pythonFiles/tests/pytestadapter/test_discovery.py @@ -206,7 +206,7 @@ def test_pytest_collect(file, expected_const): assert is_same_tree(actual_item.get("tests"), expected_const) -def test_symlink_root_dir(tmpdir): +def test_symlink_root_dir(tmp_path): """ Test to test pytest discovery with the command line arg --rootdir specified as a symlink path. Discovery should succeed and testids should be relative to the symlinked root directory. @@ -214,11 +214,21 @@ def test_symlink_root_dir(tmpdir): tmp_path -- pytest fixture that creates a temporary directory. """ # create symlink + print("tmp_path: ", tmp_path) source = TEST_DATA_PATH / "root" - # destination is a path type - sub_dir = tmpdir.mkdir("sub") - destination = sub_dir / "symlink_root" - os.symlink(source, destination) + + # Create a destination path for the symlink within the tmp_path directory + destination = tmp_path / "symlink_root" + print(f"destination: {destination}") + + # Create the symlink at the destination pointing to the source + destination.symlink_to( + source, target_is_directory=True + ) # Make sure to specify target_is_directory=True + + print("made symlink") + assert destination.is_symlink() + # os.symlink(source, destination) "--symlink-path-insert-here--" # Run pytest with the cwd being the resolved symlink path (as it will be when we run the subprocess from node). From c5d5e6b3916f4c57275ab9b68edd53dc66808814 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Fri, 9 Feb 2024 10:19:56 -0800 Subject: [PATCH 13/31] add better assertion error handling --- .../tests/pytestadapter/test_discovery.py | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/pythonFiles/tests/pytestadapter/test_discovery.py b/pythonFiles/tests/pytestadapter/test_discovery.py index 0e0f799ff589..dbced9d49d24 100644 --- a/pythonFiles/tests/pytestadapter/test_discovery.py +++ b/pythonFiles/tests/pytestadapter/test_discovery.py @@ -249,10 +249,22 @@ def test_symlink_root_dir(tmp_path): if actual_list is not None: assert actual_list.pop(-1).get("eot") actual_item = actual_list.pop(0) - assert all(item in actual_item.keys() for item in ("status", "cwd", "error")) - assert actual_item.get("status") == "success" - assert actual_item.get("cwd") == os.fspath(destination) - assert actual_item.get("tests") == expected + try: + # Check if all requirements + assert all( + item in actual_item.keys() for item in ("status", "cwd", "error") + ), "Required keys are missing" + assert actual_item.get("status") == "success", "Status is not 'success'" + assert actual_item.get("cwd") == os.fspath( + destination + ), f"CWD does not match: {os.fspath(destination)}" + assert ( + actual_item.get("tests") == expected + ), "Tests do not match expected value" + except AssertionError as e: + # Print the actual_item in JSON format if an assertion fails + print(json.dumps(actual_item, indent=4)) + pytest.fail(str(e)) def test_pytest_root_dir(): From 978ff58abc6613f136a3c62c79e48534240c5852 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Fri, 9 Feb 2024 10:33:15 -0800 Subject: [PATCH 14/31] logging --- pythonFiles/vscode_pytest/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pythonFiles/vscode_pytest/__init__.py b/pythonFiles/vscode_pytest/__init__.py index 792da6c7eb40..3daa49c63b38 100644 --- a/pythonFiles/vscode_pytest/__init__.py +++ b/pythonFiles/vscode_pytest/__init__.py @@ -365,6 +365,7 @@ def pytest_sessionfinish(session, exitstatus): } post_response(os.fsdecode(cwd), errorNode) try: + print("building test tree") session_node: Union[TestNode, None] = build_test_tree(session) if not session_node: raise VSCodePytestError( @@ -419,6 +420,7 @@ def build_test_tree(session: pytest.Session) -> TestNode: # Check to see if the global variable for symlink path is set global SYMLINK_PATH if SYMLINK_PATH: + print("local 3: symlink path and a session node") session_node["path"] = SYMLINK_PATH session_node["id_"] = os.fspath(SYMLINK_PATH) @@ -693,6 +695,7 @@ def get_node_path(node: Any) -> pathlib.Path: global SYMLINK_PATH # Check for the session node since it has the symlink already. if SYMLINK_PATH and not isinstance(node, pytest.Session): + print("local 2: symlink path and is not a session node") # Get relative between the cwd (resolved path) and the node path. try: rel_path = os.path.relpath(path, pathlib.Path.cwd()) From 1a6c662e619474ade30ad7fc3ee67578f703806b Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Fri, 9 Feb 2024 10:40:59 -0800 Subject: [PATCH 15/31] logging --- pythonFiles/vscode_pytest/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pythonFiles/vscode_pytest/__init__.py b/pythonFiles/vscode_pytest/__init__.py index 3daa49c63b38..7356205d4900 100644 --- a/pythonFiles/vscode_pytest/__init__.py +++ b/pythonFiles/vscode_pytest/__init__.py @@ -367,6 +367,7 @@ def pytest_sessionfinish(session, exitstatus): try: print("building test tree") session_node: Union[TestNode, None] = build_test_tree(session) + print("done building test tree") if not session_node: raise VSCodePytestError( "Something went wrong following pytest finish, \ @@ -420,10 +421,12 @@ def build_test_tree(session: pytest.Session) -> TestNode: # Check to see if the global variable for symlink path is set global SYMLINK_PATH if SYMLINK_PATH: - print("local 3: symlink path and a session node") + print("local 3: symlink path and a session node", SYMLINK_PATH) + print("as os path", os.fspath(SYMLINK_PATH)) session_node["path"] = SYMLINK_PATH session_node["id_"] = os.fspath(SYMLINK_PATH) + print("after local 3") for test_case in session.items: test_node = create_test_node(test_case) if isinstance(test_case.parent, pytest.Class): From 5e49484ef2c32c76fde8a41b3acfdff1d14c621b Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Fri, 9 Feb 2024 10:47:47 -0800 Subject: [PATCH 16/31] printing for permissions error --- pythonFiles/vscode_pytest/__init__.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pythonFiles/vscode_pytest/__init__.py b/pythonFiles/vscode_pytest/__init__.py index 7356205d4900..58032b86bb20 100644 --- a/pythonFiles/vscode_pytest/__init__.py +++ b/pythonFiles/vscode_pytest/__init__.py @@ -373,6 +373,7 @@ def pytest_sessionfinish(session, exitstatus): "Something went wrong following pytest finish, \ no session node was created" ) + print("posting response local 4") post_response(os.fsdecode(cwd), session_node) except Exception as e: ERRORS.append( @@ -428,6 +429,7 @@ def build_test_tree(session: pytest.Session) -> TestNode: print("after local 3") for test_case in session.items: + print("test case", test_case) test_node = create_test_node(test_case) if isinstance(test_case.parent, pytest.Class): case_iter = test_case.parent @@ -696,6 +698,7 @@ def get_node_path(node: Any) -> pathlib.Path: ) global SYMLINK_PATH + print("local 1: symlink path", SYMLINK_PATH) # Check for the session node since it has the symlink already. if SYMLINK_PATH and not isinstance(node, pytest.Session): print("local 2: symlink path and is not a session node") @@ -745,7 +748,7 @@ def post_response(cwd: str, session_node: TestNode) -> None: cwd (str): Current working directory. session_node (TestNode): Node information of the test session. """ - + print("post response") payload: DiscoveryPayloadDict = { "cwd": cwd, "status": "success" if not ERRORS else "error", @@ -754,6 +757,7 @@ def post_response(cwd: str, session_node: TestNode) -> None: } if ERRORS is not None: payload["error"] = ERRORS + print("sending post request") send_post_request(payload, cls_encoder=PathEncoder) @@ -808,6 +812,7 @@ def send_post_request( __socket = None raise VSCodePytestError(error_msg) + print("dump payload") data = json.dumps(payload, cls=cls_encoder) request = f"""Content-Length: {len(data)} Content-Type: application/json From 9fac03045dcd5915b6898db9ba21e14ec5753207 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Fri, 9 Feb 2024 10:53:09 -0800 Subject: [PATCH 17/31] make symlink destination --- pythonFiles/tests/pytestadapter/test_discovery.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pythonFiles/tests/pytestadapter/test_discovery.py b/pythonFiles/tests/pytestadapter/test_discovery.py index dbced9d49d24..1ea023d0611c 100644 --- a/pythonFiles/tests/pytestadapter/test_discovery.py +++ b/pythonFiles/tests/pytestadapter/test_discovery.py @@ -219,6 +219,7 @@ def test_symlink_root_dir(tmp_path): # Create a destination path for the symlink within the tmp_path directory destination = tmp_path / "symlink_root" + destination.mkdir() print(f"destination: {destination}") # Create the symlink at the destination pointing to the source From 717e2699d6a63a35761404fdc9a61a4490f7bca2 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 12 Feb 2024 08:51:43 -0800 Subject: [PATCH 18/31] revert mkdir --- pythonFiles/tests/pytestadapter/test_discovery.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pythonFiles/tests/pytestadapter/test_discovery.py b/pythonFiles/tests/pytestadapter/test_discovery.py index 1ea023d0611c..dbced9d49d24 100644 --- a/pythonFiles/tests/pytestadapter/test_discovery.py +++ b/pythonFiles/tests/pytestadapter/test_discovery.py @@ -219,7 +219,6 @@ def test_symlink_root_dir(tmp_path): # Create a destination path for the symlink within the tmp_path directory destination = tmp_path / "symlink_root" - destination.mkdir() print(f"destination: {destination}") # Create the symlink at the destination pointing to the source From 12555b215b1fb42dbe0770bc186a5e1c38741899 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Wed, 14 Feb 2024 08:18:41 -0800 Subject: [PATCH 19/31] fix to actual symlink folder --- .../tests/pytestadapter/.data/symlink_folder | 1 + .../expected_discovery_test_output.py | 2 +- pythonFiles/tests/pytestadapter/test_discovery.py | 14 ++------------ 3 files changed, 4 insertions(+), 13 deletions(-) create mode 120000 pythonFiles/tests/pytestadapter/.data/symlink_folder diff --git a/pythonFiles/tests/pytestadapter/.data/symlink_folder b/pythonFiles/tests/pytestadapter/.data/symlink_folder new file mode 120000 index 000000000000..0a99a4a6a2fb --- /dev/null +++ b/pythonFiles/tests/pytestadapter/.data/symlink_folder @@ -0,0 +1 @@ +/Users/eleanorboyd/vscode-python/pythonFiles/tests/pytestadapter/.data/root \ No newline at end of file diff --git a/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py b/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py index 51bd9780aeea..66a36970ad08 100644 --- a/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py +++ b/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py @@ -996,7 +996,7 @@ } symlink_expected_discovery_output = { - "name": "symlink_root", + "name": "symlink_folder", "path": "--symlink-path-insert-here--", "type_": "folder", "children": [ diff --git a/pythonFiles/tests/pytestadapter/test_discovery.py b/pythonFiles/tests/pytestadapter/test_discovery.py index dbced9d49d24..53663302f743 100644 --- a/pythonFiles/tests/pytestadapter/test_discovery.py +++ b/pythonFiles/tests/pytestadapter/test_discovery.py @@ -206,27 +206,17 @@ def test_pytest_collect(file, expected_const): assert is_same_tree(actual_item.get("tests"), expected_const) -def test_symlink_root_dir(tmp_path): +def test_symlink_root_dir(): """ Test to test pytest discovery with the command line arg --rootdir specified as a symlink path. Discovery should succeed and testids should be relative to the symlinked root directory. - Keyword arguments: - tmp_path -- pytest fixture that creates a temporary directory. """ # create symlink - print("tmp_path: ", tmp_path) source = TEST_DATA_PATH / "root" # Create a destination path for the symlink within the tmp_path directory - destination = tmp_path / "symlink_root" + destination = TEST_DATA_PATH / "symlink_folder" print(f"destination: {destination}") - - # Create the symlink at the destination pointing to the source - destination.symlink_to( - source, target_is_directory=True - ) # Make sure to specify target_is_directory=True - - print("made symlink") assert destination.is_symlink() # os.symlink(source, destination) From 03cd94f068eefdb34554ab29ffaf499e69603bc9 Mon Sep 17 00:00:00 2001 From: Eleanor Boyd Date: Wed, 14 Feb 2024 09:28:03 -0800 Subject: [PATCH 20/31] Update ThirdPartyNotices-Repository.txt Co-authored-by: Karthik Nadig --- ThirdPartyNotices-Repository.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ThirdPartyNotices-Repository.txt b/ThirdPartyNotices-Repository.txt index cfad56ee0a94..5e943d5c42ab 100644 --- a/ThirdPartyNotices-Repository.txt +++ b/ThirdPartyNotices-Repository.txt @@ -17,7 +17,7 @@ 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. Test controller arg mapping utils functions (https://github.com/microsoft/vscode-js-debug) +14. vscode-js-debug (https://github.com/microsoft/vscode-js-debug) %% Go for Visual Studio Code NOTICES, INFORMATION, AND LICENSE BEGIN HERE From 21858bca8c72fdda7a49855437217a66bb04b149 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Wed, 14 Feb 2024 09:37:25 -0800 Subject: [PATCH 21/31] add license --- ThirdPartyNotices-Repository.txt | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/ThirdPartyNotices-Repository.txt b/ThirdPartyNotices-Repository.txt index 5e943d5c42ab..bbb00d523f91 100644 --- a/ThirdPartyNotices-Repository.txt +++ b/ThirdPartyNotices-Repository.txt @@ -1033,3 +1033,31 @@ 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 From 82341dcdd7264f363c47de9c68b9cb9817ab4c3e Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Wed, 14 Feb 2024 09:43:49 -0800 Subject: [PATCH 22/31] fix pyright --- pythonFiles/vscode_pytest/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pythonFiles/vscode_pytest/__init__.py b/pythonFiles/vscode_pytest/__init__.py index 58032b86bb20..1d6ef67eb116 100644 --- a/pythonFiles/vscode_pytest/__init__.py +++ b/pythonFiles/vscode_pytest/__init__.py @@ -710,8 +710,8 @@ def get_node_path(node: Any) -> pathlib.Path: return sym_path except Exception as e: raise VSCodePytestError( - f"Error occurred while calculating symlink equivalent from node path: {e} \n", - "SYMLINK_PATH: {SYMLINK_PATH}, \n node path: {path}, \n cwd: {{pathlib.Path.cwd()}}", + f"Error occurred while calculating symlink equivalent from node path: {e}" + "\n SYMLINK_PATH: {SYMLINK_PATH}, \n node path: {path}, \n cwd: {{pathlib.Path.cwd()}}" ) return path From add10d93d8d08987879fd163503edef469edd08a Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Wed, 14 Feb 2024 11:06:46 -0800 Subject: [PATCH 23/31] switching to setup and cleanup for symlink tests --- .../tests/pytestadapter/.data/symlink_folder | 1 - pythonFiles/tests/pytestadapter/helpers.py | 19 ++++ .../tests/pytestadapter/test_discovery.py | 87 +++++++++---------- .../testing/common/testingAdapter.test.ts | 44 +++++++--- .../symlinkWorkspace/smallWorkspace | 1 - 5 files changed, 94 insertions(+), 58 deletions(-) delete mode 120000 pythonFiles/tests/pytestadapter/.data/symlink_folder delete mode 120000 src/testTestingRootWkspc/symlinkWorkspace/smallWorkspace diff --git a/pythonFiles/tests/pytestadapter/.data/symlink_folder b/pythonFiles/tests/pytestadapter/.data/symlink_folder deleted file mode 120000 index 0a99a4a6a2fb..000000000000 --- a/pythonFiles/tests/pytestadapter/.data/symlink_folder +++ /dev/null @@ -1 +0,0 @@ -/Users/eleanorboyd/vscode-python/pythonFiles/tests/pytestadapter/.data/root \ No newline at end of file diff --git a/pythonFiles/tests/pytestadapter/helpers.py b/pythonFiles/tests/pytestadapter/helpers.py index 2d36da59956b..c568d590032c 100644 --- a/pythonFiles/tests/pytestadapter/helpers.py +++ b/pythonFiles/tests/pytestadapter/helpers.py @@ -1,10 +1,12 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +import contextlib import io import json import os import pathlib +import random import socket import subprocess import sys @@ -27,6 +29,23 @@ def get_absolute_test_id(test_id: str, testPath: pathlib.Path) -> str: return absolute_test_id +@contextlib.contextmanager +def create_symlink(root: pathlib.Path, target_ext: str, destination_ext: str): + try: + destination = root / destination_ext + target = root / target_ext + if destination.exists(): + print("destination already exists", destination) + try: + destination.symlink_to(target) + except Exception as e: + print("error occurred", e) + yield target, destination + finally: + destination.unlink() + print("destination unlinked", destination) + + def create_server( host: str = "127.0.0.1", port: int = 0, diff --git a/pythonFiles/tests/pytestadapter/test_discovery.py b/pythonFiles/tests/pytestadapter/test_discovery.py index 53663302f743..90208d9d4824 100644 --- a/pythonFiles/tests/pytestadapter/test_discovery.py +++ b/pythonFiles/tests/pytestadapter/test_discovery.py @@ -15,7 +15,7 @@ from tests.tree_comparison_helper import is_same_tree from . import expected_discovery_test_output -from .helpers import TEST_DATA_PATH, runner, runner_with_cwd +from .helpers import TEST_DATA_PATH, runner, runner_with_cwd, create_symlink def test_import_error(tmp_path): @@ -211,50 +211,49 @@ def test_symlink_root_dir(): Test to test pytest discovery with the command line arg --rootdir specified as a symlink path. Discovery should succeed and testids should be relative to the symlinked root directory. """ - # create symlink - source = TEST_DATA_PATH / "root" - - # Create a destination path for the symlink within the tmp_path directory - destination = TEST_DATA_PATH / "symlink_folder" - print(f"destination: {destination}") - assert destination.is_symlink() - # os.symlink(source, destination) - - "--symlink-path-insert-here--" - # Run pytest with the cwd being the resolved symlink path (as it will be when we run the subprocess from node). - actual = runner_with_cwd( - ["--collect-only", f"--rootdir={os.fspath(destination)}"], source - ) + with create_symlink(TEST_DATA_PATH, "root", "symlink_folder") as ( + source, + destination, + ): + print(f"symlink destination: {destination}") + print(f"symlink target: {source}") + assert destination.is_symlink() + + "--symlink-path-insert-here--" + # Run pytest with the cwd being the resolved symlink path (as it will be when we run the subprocess from node). + actual = runner_with_cwd( + ["--collect-only", f"--rootdir={os.fspath(destination)}"], source + ) - # now assert the output is the same, but sub in the tmp_path for a placeholder in the expected output object. - json_expected = json.dumps( - expected_discovery_test_output.symlink_expected_discovery_output - ) - json_expected_new = json_expected.replace( - "--symlink-path-insert-here--", os.fspath(destination) - ) - expected = json.loads(json_expected_new) - assert actual - actual_list: List[Dict[str, Any]] = actual - if actual_list is not None: - assert actual_list.pop(-1).get("eot") - actual_item = actual_list.pop(0) - try: - # Check if all requirements - assert all( - item in actual_item.keys() for item in ("status", "cwd", "error") - ), "Required keys are missing" - assert actual_item.get("status") == "success", "Status is not 'success'" - assert actual_item.get("cwd") == os.fspath( - destination - ), f"CWD does not match: {os.fspath(destination)}" - assert ( - actual_item.get("tests") == expected - ), "Tests do not match expected value" - except AssertionError as e: - # Print the actual_item in JSON format if an assertion fails - print(json.dumps(actual_item, indent=4)) - pytest.fail(str(e)) + # now assert the output is the same, but sub in the tmp_path for a placeholder in the expected output object. + json_expected = json.dumps( + expected_discovery_test_output.symlink_expected_discovery_output + ) + json_expected_new = json_expected.replace( + "--symlink-path-insert-here--", os.fspath(destination) + ) + expected = json.loads(json_expected_new) + assert actual + actual_list: List[Dict[str, Any]] = actual + if actual_list is not None: + assert actual_list.pop(-1).get("eot") + actual_item = actual_list.pop(0) + try: + # Check if all requirements + assert all( + item in actual_item.keys() for item in ("status", "cwd", "error") + ), "Required keys are missing" + assert actual_item.get("status") == "success", "Status is not 'success'" + assert actual_item.get("cwd") == os.fspath( + destination + ), f"CWD does not match: {os.fspath(destination)}" + assert ( + actual_item.get("tests") == expected + ), "Tests do not match expected value" + except AssertionError as e: + # Print the actual_item in JSON format if an assertion fails + print(json.dumps(actual_item, indent=4)) + pytest.fail(str(e)) def test_pytest_root_dir(): diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index 45253f5ff8fc..47ac813f7dd7 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -60,8 +60,25 @@ suite('End to End Tests: test adapters', () => { 'testTestingRootWkspc', 'discoveryErrorWorkspace', ); + const rootPathDiscoverySymlink = path.join( + EXTENSION_ROOT_DIR_FOR_TESTS, + 'src', + 'testTestingRootWkspc', + 'symlinkWorkspace', + ); suiteSetup(async () => { serviceContainer = (await initialize()).serviceContainer; + + // 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.'); + } + }); }); setup(async () => { @@ -97,6 +114,17 @@ suite('End to End Tests: test adapters', () => { teardown(async () => { pythonTestServer.dispose(); }); + suiteTeardown(async () => { + // remove symlink + const dest = rootPathDiscoverySymlink; + fs.unlink(dest, (err) => { + if (err) { + console.error(err); + } else { + console.log('Symlink removed successfully after tests.'); + } + }); + }); test('unittest discovery adapter small workspace', async () => { // result resolver and saved data for assertions let actualData: { @@ -242,17 +270,9 @@ suite('End to End Tests: test adapters', () => { error?: string[]; }; // set workspace to test workspace folder - // const target = rootPathSmallWorkspace; - const rootPathSmallWorkspaceSymlink = path.join( - EXTENSION_ROOT_DIR_FOR_TESTS, - 'src', - 'testTestingRootWkspc', - 'symlinkWorkspace', - 'smallWorkspace', - ); - const testSimpleSymlinkPath = path.join(rootPathSmallWorkspaceSymlink, 'test_simple.py'); - workspaceUri = Uri.parse(rootPathSmallWorkspaceSymlink); - const stats = fs.lstatSync(rootPathSmallWorkspaceSymlink); + const testSimpleSymlinkPath = path.join(rootPathDiscoverySymlink, 'test_simple.py'); + workspaceUri = Uri.parse(rootPathDiscoverySymlink); + const stats = fs.lstatSync(rootPathDiscoverySymlink); // confirm that the path is a symbolic link assert.ok(stats.isSymbolicLink(), 'The path is not a symbolic link but must be for this test.'); @@ -287,7 +307,7 @@ suite('End to End Tests: test adapters', () => { // 3. Confirm tests are found assert.ok(actualData.tests, 'Expected tests to be present'); // 4. Confirm that the cwd returned is the symlink path - assert.strictEqual(actualData.cwd, rootPathSmallWorkspaceSymlink, 'Expected cwd to be the symlink path'); + assert.strictEqual(actualData.cwd, rootPathDiscoverySymlink, 'Expected cwd to be the symlink path'); // 5. Confirm that the test's path is also using the symlink as the root assert.strictEqual( (actualData.tests as { children: { path: string }[] }).children[0].path, diff --git a/src/testTestingRootWkspc/symlinkWorkspace/smallWorkspace b/src/testTestingRootWkspc/symlinkWorkspace/smallWorkspace deleted file mode 120000 index f5736036f95e..000000000000 --- a/src/testTestingRootWkspc/symlinkWorkspace/smallWorkspace +++ /dev/null @@ -1 +0,0 @@ -/Users/eleanorboyd/vscode-python/src/testTestingRootWkspc/smallWorkspace \ No newline at end of file From 76f292d22b25cc55fdde605d4015ca042cd6c82e Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Wed, 14 Feb 2024 11:10:02 -0800 Subject: [PATCH 24/31] fix import --- pythonFiles/tests/pytestadapter/helpers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pythonFiles/tests/pytestadapter/helpers.py b/pythonFiles/tests/pytestadapter/helpers.py index c568d590032c..6b96eb186d37 100644 --- a/pythonFiles/tests/pytestadapter/helpers.py +++ b/pythonFiles/tests/pytestadapter/helpers.py @@ -6,7 +6,6 @@ import json import os import pathlib -import random import socket import subprocess import sys From 89a4713ec479c4a87e18af4f9c77a851f8716dda Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Wed, 14 Feb 2024 11:22:02 -0800 Subject: [PATCH 25/31] add error msg --- pythonFiles/tests/pytestadapter/test_discovery.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pythonFiles/tests/pytestadapter/test_discovery.py b/pythonFiles/tests/pytestadapter/test_discovery.py index 90208d9d4824..f5431f6022ce 100644 --- a/pythonFiles/tests/pytestadapter/test_discovery.py +++ b/pythonFiles/tests/pytestadapter/test_discovery.py @@ -232,7 +232,11 @@ def test_symlink_root_dir(): json_expected_new = json_expected.replace( "--symlink-path-insert-here--", os.fspath(destination) ) - expected = json.loads(json_expected_new) + try: + expected = json.loads(json_expected_new) + except json.JSONDecodeError: + print(f"Output which could not be parsed as JSON: {json_expected_new}") + pytest.fail("Expected output is not valid JSON") assert actual actual_list: List[Dict[str, Any]] = actual if actual_list is not None: From 6b55a2ced85b0889eb3662106b0ab018a713cb23 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Wed, 14 Feb 2024 11:33:24 -0800 Subject: [PATCH 26/31] fixing issue with path creation for expected output constant --- .../expected_discovery_test_output.py | 48 +++++++++++++------ .../tests/pytestadapter/test_discovery.py | 15 +----- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py b/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py index 66a36970ad08..7fbb0c5c43e7 100644 --- a/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py +++ b/pythonFiles/tests/pytestadapter/expected_discovery_test_output.py @@ -994,58 +994,78 @@ ], "id_": str(TEST_DATA_PATH), } +SYMLINK_FOLDER_PATH = TEST_DATA_PATH / "symlink_folder" +SYMLINK_FOLDER_PATH_TESTS = TEST_DATA_PATH / "symlink_folder" / "tests" +SYMLINK_FOLDER_PATH_TESTS_TEST_A = ( + TEST_DATA_PATH / "symlink_folder" / "tests" / "test_a.py" +) +SYMLINK_FOLDER_PATH_TESTS_TEST_B = ( + TEST_DATA_PATH / "symlink_folder" / "tests" / "test_b.py" +) symlink_expected_discovery_output = { "name": "symlink_folder", - "path": "--symlink-path-insert-here--", + "path": str(SYMLINK_FOLDER_PATH), "type_": "folder", "children": [ { "name": "tests", - "path": "--symlink-path-insert-here--/tests", + "path": str(SYMLINK_FOLDER_PATH_TESTS), "type_": "folder", - "id_": "--symlink-path-insert-here--/tests", + "id_": str(SYMLINK_FOLDER_PATH_TESTS), "children": [ { "name": "test_a.py", - "path": "--symlink-path-insert-here--/tests/test_a.py", + "path": str(SYMLINK_FOLDER_PATH_TESTS_TEST_A), "type_": "file", - "id_": "--symlink-path-insert-here--/tests/test_a.py", + "id_": str(SYMLINK_FOLDER_PATH_TESTS_TEST_A), "children": [ { "name": "test_a_function", - "path": "--symlink-path-insert-here--/tests/test_a.py", + "path": str(SYMLINK_FOLDER_PATH_TESTS_TEST_A), "lineno": find_test_line_number( "test_a_function", os.path.join(tests_path, "test_a.py"), ), "type_": "test", - "id_": "--symlink-path-insert-here--/tests/test_a.py::test_a_function", - "runID": "--symlink-path-insert-here--/tests/test_a.py::test_a_function", + "id_": get_absolute_test_id( + "tests/test_a.py::test_a_function", + SYMLINK_FOLDER_PATH_TESTS_TEST_A, + ), + "runID": get_absolute_test_id( + "tests/test_a.py::test_a_function", + SYMLINK_FOLDER_PATH_TESTS_TEST_A, + ), } ], }, { "name": "test_b.py", - "path": "--symlink-path-insert-here--/tests/test_b.py", + "path": str(SYMLINK_FOLDER_PATH_TESTS_TEST_B), "type_": "file", - "id_": "--symlink-path-insert-here--/tests/test_b.py", + "id_": str(SYMLINK_FOLDER_PATH_TESTS_TEST_B), "children": [ { "name": "test_b_function", - "path": "--symlink-path-insert-here--/tests/test_b.py", + "path": str(SYMLINK_FOLDER_PATH_TESTS_TEST_B), "lineno": find_test_line_number( "test_b_function", os.path.join(tests_path, "test_b.py"), ), "type_": "test", - "id_": "--symlink-path-insert-here--/tests/test_b.py::test_b_function", - "runID": "--symlink-path-insert-here--/tests/test_b.py::test_b_function", + "id_": get_absolute_test_id( + "tests/test_b.py::test_b_function", + SYMLINK_FOLDER_PATH_TESTS_TEST_B, + ), + "runID": get_absolute_test_id( + "tests/test_b.py::test_b_function", + SYMLINK_FOLDER_PATH_TESTS_TEST_B, + ), } ], }, ], } ], - "id_": "--symlink-path-insert-here--", + "id_": str(SYMLINK_FOLDER_PATH), } diff --git a/pythonFiles/tests/pytestadapter/test_discovery.py b/pythonFiles/tests/pytestadapter/test_discovery.py index f5431f6022ce..93176df80ae0 100644 --- a/pythonFiles/tests/pytestadapter/test_discovery.py +++ b/pythonFiles/tests/pytestadapter/test_discovery.py @@ -219,24 +219,11 @@ def test_symlink_root_dir(): print(f"symlink target: {source}") assert destination.is_symlink() - "--symlink-path-insert-here--" # Run pytest with the cwd being the resolved symlink path (as it will be when we run the subprocess from node). actual = runner_with_cwd( ["--collect-only", f"--rootdir={os.fspath(destination)}"], source ) - - # now assert the output is the same, but sub in the tmp_path for a placeholder in the expected output object. - json_expected = json.dumps( - expected_discovery_test_output.symlink_expected_discovery_output - ) - json_expected_new = json_expected.replace( - "--symlink-path-insert-here--", os.fspath(destination) - ) - try: - expected = json.loads(json_expected_new) - except json.JSONDecodeError: - print(f"Output which could not be parsed as JSON: {json_expected_new}") - pytest.fail("Expected output is not valid JSON") + expected = expected_discovery_test_output.symlink_expected_discovery_output assert actual actual_list: List[Dict[str, Any]] = actual if actual_list is not None: From 477c41aee18326aaeb951e6479748b04df9d346a Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Wed, 14 Feb 2024 13:17:13 -0800 Subject: [PATCH 27/31] remove test printing --- pythonFiles/tests/pytestadapter/helpers.py | 2 +- pythonFiles/vscode_pytest/__init__.py | 12 ------------ src/test/testing/common/testingAdapter.test.ts | 6 +++++- 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/pythonFiles/tests/pytestadapter/helpers.py b/pythonFiles/tests/pytestadapter/helpers.py index 6b96eb186d37..a3ed21cc5538 100644 --- a/pythonFiles/tests/pytestadapter/helpers.py +++ b/pythonFiles/tests/pytestadapter/helpers.py @@ -38,7 +38,7 @@ def create_symlink(root: pathlib.Path, target_ext: str, destination_ext: str): try: destination.symlink_to(target) except Exception as e: - print("error occurred", e) + print("error occurred when attempting to create a symlink", e) yield target, destination finally: destination.unlink() diff --git a/pythonFiles/vscode_pytest/__init__.py b/pythonFiles/vscode_pytest/__init__.py index 1d6ef67eb116..4250d5359029 100644 --- a/pythonFiles/vscode_pytest/__init__.py +++ b/pythonFiles/vscode_pytest/__init__.py @@ -365,15 +365,12 @@ def pytest_sessionfinish(session, exitstatus): } post_response(os.fsdecode(cwd), errorNode) try: - print("building test tree") session_node: Union[TestNode, None] = build_test_tree(session) - print("done building test tree") if not session_node: raise VSCodePytestError( "Something went wrong following pytest finish, \ no session node was created" ) - print("posting response local 4") post_response(os.fsdecode(cwd), session_node) except Exception as e: ERRORS.append( @@ -422,14 +419,10 @@ def build_test_tree(session: pytest.Session) -> TestNode: # Check to see if the global variable for symlink path is set global SYMLINK_PATH if SYMLINK_PATH: - print("local 3: symlink path and a session node", SYMLINK_PATH) - print("as os path", os.fspath(SYMLINK_PATH)) session_node["path"] = SYMLINK_PATH session_node["id_"] = os.fspath(SYMLINK_PATH) - print("after local 3") for test_case in session.items: - print("test case", test_case) test_node = create_test_node(test_case) if isinstance(test_case.parent, pytest.Class): case_iter = test_case.parent @@ -698,10 +691,8 @@ def get_node_path(node: Any) -> pathlib.Path: ) global SYMLINK_PATH - print("local 1: symlink path", SYMLINK_PATH) # Check for the session node since it has the symlink already. if SYMLINK_PATH and not isinstance(node, pytest.Session): - print("local 2: symlink path and is not a session node") # Get relative between the cwd (resolved path) and the node path. try: rel_path = os.path.relpath(path, pathlib.Path.cwd()) @@ -748,7 +739,6 @@ def post_response(cwd: str, session_node: TestNode) -> None: cwd (str): Current working directory. session_node (TestNode): Node information of the test session. """ - print("post response") payload: DiscoveryPayloadDict = { "cwd": cwd, "status": "success" if not ERRORS else "error", @@ -757,7 +747,6 @@ def post_response(cwd: str, session_node: TestNode) -> None: } if ERRORS is not None: payload["error"] = ERRORS - print("sending post request") send_post_request(payload, cls_encoder=PathEncoder) @@ -812,7 +801,6 @@ def send_post_request( __socket = None raise VSCodePytestError(error_msg) - print("dump payload") data = json.dumps(payload, cls=cls_encoder) request = f"""Content-Length: {len(data)} Content-Type: application/json diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index 47ac813f7dd7..07d7da751603 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -307,7 +307,11 @@ suite('End to End Tests: test adapters', () => { // 3. Confirm tests are found assert.ok(actualData.tests, 'Expected tests to be present'); // 4. Confirm that the cwd returned is the symlink path - assert.strictEqual(actualData.cwd, rootPathDiscoverySymlink, 'Expected cwd to be the symlink path'); + assert.strictEqual( + path.join(actualData.cwd), + path.join(rootPathDiscoverySymlink), + 'Expected cwd to be the symlink path', + ); // 5. Confirm that the test's path is also using the symlink as the root assert.strictEqual( (actualData.tests as { children: { path: string }[] }).children[0].path, From 1348d360072f21e688231bdf244f3f0a36fbda60 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Wed, 14 Feb 2024 14:27:24 -0800 Subject: [PATCH 28/31] windows specific test --- .../testing/common/testingAdapter.test.ts | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index 07d7da751603..9588da2d2409 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -307,11 +307,21 @@ suite('End to End Tests: test adapters', () => { // 3. Confirm tests are found assert.ok(actualData.tests, 'Expected tests to be present'); // 4. Confirm that the cwd returned is the symlink path - assert.strictEqual( - path.join(actualData.cwd), - path.join(rootPathDiscoverySymlink), - 'Expected cwd to be the symlink path', - ); + if (process.platform === 'win32') { + // covert string to lowercase for windows as the path is case insensitive + assert.strictEqual( + actualData.cwd.toLowerCase(), + rootPathDiscoverySymlink.toLowerCase(), + 'Expected cwd to be the symlink path', + ); + } else { + assert.strictEqual( + path.join(actualData.cwd), + path.join(rootPathDiscoverySymlink), + 'Expected cwd to be the symlink path', + ); + } + // 5. Confirm that the test's path is also using the symlink as the root assert.strictEqual( (actualData.tests as { children: { path: string }[] }).children[0].path, From e670907bbff3ece3cf0c19f397116afeb969d27a Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Wed, 14 Feb 2024 14:49:03 -0800 Subject: [PATCH 29/31] add trace messaging --- src/test/testing/common/testingAdapter.test.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index 9588da2d2409..4e325f21b093 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -309,16 +309,15 @@ suite('End to End Tests: test adapters', () => { // 4. Confirm that the cwd returned is the symlink path if (process.platform === 'win32') { // covert string to lowercase for windows as the path is case insensitive - assert.strictEqual( - actualData.cwd.toLowerCase(), - rootPathDiscoverySymlink.toLowerCase(), - 'Expected cwd to be the symlink path', - ); + traceLog('windows machine detected, converting path to lowercase for comparison'); + const a = actualData.cwd.toLowerCase(); + const b = rootPathDiscoverySymlink.toLowerCase(); + assert.strictEqual(a, b, `Expected cwd to be the symlink path actual: ${a} expected: ${b}`); } else { assert.strictEqual( path.join(actualData.cwd), path.join(rootPathDiscoverySymlink), - 'Expected cwd to be the symlink path', + 'Expected cwd to be the symlink path, check for non-windows machines', ); } From ce21dacf2fc321de44b599f6358d3f13640cdda6 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Wed, 14 Feb 2024 15:07:33 -0800 Subject: [PATCH 30/31] fix other assert to also work with the lowercase check --- .../testing/common/testingAdapter.test.ts | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/test/testing/common/testingAdapter.test.ts b/src/test/testing/common/testingAdapter.test.ts index 4e325f21b093..8e2f6200003e 100644 --- a/src/test/testing/common/testingAdapter.test.ts +++ b/src/test/testing/common/testingAdapter.test.ts @@ -306,27 +306,41 @@ suite('End to End Tests: test adapters', () => { assert.strictEqual(actualData.error?.length, 0, "Expected no errors in 'error' field"); // 3. Confirm tests are found assert.ok(actualData.tests, 'Expected tests to be present'); - // 4. Confirm that the cwd returned is the symlink path + // 4. Confirm that the cwd returned is the symlink path and the test's path is also using the symlink as the root if (process.platform === 'win32') { // covert string to lowercase for windows as the path is case insensitive traceLog('windows machine detected, converting path to lowercase for comparison'); const a = actualData.cwd.toLowerCase(); const b = rootPathDiscoverySymlink.toLowerCase(); + const testSimpleActual = (actualData.tests as { + children: { + path: string; + }[]; + }).children[0].path.toLowerCase(); + const testSimpleExpected = testSimpleSymlinkPath.toLowerCase(); assert.strictEqual(a, b, `Expected cwd to be the symlink path actual: ${a} expected: ${b}`); + assert.strictEqual( + testSimpleActual, + testSimpleExpected, + `Expected test path to be the symlink path actual: ${testSimpleActual} expected: ${testSimpleExpected}`, + ); } else { assert.strictEqual( path.join(actualData.cwd), path.join(rootPathDiscoverySymlink), 'Expected cwd to be the symlink path, check for non-windows machines', ); + assert.strictEqual( + (actualData.tests as { + children: { + path: string; + }[]; + }).children[0].path, + testSimpleSymlinkPath, + 'Expected test path to be the symlink path, check for non windows machines', + ); } - // 5. Confirm that the test's path is also using the symlink as the root - assert.strictEqual( - (actualData.tests as { children: { path: string }[] }).children[0].path, - testSimpleSymlinkPath, - 'Expected test path to be the symlink path', - ); // 5. Confirm that resolveDiscovery was called once assert.strictEqual(callCount, 1, 'Expected _resolveDiscovery to be called once'); }); From 86099101fb6179e07eaf52fb93a0e4ef7fc717f3 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Fri, 16 Feb 2024 11:51:22 -0800 Subject: [PATCH 31/31] small edits- karthik review --- pythonFiles/tests/pytestadapter/test_discovery.py | 2 -- pythonFiles/vscode_pytest/__init__.py | 6 ++---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/pythonFiles/tests/pytestadapter/test_discovery.py b/pythonFiles/tests/pytestadapter/test_discovery.py index 93176df80ae0..b28a2b307ae2 100644 --- a/pythonFiles/tests/pytestadapter/test_discovery.py +++ b/pythonFiles/tests/pytestadapter/test_discovery.py @@ -215,8 +215,6 @@ def test_symlink_root_dir(): source, destination, ): - print(f"symlink destination: {destination}") - print(f"symlink target: {source}") assert destination.is_symlink() # Run pytest with the cwd being the resolved symlink path (as it will be when we run the subprocess from node). diff --git a/pythonFiles/vscode_pytest/__init__.py b/pythonFiles/vscode_pytest/__init__.py index 4250d5359029..256f2bfdb099 100644 --- a/pythonFiles/vscode_pytest/__init__.py +++ b/pythonFiles/vscode_pytest/__init__.py @@ -346,7 +346,6 @@ def pytest_sessionfinish(session, exitstatus): Exit code 5: No tests were collected """ cwd = pathlib.Path.cwd() - global SYMLINK_PATH if SYMLINK_PATH: print("Plugin warning[vscode-pytest]: SYMLINK set, adjusting cwd.") # Get relative between the cwd (resolved path) and the node path. @@ -417,7 +416,6 @@ def build_test_tree(session: pytest.Session) -> TestNode: function_nodes_dict: Dict[str, TestNode] = {} # Check to see if the global variable for symlink path is set - global SYMLINK_PATH if SYMLINK_PATH: session_node["path"] = SYMLINK_PATH session_node["id_"] = os.fspath(SYMLINK_PATH) @@ -690,12 +688,12 @@ def get_node_path(node: Any) -> pathlib.Path: f"Unable to find path for node: {node}, node.path: {node.path}, node.fspath: {node.fspath}" ) - global SYMLINK_PATH # Check for the session node since it has the symlink already. if SYMLINK_PATH and not isinstance(node, pytest.Session): # Get relative between the cwd (resolved path) and the node path. try: - rel_path = os.path.relpath(path, pathlib.Path.cwd()) + rel_path = path.relative_to(pathlib.Path.cwd()) + # Calculate the new node path by making it relative to the symlink path. sym_path = pathlib.Path(os.path.join(SYMLINK_PATH, rel_path)) return sym_path