Skip to content

Commit b4179e4

Browse files
test: enable pytest strict mode, fix duplicate parametrization ids, and add 10m timeout to individual tests (#4140)
* chore(deps): bump the actions group across 1 directory with 8 updates (#176) Bumps the actions group with 8 updates in the / directory: | Package | From | To | | --- | --- | --- | | [prefix-dev/setup-pixi](https://github.com/prefix-dev/setup-pixi) | `0.9.5` | `0.9.6` | | [codecov/codecov-action](https://github.com/codecov/codecov-action) | `6.0.0` | `6.0.1` | | [github/issue-metrics](https://github.com/github/issue-metrics) | `4.2.2` | `4.2.7` | | [j178/prek-action](https://github.com/j178/prek-action) | `2.0.3` | `2.0.4` | | [actions/upload-artifact](https://github.com/actions/upload-artifact) | `7.0.0` | `7.0.1` | | [actions/download-artifact](https://github.com/actions/download-artifact) | `7.0.0` | `8.0.1` | | [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish) | `1.13.0` | `1.14.0` | | [zizmorcore/zizmor-action](https://github.com/zizmorcore/zizmor-action) | `0.5.3` | `0.5.6` | Updates `prefix-dev/setup-pixi` from 0.9.5 to 0.9.6 - [Release notes](https://github.com/prefix-dev/setup-pixi/releases) - [Commits](prefix-dev/setup-pixi@1b2de7f...5185adf) Updates `codecov/codecov-action` from 6.0.0 to 6.0.1 - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@57e3a13...e79a696) Updates `github/issue-metrics` from 4.2.2 to 4.2.7 - [Release notes](https://github.com/github/issue-metrics/releases) - [Commits](github-community-projects/issue-metrics@c9e9838...1e38d5e) Updates `j178/prek-action` from 2.0.3 to 2.0.4 - [Release notes](https://github.com/j178/prek-action/releases) - [Commits](j178/prek-action@6ad8027...bdca6f1) Updates `actions/upload-artifact` from 7.0.0 to 7.0.1 - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v7...043fb46) Updates `actions/download-artifact` from 7.0.0 to 8.0.1 - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@v7...3e5f45b) Updates `pypa/gh-action-pypi-publish` from 1.13.0 to 1.14.0 - [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases) - [Commits](pypa/gh-action-pypi-publish@v1.13.0...cef2210) Updates `zizmorcore/zizmor-action` from 0.5.3 to 0.5.6 - [Release notes](https://github.com/zizmorcore/zizmor-action/releases) - [Commits](zizmorcore/zizmor-action@b1d7e1f...5f14fd0) --- updated-dependencies: - dependency-name: prefix-dev/setup-pixi dependency-version: 0.9.6 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions - dependency-name: codecov/codecov-action dependency-version: 6.0.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions - dependency-name: github/issue-metrics dependency-version: 4.2.7 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions - dependency-name: j178/prek-action dependency-version: 2.0.4 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions - dependency-name: actions/upload-artifact dependency-version: 7.0.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions - dependency-name: actions/download-artifact dependency-version: 8.0.1 dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: pypa/gh-action-pypi-publish dependency-version: 1.14.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: actions - dependency-name: zizmorcore/zizmor-action dependency-version: 0.5.6 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * test: bind moto server to an ephemeral port to fix xdist deadlock The session-scoped moto_server fixture bound a fixed port (5555). Under pytest-xdist, session-scoped fixtures run once per worker, so concurrent workers raced to bind the same port. The losers blocked forever inside ThreadedMotoServer.start(): its werkzeug thread dies on "Address already in use" before setting the ready event that start() waits on, and the wait has no timeout. Any multi-worker run whose s3 tests landed on 2+ workers deadlocked near the end of the suite at 0% CPU. CI never saw this because it runs the suite single-process. Bind port 0 instead and yield the actual endpoint from the fixture. Since the endpoint is now only known at runtime, test_fsspec.py's module-level endpoint_url constant becomes an endpoint_url fixture (replacing the s3_base alias), and its consumers take it as a parameter. With this fix the full suite completes in ~1 minute under -n auto on a 10-core machine, where it previously hung indefinitely. Assisted-by: ClaudeCode:claude-fable-5 * doc: add changelog fragment for moto ephemeral-port fix Assisted-by: ClaudeCode:claude-fable-5 * test: enable pytest strict mode and fix duplicate parametrization ids Adopt pytest 9's `strict = true` config option, which enables strict_config, strict_markers, strict_xfail, strict_parametrization_ids, and future strictness options. This replaces the --strict-config and --strict-markers addopts flags (which pytest silently ignored when passed via addopts before 9.1, pytest#14442) and the separate xfail_strict setting. minversion is bumped to 9 accordingly. strict_parametrization_ids surfaced seven parametrizations whose ids collided, which pytest previously deduplicated silently: - test_common.py paired float("inf") with np.inf (and -inf/nan), but np.inf IS a Python float, so three cases per test ran twice and the intended numpy-scalar coverage did not exist. Use np.float64 scalars with explicit ids, restoring the intended coverage. - test_int.py deliberately includes np.dtype("i")/np.dtype("I"), which compare equal to one of the explicit-endian dtypes and stringify identically. The class-fixture hook in test_dtype/conftest now suffixes colliding ids with their index instead of relying on pytest's silent deduplication. - test_group.py test_consistent_signatures id'd both create_hierarchy pairs identically; ids now include module and qualname. - test_array.py ("a" vs b"a"), test_vlen.py (str vs "str"), and test_metadata/test_v2.py (3 vs "3") get explicit ids for the colliding member. Assisted-by: ClaudeCode:claude-fable-5 * doc: add changelog fragment for pytest strict mode Assisted-by: ClaudeCode:claude-fable-5 * test: arm faulthandler watchdog to turn deadlocks into loud failures Set faulthandler_timeout=600 with faulthandler_exit_on_timeout (new in pytest 9): if a single test exceeds 10 minutes, every thread's traceback is dumped and the run is killed, instead of hanging indefinitely with no diagnostics. Motivated by the moto fixed-port deadlock, which hung local -n auto runs at ~98% for hours and required manual kill -ABRT forensics to diagnose; with this watchdog the same bug would have failed in 10 minutes with the offending stack in the log. The ceiling is ~30x the slowest legitimate test (~20s locally) to stay clear of slow CI runners, coverage overhead, and nightly stateful hypothesis runs. Assisted-by: ClaudeCode:claude-fable-5 --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
1 parent 13279ca commit b4179e4

8 files changed

Lines changed: 53 additions & 16 deletions

File tree

changes/215.misc.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Enable pytest's `strict = true` config option (strict config, markers, xfail, and parametrization ids), replacing the `--strict-config`/`--strict-markers` addopts flags that pytest silently ignored before 9.1, and fix the seven duplicate parametrization ids it surfaced — including restoring float-JSON roundtrip cases that were meant to cover numpy scalars but ran plain-float cases twice instead. Also arm pytest's faulthandler watchdog (`faulthandler_timeout = 600` with `faulthandler_exit_on_timeout`) so a deadlocked test dumps every thread's traceback and fails the run instead of hanging indefinitely.

pyproject.toml

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -426,11 +426,21 @@ module = [
426426
ignore_errors = true
427427

428428
[tool.pytest.ini_options]
429-
minversion = "7"
429+
minversion = "9"
430430
testpaths = ["src", "tests", "docs/user-guide"]
431431
log_cli_level = "INFO"
432432
log_level = "INFO"
433-
xfail_strict = true
433+
# Enables strict_config, strict_markers, strict_xfail, strict_parametrization_ids, and
434+
# any strictness options added in future pytest releases. Note that the equivalent
435+
# `--strict-config`/`--strict-markers` flags were silently ignored when passed via
436+
# addopts before pytest 9.1 (pytest#14442), so this option is the reliable spelling.
437+
strict = true
438+
# Turn deadlocks into loud failures: if a single test exceeds this many seconds, dump
439+
# every thread's traceback and kill the run (exit_on_timeout is new in pytest 9). Sized
440+
# far above the slowest legitimate test (~20s locally; slower under coverage/Windows/
441+
# nightly stateful-hypothesis runs) so only a genuine hang can trip it.
442+
faulthandler_timeout = 600
443+
faulthandler_exit_on_timeout = true
434444
asyncio_mode = "auto"
435445
asyncio_default_fixture_loop_scope = "function"
436446
doctest_optionflags = [
@@ -442,7 +452,7 @@ addopts = [
442452
"--benchmark-columns", "min,mean,stddev,outliers,rounds,iterations",
443453
"--benchmark-disable", # benchmark routines run as tests without benchmarking instrumentation
444454
"--durations", "10",
445-
"-ra", "--strict-config", "--strict-markers",
455+
"-ra",
446456
"--doctest-modules",
447457
"--ignore=tests/test_regression/scripts",
448458
"--ignore=src/zarr/_cli",

tests/test_array.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1693,7 +1693,8 @@ def test_default_endianness(
16931693
assert endianness_from_numpy_str(byte_order) == endianness # type: ignore[arg-type]
16941694

16951695

1696-
@pytest.mark.parametrize("value", [1, 1.4, "a", b"a", np.array(1)])
1696+
# The explicit id for b"a" avoids colliding with the auto-generated id for "a".
1697+
@pytest.mark.parametrize("value", [1, 1.4, "a", pytest.param(b"a", id="a-bytes"), np.array(1)])
16971698
@pytest.mark.parametrize("zarr_format", [2, 3])
16981699
@pytest.mark.filterwarnings("ignore::zarr.core.dtype.common.UnstableSpecificationWarning")
16991700
def test_scalar_array(value: Any, zarr_format: ZarrFormat) -> None:

tests/test_codecs/test_vlen.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@
1313
from zarr.core.metadata.v3 import ArrayV3Metadata
1414
from zarr.storage import StorePath
1515

16-
numpy_str_dtypes: list[type | str | None] = [
16+
# The explicit id for the "str" literal avoids colliding with the auto-generated id for
17+
# the `str` builtin.
18+
numpy_str_dtypes: list[Any] = [
1719
None,
1820
str,
19-
"str",
21+
pytest.param("str", id="str-literal"),
2022
np.dtypes.StrDType,
2123
"S",
2224
"U",

tests/test_dtype/conftest.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# Generate a collection of zdtype instances for use in testing.
22
import warnings
3+
from collections import Counter
34
from typing import Any
45

56
import numpy as np
@@ -64,4 +65,19 @@ class TestB(TestExample):
6465
for fixture_name in metafunc.fixturenames:
6566
if hasattr(metafunc.cls, fixture_name):
6667
params = getattr(metafunc.cls, fixture_name)
67-
metafunc.parametrize(fixture_name, params, scope="class", ids=str)
68+
metafunc.parametrize(
69+
fixture_name, params, scope="class", ids=_unique_ids([str(p) for p in params])
70+
)
71+
72+
73+
def _unique_ids(ids: list[str]) -> list[str]:
74+
"""Suffix repeated ids with their positional index so every id is unique.
75+
76+
Distinct parameters can stringify identically: for example `np.dtype("i")` and
77+
`np.dtype("<i4")` both render as `int32` on little-endian platforms, but they are
78+
deliberately distinct test cases. Pytest used to deduplicate colliding ids silently;
79+
`strict_parametrization_ids` makes them a collection error, so we disambiguate
80+
explicitly. Ids that are already unique are left untouched.
81+
"""
82+
counts = Counter(ids)
83+
return [f"{id_}-{idx}" if counts[id_] > 1 else id_ for idx, id_ in enumerate(ids)]

tests/test_dtype/test_npy/test_common.py

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,17 @@
3636
from zarr.core.common import JSON, ZarrFormat
3737

3838

39-
json_float_v2_roundtrip_cases: tuple[tuple[JSONFloatV2, float | np.floating[Any]], ...] = (
40-
("Infinity", float("inf")),
41-
("Infinity", np.inf),
42-
("-Infinity", float("-inf")),
43-
("-Infinity", -np.inf),
44-
("NaN", float("nan")),
45-
("NaN", np.nan),
46-
(1.0, 1.0),
39+
# Each special value is tested as both a Python float and a numpy scalar. The explicit
40+
# ids are load-bearing: np.float64("inf") stringifies identically to float("inf"), so
41+
# without them these parameter sets would produce duplicate test ids.
42+
json_float_v2_roundtrip_cases: tuple[Any, ...] = (
43+
pytest.param("Infinity", float("inf"), id="Infinity-float"),
44+
pytest.param("Infinity", np.float64("inf"), id="Infinity-float64"),
45+
pytest.param("-Infinity", float("-inf"), id="-Infinity-float"),
46+
pytest.param("-Infinity", np.float64("-inf"), id="-Infinity-float64"),
47+
pytest.param("NaN", float("nan"), id="NaN-float"),
48+
pytest.param("NaN", np.float64("nan"), id="NaN-float64"),
49+
pytest.param(1.0, 1.0, id="1.0-1.0"),
4750
)
4851

4952
json_float_v3_cases = json_float_v2_roundtrip_cases

tests/test_group.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1713,6 +1713,9 @@ def test_create_nodes_concurrency_limit(store: MemoryStore) -> None:
17131713
(zarr.core.group.create_rooted_hierarchy, zarr.core.sync_group.create_rooted_hierarchy),
17141714
(zarr.core.group.get_node, zarr.core.sync_group.get_node),
17151715
],
1716+
# The default ids (from __name__) collide: the method pair and the module-level pair
1717+
# for create_hierarchy would both be id'd "create_hierarchy-create_hierarchy".
1718+
ids=lambda func: f"{func.__module__.rsplit('.', maxsplit=1)[-1]}.{func.__qualname__}",
17161719
)
17171720
def test_consistent_signatures(
17181721
a_func: Callable[[object], object], b_func: Callable[[object], object]

tests/test_metadata/test_v2.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ def test_parse_zarr_format_valid() -> None:
2929
assert parse_zarr_format(2) == 2
3030

3131

32-
@pytest.mark.parametrize("data", [None, 1, 3, 4, 5, "3"])
32+
# The explicit id for "3" avoids colliding with the auto-generated id for the int 3.
33+
@pytest.mark.parametrize("data", [None, 1, 3, 4, 5, pytest.param("3", id="3-str")])
3334
def test_parse_zarr_format_invalid(data: Any) -> None:
3435
with pytest.raises(ValueError, match=f"Invalid value. Expected 2. Got {data}"):
3536
parse_zarr_format(data)

0 commit comments

Comments
 (0)