Skip to content

Commit 75ed73e

Browse files
Fix Bug with Pytest when using symlinked workspaces (microsoft#22885)
fixes microsoft#22658 also implements switching to arg mapping which is this issue here: microsoft#22076 --------- Co-authored-by: Karthik Nadig <[email protected]>
1 parent e53651d commit 75ed73e

File tree

12 files changed

+681
-18
lines changed

12 files changed

+681
-18
lines changed

ThirdPartyNotices-Repository.txt

+29
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ Microsoft Python extension for Visual Studio Code incorporates third party mater
1717
11. vscode-cpptools (https://github.com/microsoft/vscode-cpptools)
1818
12. mocha (https://github.com/mochajs/mocha)
1919
13. get-pip (https://github.com/pypa/get-pip)
20+
14. vscode-js-debug (https://github.com/microsoft/vscode-js-debug)
2021

2122
%%
2223
Go for Visual Studio Code NOTICES, INFORMATION, AND LICENSE BEGIN HERE
@@ -1032,3 +1033,31 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
10321033

10331034
=========================================
10341035
END OF get-pip NOTICES, INFORMATION, AND LICENSE
1036+
1037+
1038+
%% vscode-js-debug NOTICES, INFORMATION, AND LICENSE BEGIN HERE
1039+
=========================================
1040+
1041+
MIT License
1042+
1043+
Copyright (c) Microsoft Corporation. All rights reserved.
1044+
1045+
Permission is hereby granted, free of charge, to any person obtaining a copy
1046+
of this software and associated documentation files (the "Software"), to deal
1047+
in the Software without restriction, including without limitation the rights
1048+
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
1049+
copies of the Software, and to permit persons to whom the Software is
1050+
furnished to do so, subject to the following conditions:
1051+
1052+
The above copyright notice and this permission notice shall be included in all
1053+
copies or substantial portions of the Software.
1054+
1055+
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
1056+
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
1057+
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
1058+
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
1059+
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
1060+
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
1061+
SOFTWARE
1062+
=========================================
1063+
END OF vscode-js-debug NOTICES, INFORMATION, AND LICENSE

pythonFiles/tests/pytestadapter/expected_discovery_test_output.py

+75
Original file line numberDiff line numberDiff line change
@@ -994,3 +994,78 @@
994994
],
995995
"id_": str(TEST_DATA_PATH),
996996
}
997+
SYMLINK_FOLDER_PATH = TEST_DATA_PATH / "symlink_folder"
998+
SYMLINK_FOLDER_PATH_TESTS = TEST_DATA_PATH / "symlink_folder" / "tests"
999+
SYMLINK_FOLDER_PATH_TESTS_TEST_A = (
1000+
TEST_DATA_PATH / "symlink_folder" / "tests" / "test_a.py"
1001+
)
1002+
SYMLINK_FOLDER_PATH_TESTS_TEST_B = (
1003+
TEST_DATA_PATH / "symlink_folder" / "tests" / "test_b.py"
1004+
)
1005+
1006+
symlink_expected_discovery_output = {
1007+
"name": "symlink_folder",
1008+
"path": str(SYMLINK_FOLDER_PATH),
1009+
"type_": "folder",
1010+
"children": [
1011+
{
1012+
"name": "tests",
1013+
"path": str(SYMLINK_FOLDER_PATH_TESTS),
1014+
"type_": "folder",
1015+
"id_": str(SYMLINK_FOLDER_PATH_TESTS),
1016+
"children": [
1017+
{
1018+
"name": "test_a.py",
1019+
"path": str(SYMLINK_FOLDER_PATH_TESTS_TEST_A),
1020+
"type_": "file",
1021+
"id_": str(SYMLINK_FOLDER_PATH_TESTS_TEST_A),
1022+
"children": [
1023+
{
1024+
"name": "test_a_function",
1025+
"path": str(SYMLINK_FOLDER_PATH_TESTS_TEST_A),
1026+
"lineno": find_test_line_number(
1027+
"test_a_function",
1028+
os.path.join(tests_path, "test_a.py"),
1029+
),
1030+
"type_": "test",
1031+
"id_": get_absolute_test_id(
1032+
"tests/test_a.py::test_a_function",
1033+
SYMLINK_FOLDER_PATH_TESTS_TEST_A,
1034+
),
1035+
"runID": get_absolute_test_id(
1036+
"tests/test_a.py::test_a_function",
1037+
SYMLINK_FOLDER_PATH_TESTS_TEST_A,
1038+
),
1039+
}
1040+
],
1041+
},
1042+
{
1043+
"name": "test_b.py",
1044+
"path": str(SYMLINK_FOLDER_PATH_TESTS_TEST_B),
1045+
"type_": "file",
1046+
"id_": str(SYMLINK_FOLDER_PATH_TESTS_TEST_B),
1047+
"children": [
1048+
{
1049+
"name": "test_b_function",
1050+
"path": str(SYMLINK_FOLDER_PATH_TESTS_TEST_B),
1051+
"lineno": find_test_line_number(
1052+
"test_b_function",
1053+
os.path.join(tests_path, "test_b.py"),
1054+
),
1055+
"type_": "test",
1056+
"id_": get_absolute_test_id(
1057+
"tests/test_b.py::test_b_function",
1058+
SYMLINK_FOLDER_PATH_TESTS_TEST_B,
1059+
),
1060+
"runID": get_absolute_test_id(
1061+
"tests/test_b.py::test_b_function",
1062+
SYMLINK_FOLDER_PATH_TESTS_TEST_B,
1063+
),
1064+
}
1065+
],
1066+
},
1067+
],
1068+
}
1069+
],
1070+
"id_": str(SYMLINK_FOLDER_PATH),
1071+
}

pythonFiles/tests/pytestadapter/helpers.py

+18
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Copyright (c) Microsoft Corporation. All rights reserved.
22
# Licensed under the MIT License.
33

4+
import contextlib
45
import io
56
import json
67
import os
@@ -27,6 +28,23 @@ def get_absolute_test_id(test_id: str, testPath: pathlib.Path) -> str:
2728
return absolute_test_id
2829

2930

31+
@contextlib.contextmanager
32+
def create_symlink(root: pathlib.Path, target_ext: str, destination_ext: str):
33+
try:
34+
destination = root / destination_ext
35+
target = root / target_ext
36+
if destination.exists():
37+
print("destination already exists", destination)
38+
try:
39+
destination.symlink_to(target)
40+
except Exception as e:
41+
print("error occurred when attempting to create a symlink", e)
42+
yield target, destination
43+
finally:
44+
destination.unlink()
45+
print("destination unlinked", destination)
46+
47+
3048
def create_server(
3149
host: str = "127.0.0.1",
3250
port: int = 0,

pythonFiles/tests/pytestadapter/test_discovery.py

+41-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# Copyright (c) Microsoft Corporation. All rights reserved.
22
# Licensed under the MIT License.
3+
import json
34
import os
45
import pathlib
56
import shutil
@@ -14,7 +15,7 @@
1415
from tests.tree_comparison_helper import is_same_tree
1516

1617
from . import expected_discovery_test_output
17-
from .helpers import TEST_DATA_PATH, runner, runner_with_cwd
18+
from .helpers import TEST_DATA_PATH, runner, runner_with_cwd, create_symlink
1819

1920

2021
def test_import_error(tmp_path):
@@ -205,6 +206,45 @@ def test_pytest_collect(file, expected_const):
205206
assert is_same_tree(actual_item.get("tests"), expected_const)
206207

207208

209+
def test_symlink_root_dir():
210+
"""
211+
Test to test pytest discovery with the command line arg --rootdir specified as a symlink path.
212+
Discovery should succeed and testids should be relative to the symlinked root directory.
213+
"""
214+
with create_symlink(TEST_DATA_PATH, "root", "symlink_folder") as (
215+
source,
216+
destination,
217+
):
218+
assert destination.is_symlink()
219+
220+
# Run pytest with the cwd being the resolved symlink path (as it will be when we run the subprocess from node).
221+
actual = runner_with_cwd(
222+
["--collect-only", f"--rootdir={os.fspath(destination)}"], source
223+
)
224+
expected = expected_discovery_test_output.symlink_expected_discovery_output
225+
assert actual
226+
actual_list: List[Dict[str, Any]] = actual
227+
if actual_list is not None:
228+
assert actual_list.pop(-1).get("eot")
229+
actual_item = actual_list.pop(0)
230+
try:
231+
# Check if all requirements
232+
assert all(
233+
item in actual_item.keys() for item in ("status", "cwd", "error")
234+
), "Required keys are missing"
235+
assert actual_item.get("status") == "success", "Status is not 'success'"
236+
assert actual_item.get("cwd") == os.fspath(
237+
destination
238+
), f"CWD does not match: {os.fspath(destination)}"
239+
assert (
240+
actual_item.get("tests") == expected
241+
), "Tests do not match expected value"
242+
except AssertionError as e:
243+
# Print the actual_item in JSON format if an assertion fails
244+
print(json.dumps(actual_item, indent=4))
245+
pytest.fail(str(e))
246+
247+
208248
def test_pytest_root_dir():
209249
"""
210250
Test to test pytest discovery with the command line arg --rootdir specified to be a subfolder

pythonFiles/vscode_pytest/__init__.py

+51-2
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ def __init__(self, message):
5757
collected_tests_so_far = list()
5858
TEST_PORT = os.getenv("TEST_PORT")
5959
TEST_UUID = os.getenv("TEST_UUID")
60+
SYMLINK_PATH = None
6061

6162

6263
def pytest_load_initial_conftests(early_config, parser, args):
@@ -75,6 +76,25 @@ def pytest_load_initial_conftests(early_config, parser, args):
7576
global IS_DISCOVERY
7677
IS_DISCOVERY = True
7778

79+
# check if --rootdir is in the args
80+
for arg in args:
81+
if "--rootdir=" in arg:
82+
rootdir = arg.split("--rootdir=")[1]
83+
if not os.path.exists(rootdir):
84+
raise VSCodePytestError(
85+
f"The path set in the argument --rootdir={rootdir} does not exist."
86+
)
87+
if (
88+
os.path.islink(rootdir)
89+
and pathlib.Path(os.path.realpath(rootdir)) == pathlib.Path.cwd()
90+
):
91+
print(
92+
f"Plugin info[vscode-pytest]: rootdir argument, {rootdir}, is identified as a symlink to the cwd, {pathlib.Path.cwd()}.",
93+
"Therefore setting symlink path to rootdir argument.",
94+
)
95+
global SYMLINK_PATH
96+
SYMLINK_PATH = pathlib.Path(rootdir)
97+
7898

7999
def pytest_internalerror(excrepr, excinfo):
80100
"""A pytest hook that is called when an internal error occurs.
@@ -326,6 +346,13 @@ def pytest_sessionfinish(session, exitstatus):
326346
Exit code 5: No tests were collected
327347
"""
328348
cwd = pathlib.Path.cwd()
349+
if SYMLINK_PATH:
350+
print("Plugin warning[vscode-pytest]: SYMLINK set, adjusting cwd.")
351+
# Get relative between the cwd (resolved path) and the node path.
352+
rel_path = os.path.relpath(cwd, pathlib.Path.cwd())
353+
# Calculate the new node path by making it relative to the symlink path.
354+
cwd = pathlib.Path(os.path.join(SYMLINK_PATH, rel_path))
355+
329356
if IS_DISCOVERY:
330357
if not (exitstatus == 0 or exitstatus == 1 or exitstatus == 5):
331358
errorNode: TestNode = {
@@ -388,6 +415,11 @@ def build_test_tree(session: pytest.Session) -> TestNode:
388415
class_nodes_dict: Dict[str, TestNode] = {}
389416
function_nodes_dict: Dict[str, TestNode] = {}
390417

418+
# Check to see if the global variable for symlink path is set
419+
if SYMLINK_PATH:
420+
session_node["path"] = SYMLINK_PATH
421+
session_node["id_"] = os.fspath(SYMLINK_PATH)
422+
391423
for test_case in session.items:
392424
test_node = create_test_node(test_case)
393425
if isinstance(test_case.parent, pytest.Class):
@@ -645,13 +677,31 @@ class EOTPayloadDict(TypedDict):
645677

646678

647679
def get_node_path(node: Any) -> pathlib.Path:
648-
"""A function that returns the path of a node given the switch to pathlib.Path."""
680+
"""
681+
A function that returns the path of a node given the switch to pathlib.Path.
682+
It also evaluates if the node is a symlink and returns the equivalent path.
683+
"""
649684
path = getattr(node, "path", None) or pathlib.Path(node.fspath)
650685

651686
if not path:
652687
raise VSCodePytestError(
653688
f"Unable to find path for node: {node}, node.path: {node.path}, node.fspath: {node.fspath}"
654689
)
690+
691+
# Check for the session node since it has the symlink already.
692+
if SYMLINK_PATH and not isinstance(node, pytest.Session):
693+
# Get relative between the cwd (resolved path) and the node path.
694+
try:
695+
rel_path = path.relative_to(pathlib.Path.cwd())
696+
697+
# Calculate the new node path by making it relative to the symlink path.
698+
sym_path = pathlib.Path(os.path.join(SYMLINK_PATH, rel_path))
699+
return sym_path
700+
except Exception as e:
701+
raise VSCodePytestError(
702+
f"Error occurred while calculating symlink equivalent from node path: {e}"
703+
"\n SYMLINK_PATH: {SYMLINK_PATH}, \n node path: {path}, \n cwd: {{pathlib.Path.cwd()}}"
704+
)
655705
return path
656706

657707

@@ -687,7 +737,6 @@ def post_response(cwd: str, session_node: TestNode) -> None:
687737
cwd (str): Current working directory.
688738
session_node (TestNode): Node information of the test session.
689739
"""
690-
691740
payload: DiscoveryPayloadDict = {
692741
"cwd": cwd,
693742
"status": "success" if not ERRORS else "error",

src/client/testing/testController/common/utils.ts

+80
Original file line numberDiff line numberDiff line change
@@ -349,3 +349,83 @@ export function splitTestNameWithRegex(testName: string): [string, string] {
349349
}
350350
return [testName, testName];
351351
}
352+
353+
/**
354+
* Converts an array of strings (with or without '=') into a map.
355+
* If a string contains '=', it is split into a key-value pair, with the portion
356+
* before the '=' as the key and the portion after the '=' as the value.
357+
* If no '=' is found in the string, the entire string becomes a key with a value of null.
358+
*
359+
* @param args - Readonly array of strings to be converted to a map.
360+
* @returns A map representation of the input strings.
361+
*/
362+
export const argsToMap = (args: ReadonlyArray<string>): { [key: string]: string | null | undefined } => {
363+
const map: { [key: string]: string | null } = {};
364+
for (const arg of args) {
365+
const delimiter = arg.indexOf('=');
366+
if (delimiter === -1) {
367+
map[arg] = null;
368+
} else {
369+
map[arg.slice(0, delimiter)] = arg.slice(delimiter + 1);
370+
}
371+
}
372+
373+
return map;
374+
};
375+
376+
/**
377+
* Converts a map into an array of strings.
378+
* Each key-value pair in the map is transformed into a string.
379+
* If the value is null, only the key is represented in the string.
380+
* If the value is defined (and not null), the string is in the format "key=value".
381+
* If a value is undefined, the key-value pair is skipped.
382+
*
383+
* @param map - The map to be converted to an array of strings.
384+
* @returns An array of strings representation of the input map.
385+
*/
386+
export const mapToArgs = (map: { [key: string]: string | null | undefined }): string[] => {
387+
const out: string[] = [];
388+
for (const key of Object.keys(map)) {
389+
const value = map[key];
390+
if (value === undefined) {
391+
// eslint-disable-next-line no-continue
392+
continue;
393+
}
394+
395+
out.push(value === null ? key : `${key}=${value}`);
396+
}
397+
398+
return out;
399+
};
400+
401+
/**
402+
* Adds an argument to the map only if it doesn't already exist.
403+
*
404+
* @param map - The map of arguments.
405+
* @param argKey - The argument key to be checked and added.
406+
* @param argValue - The value to set for the argument if it's not already in the map.
407+
* @returns The updated map.
408+
*/
409+
export function addArgIfNotExist(
410+
map: { [key: string]: string | null | undefined },
411+
argKey: string,
412+
argValue: string | null,
413+
): { [key: string]: string | null | undefined } {
414+
// Only add the argument if it doesn't exist in the map.
415+
if (map[argKey] === undefined) {
416+
map[argKey] = argValue;
417+
}
418+
419+
return map;
420+
}
421+
422+
/**
423+
* Checks if an argument key exists in the map.
424+
*
425+
* @param map - The map of arguments.
426+
* @param argKey - The argument key to be checked.
427+
* @returns True if the argument key exists in the map, false otherwise.
428+
*/
429+
export function argKeyExists(map: { [key: string]: string | null | undefined }, argKey: string): boolean {
430+
return map[argKey] !== undefined;
431+
}

0 commit comments

Comments
 (0)