Skip to content

Commit

Permalink
Fix Bug with Pytest when using symlinked workspaces (#22885)
Browse files Browse the repository at this point in the history
fixes #22658

also implements switching to arg mapping which is this issue here:
#22076

---------

Co-authored-by: Karthik Nadig <kanadig@microsoft.com>
  • Loading branch information
eleanorjboyd and karthiknadig committed Feb 20, 2024
1 parent e53651d commit 75ed73e
Show file tree
Hide file tree
Showing 12 changed files with 681 additions and 18 deletions.
29 changes: 29 additions & 0 deletions ThirdPartyNotices-Repository.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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. vscode-js-debug (https://github.com/microsoft/vscode-js-debug)

%%
Go for Visual Studio Code NOTICES, INFORMATION, AND LICENSE BEGIN HERE
Expand Down Expand Up @@ -1032,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
75 changes: 75 additions & 0 deletions pythonFiles/tests/pytestadapter/expected_discovery_test_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -994,3 +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": str(SYMLINK_FOLDER_PATH),
"type_": "folder",
"children": [
{
"name": "tests",
"path": str(SYMLINK_FOLDER_PATH_TESTS),
"type_": "folder",
"id_": str(SYMLINK_FOLDER_PATH_TESTS),
"children": [
{
"name": "test_a.py",
"path": str(SYMLINK_FOLDER_PATH_TESTS_TEST_A),
"type_": "file",
"id_": str(SYMLINK_FOLDER_PATH_TESTS_TEST_A),
"children": [
{
"name": "test_a_function",
"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_": 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": str(SYMLINK_FOLDER_PATH_TESTS_TEST_B),
"type_": "file",
"id_": str(SYMLINK_FOLDER_PATH_TESTS_TEST_B),
"children": [
{
"name": "test_b_function",
"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_": 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_": str(SYMLINK_FOLDER_PATH),
}
18 changes: 18 additions & 0 deletions pythonFiles/tests/pytestadapter/helpers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.

import contextlib
import io
import json
import os
Expand All @@ -27,6 +28,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 when attempting to create a symlink", e)
yield target, destination
finally:
destination.unlink()
print("destination unlinked", destination)


def create_server(
host: str = "127.0.0.1",
port: int = 0,
Expand Down
42 changes: 41 additions & 1 deletion pythonFiles/tests/pytestadapter/test_discovery.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.
import json
import os
import pathlib
import shutil
Expand All @@ -14,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):
Expand Down Expand Up @@ -205,6 +206,45 @@ def test_pytest_collect(file, expected_const):
assert is_same_tree(actual_item.get("tests"), expected_const)


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.
"""
with create_symlink(TEST_DATA_PATH, "root", "symlink_folder") as (
source,
destination,
):
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).
actual = runner_with_cwd(
["--collect-only", f"--rootdir={os.fspath(destination)}"], source
)
expected = expected_discovery_test_output.symlink_expected_discovery_output
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():
"""
Test to test pytest discovery with the command line arg --rootdir specified to be a subfolder
Expand Down
53 changes: 51 additions & 2 deletions pythonFiles/vscode_pytest/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -75,6 +76,25 @@ 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]
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"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
SYMLINK_PATH = pathlib.Path(rootdir)


def pytest_internalerror(excrepr, excinfo):
"""A pytest hook that is called when an internal error occurs.
Expand Down Expand Up @@ -326,6 +346,13 @@ def pytest_sessionfinish(session, exitstatus):
Exit code 5: No tests were collected
"""
cwd = pathlib.Path.cwd()
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 = {
Expand Down Expand Up @@ -388,6 +415,11 @@ 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
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):
Expand Down Expand Up @@ -645,13 +677,31 @@ 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}"
)

# 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 = 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
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


Expand Down Expand Up @@ -687,7 +737,6 @@ def post_response(cwd: str, session_node: TestNode) -> None:
cwd (str): Current working directory.
session_node (TestNode): Node information of the test session.
"""

payload: DiscoveryPayloadDict = {
"cwd": cwd,
"status": "success" if not ERRORS else "error",
Expand Down
80 changes: 80 additions & 0 deletions src/client/testing/testController/common/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>): { [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;
}
Loading

0 comments on commit 75ed73e

Please sign in to comment.