Skip to content

optimize compact_path() in pytest discovery for the common case #26016

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
vaclavHala wants to merge 1 commit into
base: main
Choose a base branch
from
+32 −11
Open

optimize compact_path() in pytest discovery for the common case #26016

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions python_files/tests/pytestadapter/test_discovery.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ def test_compact_discovery_payload_keeps_paths_outside_base_absolute(tmp_path):
base_path = tmp_path / "workspace"
external_file = tmp_path / "external" / "test_external.py"

assert vscode_pytest.compact_path(external_file, base_path) == os.fspath(external_file)
assert vscode_pytest.compact_path(external_file, base_path, str(base_path)) == os.fspath(external_file)
assert (
vscode_pytest.compact_test_id(f"{os.fspath(external_file)}::test_external", base_path)
vscode_pytest.compact_test_id(f"{os.fspath(external_file)}::test_external", base_path, str(base_path))
== f"{os.fspath(external_file)}::test_external"
)

Expand Down
39 changes: 30 additions & 9 deletions python_files/vscode_pytest/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1005,9 +1005,26 @@ def cached_fsdecode(path: pathlib.Path) -> str:
return _path_to_str_cache[path]


def compact_path(path: pathlib.Path | str, path_base: pathlib.Path) -> str:
def compact_path(path: pathlib.Path | str, path_base: pathlib.Path, path_base_str: str) -> str:
"""Return path relative to path_base when possible without resolving symlinks."""
current_path = pathlib.Path(path)

if isinstance(path, str):
path_str = path
current_path = pathlib.Path(path)
else:
path_str = str(path)
current_path = path

# pathlib.Path.relative_to is an expensive operation,
# for common cases where path is nested in path_base and path_base is therefore a prefix
# we skip the expensive check and just chop off the prefix to yield relative path
if path_str.startswith(path_base_str):
# +1 because pathlib.Path never ends by a trailing separator which we also need to chop off:
# path_base_str= /some/prefix
# path_str= /some/prefix/tests/mytest.py
rel_str = path_str[(len(path_base_str) + 1):]
return "." if rel_str == "" else rel_str

if not current_path.is_absolute():
return os.fspath(current_path)

Expand All @@ -1020,17 +1037,19 @@ def compact_path(path: pathlib.Path | str, path_base: pathlib.Path) -> str:
return relative_path_str or "."


def compact_test_id(test_id: str, id_base: pathlib.Path) -> str:
def compact_test_id(test_id: str, id_base: pathlib.Path, id_base_str: str) -> str:
"""Compact the path prefix in a pytest node id while preserving pytest selectors."""
test_path, separator, selector = test_id.partition("::")
compact_test_path = compact_path(test_path, id_base)
compact_test_path = compact_path(test_path, id_base, id_base_str)
return f"{compact_test_path}{separator}{selector}" if separator else compact_test_path


def compact_test_node(
test_node: TestNode | TestItem | None,
path_base: pathlib.Path,
id_base: pathlib.Path,
path_base_str: str,
id_base_str: str,
) -> dict[str, Any] | None:
"""Create a compact copy of a discovery node for JSON serialization."""
if test_node is None:
Expand All @@ -1039,13 +1058,13 @@ def compact_test_node(
compact_node: dict[str, Any] = {}
for key, value in test_node.items():
if key == "path":
compact_node[key] = compact_path(cast("pathlib.Path", value), path_base)
compact_node[key] = compact_path(cast("pathlib.Path", value), path_base, path_base_str)
elif key in {"id_", "runID"}:
compact_node[key] = compact_test_id(cast("str", value), id_base)
compact_node[key] = compact_test_id(cast("str", value), id_base, id_base_str)
elif key == "children":
children_iter = value.values() if isinstance(value, Children) else value
compact_node[key] = [
compact_test_node(child, path_base, id_base)
compact_test_node(child, path_base, id_base, path_base_str, id_base_str)
for child in cast("list[TestNode | TestItem | None]", children_iter)
]
else:
Expand All @@ -1057,12 +1076,14 @@ def create_compact_discovery_payload(
cwd: str, session_node: TestNode
) -> CompactDiscoveryPayloadDict:
"""Create the compact wire payload after discovery has fully resolved the tree."""
path_base = pathlib.Path(session_node["path"])
path_base_str = str(session_node["path"])
path_base = pathlib.Path(path_base_str)
id_base = path_base
id_base_str = path_base_str
return CompactDiscoveryPayloadDict(
cwd=cwd,
status="success" if not ERRORS else "error",
tests=cast("TestNode", compact_test_node(session_node, path_base, id_base)),
tests=cast("TestNode", compact_test_node(session_node, path_base, id_base, path_base_str, id_base_str)),
error=ERRORS,
payloadVersion=2,
pathBase=os.fspath(path_base),
Expand Down