Skip to content

Commit bbd41d4

Browse files
rparolinclaude
andcommitted
Trim review-pass: drop narrating comments, dead-branch test, JSON probe
- Drop test for the no-message NOT_FOUND fallback branch — the only producer always sets {"type", "message"}; the fallback exists only to keep the path graceful on protocol violation. - Inline the find-side error-message ternary; same behavior. - Replace JSON-payload probe with one-word stdout (not-found / loaded / ok) in the does-not-load test. - Drop two narrating test comments that paraphrase the test name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 90fd042 commit bbd41d4

2 files changed

Lines changed: 8 additions & 36 deletions

File tree

cuda_pathfinder/cuda/pathfinder/_dynamic_libs/find_nvidia_dynamic_lib.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,5 @@ def find_nvidia_dynamic_lib(libname: str) -> str:
8484
return abs_path
8585

8686
error = payload.error
87-
if error is not None and "message" in error:
88-
message = error["message"]
89-
else:
90-
message = f"find_nvidia_dynamic_lib could not locate {libname!r}"
87+
message = error["message"] if error and "message" in error else f"could not locate {libname!r}"
9188
raise DynamicLibNotFoundError(message)

cuda_pathfinder/tests/test_find_nvidia_dynamic_lib.py

Lines changed: 7 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
# SPDX-License-Identifier: Apache-2.0
33

4-
import json
54
import os
65
import platform
76
import subprocess
@@ -75,9 +74,6 @@ def test_find_nvidia_dynamic_lib_returns_existing_path_without_loading(info_summ
7574

7675

7776
def test_find_matches_load_in_subprocess(info_summary_append):
78-
# Single representative libname is enough to exercise the consistency
79-
# claim (see issue #757); per-libname coverage is provided by the
80-
# parametrized find/load tests independently.
8177
libname = "cudart"
8278
find_nvidia_dynamic_lib.cache_clear()
8379
timeout = 120 if IS_WINDOWS else 30
@@ -102,9 +98,6 @@ def test_find_matches_load_in_subprocess(info_summary_append):
10298

10399

104100
def test_find_nvidia_dynamic_lib_propagates_subprocess_not_found_message(monkeypatch):
105-
# End-to-end of the structured-error path: the subprocess child encodes
106-
# DynamicLibNotFoundError into the JSON payload; the parent re-raises
107-
# with the original message preserved.
108101
find_nvidia_dynamic_lib.cache_clear()
109102
expected = "child loader said: cudart could not be located"
110103

@@ -120,35 +113,17 @@ def fake_run(mode, libname, *, timeout, error_label):
120113
find_nvidia_dynamic_lib("cudart")
121114

122115

123-
def test_find_nvidia_dynamic_lib_falls_back_when_subprocess_not_found_omits_message(monkeypatch):
124-
find_nvidia_dynamic_lib.cache_clear()
125-
126-
def fake_run(mode, libname, *, timeout, error_label):
127-
return DynamicLibSubprocessPayload(status=STATUS_NOT_FOUND, abs_path=None, error=None)
128-
129-
monkeypatch.setattr(find_nvidia_dynamic_lib_module, "run_dynamic_lib_subprocess", fake_run)
130-
with pytest.raises(DynamicLibNotFoundError, match=r"could not locate 'cudart'"):
131-
find_nvidia_dynamic_lib("cudart")
132-
133-
134116
_DOES_NOT_LOAD_PROBE = textwrap.dedent(
135117
"""
136-
import json
137-
import os
138118
import sys
139-
140119
from cuda.pathfinder import DynamicLibNotFoundError, find_nvidia_dynamic_lib
141-
142120
libname = sys.argv[1]
143121
try:
144122
find_nvidia_dynamic_lib(libname)
145-
find_status = "found"
146123
except DynamicLibNotFoundError:
147-
find_status = "not-found"
148-
124+
print("not-found"); sys.exit(0)
149125
with open("/proc/self/maps") as f:
150-
maps = f.read()
151-
print(json.dumps({"find_status": find_status, "loaded": ("lib" + libname) in maps}))
126+
print("loaded" if ("lib" + libname) in f.read() else "ok")
152127
"""
153128
).strip()
154129

@@ -157,9 +132,9 @@ def test_find_nvidia_dynamic_lib_does_not_load_in_caller_process():
157132
if IS_WINDOWS or not os.path.exists("/proc/self/maps"):
158133
pytest.skip("Requires /proc/self/maps for in-process load detection")
159134

135+
# Run in a fresh interpreter so other pathfinder tests in the same
136+
# pytest process can't have pre-loaded the library.
160137
libname = "cudart"
161-
# Run in a fresh interpreter so test ordering / other pathfinder tests
162-
# in the same process can't have pre-loaded the library.
163138
result = subprocess.run( # noqa: S603 - trusted argv: current interpreter + inline probe
164139
[sys.executable, "-c", _DOES_NOT_LOAD_PROBE, libname],
165140
capture_output=True,
@@ -168,7 +143,7 @@ def test_find_nvidia_dynamic_lib_does_not_load_in_caller_process():
168143
check=False,
169144
)
170145
assert result.returncode == 0, f"probe failed:\nstdout={result.stdout!r}\nstderr={result.stderr!r}"
171-
payload = json.loads(result.stdout.strip().splitlines()[-1])
172-
if payload["find_status"] == "not-found":
146+
verdict = result.stdout.strip().splitlines()[-1]
147+
if verdict == "not-found":
173148
pytest.skip(f"{libname} not available on this host")
174-
assert payload["loaded"] is False, "find_nvidia_dynamic_lib must not load the library into the caller process"
149+
assert verdict == "ok", f"find_nvidia_dynamic_lib must not load the library into the caller process ({verdict=})"

0 commit comments

Comments
 (0)