Skip to content

Polygraphy: rename the shadowed is_on_gpu test so is_on_cpu is actually tested - #4847

Open
Anai-Guo wants to merge 1 commit into
NVIDIA:mainfrom
Anai-Guo:fix/polygraphy-shadowed-is-on-gpu-test
Open

Polygraphy: rename the shadowed is_on_gpu test so is_on_cpu is actually tested#4847
Anai-Guo wants to merge 1 commit into
NVIDIA:mainfrom
Anai-Guo:fix/polygraphy-shadowed-is-on-gpu-test

Conversation

@Anai-Guo

@Anai-Guo Anai-Guo commented Sep 9, 2026

Copy link
Copy Markdown

Description

tools/Polygraphy/tests/util/test_array.py defines test_is_on_cpu twice:

@pytest.mark.parametrize("obj, is_on_cpu", [... True, True, False, False])
def test_is_on_cpu(obj, is_on_cpu):
    assert util.array.is_on_cpu(obj) == is_on_cpu      # line 86

@pytest.mark.parametrize("obj, is_on_gpu", [... False, False, True, True])
def test_is_on_cpu(obj, is_on_gpu):                    # line 99  <-- same name
    assert util.array.is_on_gpu(obj) == is_on_gpu

The second def rebinds the module-level name, so the first function object is
discarded before pytest ever sees it. The result is that util.array.is_on_cpu
has no test coverage at all
— the four cases that pytest does collect under the
name test_is_on_cpu are in fact the is_on_gpu cases.

From the body and the parametrize ids it is clear the second one was meant to be
test_is_on_gpu (it asserts util.array.is_on_gpu, and its expectations are the
exact inverse of the first block's).

Reproduction

Minimal standalone reduction of the same two-def structure, before and after
this change:

$ pytest --collect-only -q test_before.py
test_before.py::test_is_on_cpu[np-False]
test_before.py::test_is_on_cpu[t_cpu-False]
test_before.py::test_is_on_cpu[t_cuda-True]
test_before.py::test_is_on_cpu[dev-True]
4 tests collected

$ pytest --collect-only -q test_after.py
test_after.py::test_is_on_cpu[np-True]
test_after.py::test_is_on_cpu[t_cpu-True]
test_after.py::test_is_on_cpu[t_cuda-False]
test_after.py::test_is_on_cpu[dev-False]
test_after.py::test_is_on_gpu[np-False]
test_after.py::test_is_on_gpu[t_cpu-False]
test_after.py::test_is_on_gpu[t_cuda-True]
test_after.py::test_is_on_gpu[dev-True]
8 tests collected

Note the ids in the "before" run: the surviving test_is_on_cpu carries the
is_on_gpu expectations (np-False), confirming the CPU test is gone.

Fix

Rename the second definition to test_is_on_gpu. One line, no behavioural
change to Polygraphy itself; it restores the four is_on_cpu cases
(4 collected → 8 collected in this file).


🤖 Generated with Claude Code

tests/util/test_array.py defines test_is_on_cpu twice.  The second
definition (which asserts util.array.is_on_gpu) rebinds the name, so
pytest only ever collects the GPU parametrization and
util.array.is_on_cpu has no coverage at all.

Signed-off-by: Anai Guo <antai12232931@outlook.com>
@Anai-Guo
Anai-Guo requested a review from a team as a code owner September 9, 2026 07:11
@Anai-Guo
Anai-Guo requested review from poweiw and wenbingl September 9, 2026 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant