From 4c159020b9a74dca200c0980caaff092f3e00f97 Mon Sep 17 00:00:00 2001 From: yuanyuyuan Date: Wed, 29 Jul 2026 19:03:02 +0800 Subject: [PATCH 1/4] test(py): actually run the ZPayloadView zero-copy assertions `TestZPayloadViewNumpy` guarded on `pytest.importorskip("numpy")`, and numpy was installed by nothing -- not pyproject dependencies, not an extra, not the test script. The venv is built with plain `python -m venv` so it inherits nothing from the devShell. Both tests had therefore always skipped, and every run ended "86 passed, 2 skipped". What never ran is the only assertion in the suite that proves the zero-copy claim: assert arr.flags["OWNDATA"] is False If ZPayloadView regressed to copying, the whole suite would still pass. Install numpy as a test dependency, and replace the importorskip with a real import so a missing numpy fails instead of skipping. A permanently skipped test is indistinguishable from a passing one in the summary line. Closes #266 --- crates/hiroz-py/tests/test_payload_view.py | 21 ++++++++++++++++++--- scripts/test-python.nu | 8 ++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/crates/hiroz-py/tests/test_payload_view.py b/crates/hiroz-py/tests/test_payload_view.py index 8a814e4dc..efc0df106 100644 --- a/crates/hiroz-py/tests/test_payload_view.py +++ b/crates/hiroz-py/tests/test_payload_view.py @@ -163,12 +163,27 @@ def test_recv_raw_view_timeout(self, byte_array_pubsub): class TestZPayloadViewNumpy: - """Test ZPayloadView with numpy (if available).""" + """Test that ZPayloadView really is zero-copy, via numpy's OWNDATA flag. + + numpy is a declared test dependency, so its absence is a failure rather + than a skip. These tests previously guarded with ``importorskip`` and + numpy was never installed, so both had *always* skipped — and the only + assertion in the suite that proves the zero-copy claim + (``arr.flags["OWNDATA"] is False``) had never executed. A permanently + skipped test is indistinguishable from a passing one in the summary line. + """ @pytest.fixture(autouse=True) def check_numpy(self): - """Skip tests if numpy is not available.""" - pytest.importorskip("numpy") + """Fail loudly if numpy is missing, rather than skipping silently.""" + try: + import numpy # noqa: F401 + except ImportError as exc: # pragma: no cover - env misconfiguration + pytest.fail( + "numpy is a declared test dependency but is not installed, so " + "the ZPayloadView zero-copy assertions cannot run. Install it " + "with the test extra; do not restore importorskip here." + ) def test_numpy_frombuffer(self, byte_array_pubsub): """Test that numpy.frombuffer works with ZPayloadView.""" diff --git a/scripts/test-python.nu b/scripts/test-python.nu index 7a21507af..50f5116d4 100755 --- a/scripts/test-python.nu +++ b/scripts/test-python.nu @@ -42,6 +42,14 @@ def setup-venv [] { run-cmd "cd crates/hiroz-py; source .venv/bin/activate && pip install -e ../hiroz-msgs/python/" --shell bash --distro (get-distro) print " Installed hiroz-msgs-py (message types)" + # Test-only dependency. `TestZPayloadViewNumpy` carries the only assertion + # that proves ZPayloadView is zero-copy (`arr.flags["OWNDATA"] is False`), + # and the venv is created with plain `python -m venv`, so it inherits + # nothing from the surrounding devShell. Without this the tests skip and + # the suite still reports success -- see #266. + run-cmd "cd crates/hiroz-py; source .venv/bin/activate && pip install 'numpy>=1.21'" --shell bash --distro (get-distro) + print " Installed numpy (test dependency for the zero-copy assertions)" + # Install hiroz-py in editable mode using maturin run-cmd "cd crates/hiroz-py; source .venv/bin/activate && RUSTFLAGS='-D warnings' maturin develop" --shell bash --distro (get-distro) print " Installed hiroz-py (Rust bindings)" From 95d098e2265807ac118d16ca8846e8d479e28e93 Mon Sep 17 00:00:00 2001 From: yuanyuyuan Date: Wed, 29 Jul 2026 19:07:25 +0800 Subject: [PATCH 2/4] style(py): drop an unused exception binding flagged by ruff --- crates/hiroz-py/tests/test_payload_view.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/hiroz-py/tests/test_payload_view.py b/crates/hiroz-py/tests/test_payload_view.py index efc0df106..3d9b0d3de 100644 --- a/crates/hiroz-py/tests/test_payload_view.py +++ b/crates/hiroz-py/tests/test_payload_view.py @@ -178,7 +178,7 @@ def check_numpy(self): """Fail loudly if numpy is missing, rather than skipping silently.""" try: import numpy # noqa: F401 - except ImportError as exc: # pragma: no cover - env misconfiguration + except ImportError: # pragma: no cover - env misconfiguration pytest.fail( "numpy is a declared test dependency but is not installed, so " "the ZPayloadView zero-copy assertions cannot run. Install it " From 295874e1fc29184d8046acfbb16949c7ce8ec9fd Mon Sep 17 00:00:00 2001 From: yuanyuyuan Date: Wed, 29 Jul 2026 19:22:43 +0800 Subject: [PATCH 3/4] build(py): declare the test extra the failure message points at Adversarial review caught that the new failure message instructs the user to "install it with the test extra", and no such extra existed -- pyproject.toml has no [project.optional-dependencies] at all. numpy was installed by exactly one imperative line in the test script. A contributor doing the natural manual setup (venv, pip install -e ., maturin develop, pytest) previously got 2 skips and now gets 2 hard errors whose stated remedy fails with "no extra 'test'". Declare test = ["numpy>=1.21"] and have the script install via it, so the dependency is declared in one place and the message is true. --- crates/hiroz-py/pyproject.toml | 7 +++++++ scripts/test-python.nu | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/hiroz-py/pyproject.toml b/crates/hiroz-py/pyproject.toml index 8aedb5298..485c1073c 100644 --- a/crates/hiroz-py/pyproject.toml +++ b/crates/hiroz-py/pyproject.toml @@ -27,6 +27,13 @@ dependencies = [ "hiroz-msgs-py>=0.1.0", ] +[project.optional-dependencies] +# numpy carries the only assertion that proves ZPayloadView is zero-copy +# (`arr.flags["OWNDATA"] is False`). Declared here rather than installed +# imperatively by the test script so that `pip install -e ".[test]"` is a +# true instruction -- the tests' own failure message points at it. +test = ["numpy>=1.21"] + [tool.maturin] features = ["pyo3/extension-module", "jazzy"] python-source = "python" diff --git a/scripts/test-python.nu b/scripts/test-python.nu index 50f5116d4..60fddae16 100755 --- a/scripts/test-python.nu +++ b/scripts/test-python.nu @@ -47,8 +47,8 @@ def setup-venv [] { # and the venv is created with plain `python -m venv`, so it inherits # nothing from the surrounding devShell. Without this the tests skip and # the suite still reports success -- see #266. - run-cmd "cd crates/hiroz-py; source .venv/bin/activate && pip install 'numpy>=1.21'" --shell bash --distro (get-distro) - print " Installed numpy (test dependency for the zero-copy assertions)" + run-cmd "cd crates/hiroz-py; source .venv/bin/activate && pip install -e '.[test]'" --shell bash --distro (get-distro) + print " Installed the test extra (numpy, for the zero-copy assertions)" # Install hiroz-py in editable mode using maturin run-cmd "cd crates/hiroz-py; source .venv/bin/activate && RUSTFLAGS='-D warnings' maturin develop" --shell bash --distro (get-distro) From 515fdbf4b1e49ba0eddf8f4507111da353394282 Mon Sep 17 00:00:00 2001 From: yuanyuyuan Date: Wed, 29 Jul 2026 19:33:41 +0800 Subject: [PATCH 4/4] docs(py): correct what the numpy OWNDATA assertion actually proves Copilot was right and the claim was overstated in three places. OWNDATA == False proves numpy borrowed the buffer ZPayloadView exported rather than copying it. It does not prove the transport was zero-copy: ZPayloadView::new falls back to PayloadBytes::Owned for a fragmented payload, and numpy borrows that owned copy just as happily. The transport-level check is `payload.is_zero_copy_py`, asserted by test_payload_view_is_zero_copy, which was already running. The reason to fix #266 is unchanged -- two tests had never executed and a permanent skip reads as a pass -- but the value claim now matches what the assertion can support. --- crates/hiroz-py/pyproject.toml | 4 ++-- crates/hiroz-py/tests/test_payload_view.py | 13 +++++++++---- scripts/test-python.nu | 10 +++++----- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/crates/hiroz-py/pyproject.toml b/crates/hiroz-py/pyproject.toml index 485c1073c..b251d7e9c 100644 --- a/crates/hiroz-py/pyproject.toml +++ b/crates/hiroz-py/pyproject.toml @@ -28,8 +28,8 @@ dependencies = [ ] [project.optional-dependencies] -# numpy carries the only assertion that proves ZPayloadView is zero-copy -# (`arr.flags["OWNDATA"] is False`). Declared here rather than installed +# numpy backs the tests that prove it can consume ZPayloadView's exported +# buffer without adding a copy. Declared here rather than installed # imperatively by the test script so that `pip install -e ".[test]"` is a # true instruction -- the tests' own failure message points at it. test = ["numpy>=1.21"] diff --git a/crates/hiroz-py/tests/test_payload_view.py b/crates/hiroz-py/tests/test_payload_view.py index 3d9b0d3de..6b9a2ee31 100644 --- a/crates/hiroz-py/tests/test_payload_view.py +++ b/crates/hiroz-py/tests/test_payload_view.py @@ -163,14 +163,19 @@ def test_recv_raw_view_timeout(self, byte_array_pubsub): class TestZPayloadViewNumpy: - """Test that ZPayloadView really is zero-copy, via numpy's OWNDATA flag. + """Test that numpy can consume ZPayloadView without adding a copy. numpy is a declared test dependency, so its absence is a failure rather than a skip. These tests previously guarded with ``importorskip`` and - numpy was never installed, so both had *always* skipped — and the only - assertion in the suite that proves the zero-copy claim - (``arr.flags["OWNDATA"] is False``) had never executed. A permanently + numpy was never installed, so both had *always* skipped. A permanently skipped test is indistinguishable from a passing one in the summary line. + + Scope, precisely: ``arr.flags["OWNDATA"] is False`` proves numpy borrowed + the buffer ZPayloadView exported rather than copying it. It does *not* + prove the transport was zero-copy — ``ZPayloadView::new`` falls back to + ``PayloadBytes::Owned`` for a fragmented payload, and numpy borrows that + owned copy just as happily. ``test_payload_view_is_zero_copy`` asserts + ``payload.is_zero_copy_py``, which is the transport-level check. """ @pytest.fixture(autouse=True) diff --git a/scripts/test-python.nu b/scripts/test-python.nu index 60fddae16..cd41bc45d 100755 --- a/scripts/test-python.nu +++ b/scripts/test-python.nu @@ -42,11 +42,11 @@ def setup-venv [] { run-cmd "cd crates/hiroz-py; source .venv/bin/activate && pip install -e ../hiroz-msgs/python/" --shell bash --distro (get-distro) print " Installed hiroz-msgs-py (message types)" - # Test-only dependency. `TestZPayloadViewNumpy` carries the only assertion - # that proves ZPayloadView is zero-copy (`arr.flags["OWNDATA"] is False`), - # and the venv is created with plain `python -m venv`, so it inherits - # nothing from the surrounding devShell. Without this the tests skip and - # the suite still reports success -- see #266. + # Test-only dependency. `TestZPayloadViewNumpy` checks that numpy consumes + # ZPayloadView's exported buffer without adding a copy, and the venv is + # created with plain `python -m venv`, so it inherits nothing from the + # surrounding devShell. Without this the tests skip and the suite still + # reports success -- see #266. run-cmd "cd crates/hiroz-py; source .venv/bin/activate && pip install -e '.[test]'" --shell bash --distro (get-distro) print " Installed the test extra (numpy, for the zero-copy assertions)"