Refactor cudf-polars test suite onto pytest fixtures by madsbk · Pull Request #22212 · NVIDIA/cudf

madsbk · 2026-04-20T14:39:51Z

Replaces module-level test configuration (DEFAULT_*) with per-test pytest fixtures, and removes the --blocksize-mode CLI option.

Note: The diff is largely mechanical fixture refactoring. The meaningful review surface is limited to:

tests/conftest.py
cudf_polars/testing/asserts.py
CI scripts

Motivation

The test suite currently relies on custom pytest CLI flags (--executor, --cluster, --runtime, --blocksize-mode) that are parsed into module-level globals. Covering multiple configurations requires running pytest multiple times in CI with different flag combinations.

This does not scale with the new frontend model (SPMDEngine, RayEngine, DaskEngine). CLI flags can only express a single configuration per run, and module-level globals make per-test resource setup and teardown awkward (e.g. communicator, RMM, thread pool). Each new engine also expands the CI matrix.

Pytest fixtures map naturally to this. Engines become fixtures with proper lifecycle and scoping, and tests can parametrize over available engines. This removes global state, collapses CI to a single pytest run, and makes it easy to add new engines without touching CI or test configuration.

Replaces module-level test configuration (`DEFAULT_EXECUTOR`, `DEFAULT_RUNTIME`, `DEFAULT_CLUSTER`, `DEFAULT_BLOCKSIZE_MODE`) with per-test pytest fixtures, and removes the `--blocksize-mode` CLI option. **Note:** The diff is largely mechanical fixture refactoring. The meaningful review surface is limited to: * `tests/conftest.py` * `cudf_polars/testing/asserts.py` * CI scripts

gforsyth

Small changes requested in the logging in the CI scripts

mroeschke

Some optional suggestions and a question.

Also agreed with #22212 (comment)

mroeschke · 2026-04-23T19:10:34Z

+_ENGINE_PARAMS = ["in-memory"]
+if importlib.util.find_spec("rapidsmpf") is not None:
+    _ENGINE_PARAMS.append("spmd")
+
+
+@pytest.fixture(params=_ENGINE_PARAMS)


Suggested change

_ENGINE_PARAMS = ["in-memory"]

if importlib.util.find_spec("rapidsmpf") is not None:

_ENGINE_PARAMS.append("spmd")

@pytest.fixture(params=_ENGINE_PARAMS)

@pytest.fixture(params=["in-memory", pytest.param("spmd", marks=pytest.mark.skipif(importlib.util.find_spec("rapidsmpf") is not None, reason="rapidsmpf not installed")

Let’s defer this until rapidsmpf is always available in CI. Right now, this would produce a lot of extra skipped: rapidsmpf not installed lines per run.

mroeschke · 2026-04-23T19:18:19Z

-    --runtime rapidsmpf \
-    --blocksize-mode small
+echo "Running tests"
+timeout 10m python -m pytest --cache-clear "$@" tests --ignore=tests/experimental/legacy


How close are we to collapsing these two pytest calls into ci/run_cudf_polars_pytests.sh?

I suppose a final state of these existing runs should be

With the in-memory executor

With the streaming executor, single cluster with rapidsmpf

With the streaming executor, distributed dask cluster with rapidsmpf

Hopefully we can remove all the legacy code next week.

At that point, we’ll be left with the in-memory engine alongside RayEngine, DaskEngine, and SPMDEngine.

…-tests

Co-authored-by: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com>

…-tests

TomAugspurger

The update CI scripts and conftest files look good.

TomAugspurger · 2026-04-24T20:07:35Z

-    return cudf_polars.testing.asserts.DEFAULT_RUNTIME == "rapidsmpf"
+def using_streaming_engine(engine: pl.GPUEngine) -> bool:
+    """True when the active ``engine`` fixture is an :class:`StreamingEngine`."""
+    try:


I guess this try/except is unavoidable for now, but once it's the default and we depend on rapidsmpf I'd be more comfortable if this errors loudly if there's an import issue.

pentschev

Minor typo suggestion and a couple of comments. LGTM.

pentschev · 2026-04-27T06:53:49Z

+    """Session-scoped communicator — bootstrapped once and shared across all tests.
+
+    Sharing a single communicator avoids the file-based bootstrap race that can
+    cause hangs when ``create_ucxx_comm()`` is called repeatedly in the same
+    ``rrun`` session (stale barrier files / stale ``ucxx_root_address`` KV entry).
+    """


This is fine here, but I wonder if we should change something in rrun or there's nothing we should do there. We will probably be bitten by the same type of issue in every new pytest job that includes rrun. One thing that could be done differently (although that's also on the client/user side) would be to add a barrier at the end of each test, probably not worth the extra cost here though. On rrun's side I can't immediately think of a solution to that, perhaps it would need some care on permitting proper destruction/respawning of the worker being used, this is definitely something that wasn't prioritized.

pentschev · 2026-04-27T07:08:52Z

+    # Ray's internal subprocess management leaks `/dev/null` file handles, and
+    # distributed's shutdown leaves unclosed sockets. Under Python 3.14 +
+    # pytest 9, these surface as unraisable `ResourceWarning`s and — combined
+    # with `filterwarnings = ["error", ...]` in pyproject.toml — fail
+    # otherwise-unrelated tests when the GC finalizer happens to fire during
+    # them. With `pytest-xdist --dist=worksteal`, the leak can land in any
+    # test that shares a worker with a ray/dask test, so the suppression must
+    # apply globally rather than per-module.
+    config.addinivalue_line("filterwarnings", "ignore::ResourceWarning")


The only downside of this is that any legit non-Ray/non-Distributed leaks will be swallowed too.

Co-authored-by: Peter Andreas Entschev <peter@entschev.com>

madsbk · 2026-04-27T08:50:30Z

/merge

madsbk · 2026-04-27T09:05:34Z

/merge

Replaces module-level test configuration (`DEFAULT_*`) with per-test pytest fixtures, and removes the `--blocksize-mode` CLI option. **Note:** The diff is largely mechanical fixture refactoring. The meaningful review surface is limited to: * `tests/conftest.py` * `cudf_polars/testing/asserts.py` * CI scripts --- ### Motivation The test suite currently relies on custom pytest CLI flags (`--executor`, `--cluster`, `--runtime`, `--blocksize-mode`) that are parsed into module-level globals. Covering multiple configurations requires running pytest multiple times in CI with different flag combinations. This does not scale with the new frontend model (`SPMDEngine`, `RayEngine`, `DaskEngine`). CLI flags can only express a single configuration per run, and module-level globals make per-test resource setup and teardown awkward (e.g. communicator, RMM, thread pool). Each new engine also expands the CI matrix. Pytest fixtures map naturally to this. Engines become fixtures with proper lifecycle and scoping, and tests can parametrize over available engines. This removes global state, collapses CI to a single pytest run, and makes it easy to add new engines without touching CI or test configuration. Authors: - Mads R. B. Kristensen (https://github.com/madsbk) - Peter Andreas Entschev (https://github.com/pentschev) Approvers: - Gil Forsyth (https://github.com/gforsyth) - Tom Augspurger (https://github.com/TomAugspurger) - Peter Andreas Entschev (https://github.com/pentschev) - Lawrence Mitchell (https://github.com/wence-) URL: NVIDIA#22212

…#22281) closes #21466 closes #21767 Waiting for #22212 * Makes rapidsmpf a required dependency of cudf_polars * Removes the following `StreamingExecutor` options as they were "experimental" with associated code paths * `StreamingExecutor.runtime` * `StreamingExecutor.shuffle_method` * `StreamingExecutor.unique_fraction` * `StreamingExecutor.groupby_n_ary` * `StreamingExecutor.rapidsmpf_spill` * Removes the task runtime and associated tests * Some tests we modified to only test 1 specific test configuration because of #22346 to pass these tests for now. Planning on revisiting this once rapidsmpf becomes the default Ops-Bot-Merge-Barrier: true Authors: - Matthew Roeschke (https://github.com/mroeschke) Approvers: - Mads R. B. Kristensen (https://github.com/madsbk) - Bradley Dice (https://github.com/bdice) - Matthew Murray (https://github.com/Matt711) - Lawrence Mitchell (https://github.com/wence-) URL: #22281

…NVIDIA#22281) closes NVIDIA#21466 closes NVIDIA#21767 Waiting for NVIDIA#22212 * Makes rapidsmpf a required dependency of cudf_polars * Removes the following `StreamingExecutor` options as they were "experimental" with associated code paths * `StreamingExecutor.runtime` * `StreamingExecutor.shuffle_method` * `StreamingExecutor.unique_fraction` * `StreamingExecutor.groupby_n_ary` * `StreamingExecutor.rapidsmpf_spill` * Removes the task runtime and associated tests * Some tests we modified to only test 1 specific test configuration because of NVIDIA#22346 to pass these tests for now. Planning on revisiting this once rapidsmpf becomes the default Ops-Bot-Merge-Barrier: true Authors: - Matthew Roeschke (https://github.com/mroeschke) Approvers: - Mads R. B. Kristensen (https://github.com/madsbk) - Bradley Dice (https://github.com/bdice) - Matthew Murray (https://github.com/Matt711) - Lawrence Mitchell (https://github.com/wence-) URL: NVIDIA#22281

Replaces module-level test configuration (`DEFAULT_*`) with per-test pytest fixtures, and removes the `--blocksize-mode` CLI option. **Note:** The diff is largely mechanical fixture refactoring. The meaningful review surface is limited to: * `tests/conftest.py` * `cudf_polars/testing/asserts.py` * CI scripts --- ### Motivation The test suite currently relies on custom pytest CLI flags (`--executor`, `--cluster`, `--runtime`, `--blocksize-mode`) that are parsed into module-level globals. Covering multiple configurations requires running pytest multiple times in CI with different flag combinations. This does not scale with the new frontend model (`SPMDEngine`, `RayEngine`, `DaskEngine`). CLI flags can only express a single configuration per run, and module-level globals make per-test resource setup and teardown awkward (e.g. communicator, RMM, thread pool). Each new engine also expands the CI matrix. Pytest fixtures map naturally to this. Engines become fixtures with proper lifecycle and scoping, and tests can parametrize over available engines. This removes global state, collapses CI to a single pytest run, and makes it easy to add new engines without touching CI or test configuration. Authors: - Mads R. B. Kristensen (https://github.com/madsbk) - Peter Andreas Entschev (https://github.com/pentschev) Approvers: - Gil Forsyth (https://github.com/gforsyth) - Tom Augspurger (https://github.com/TomAugspurger) - Peter Andreas Entschev (https://github.com/pentschev) - Lawrence Mitchell (https://github.com/wence-) URL: NVIDIA#22212

…NVIDIA#22281) closes NVIDIA#21466 closes NVIDIA#21767 Waiting for NVIDIA#22212 * Makes rapidsmpf a required dependency of cudf_polars * Removes the following `StreamingExecutor` options as they were "experimental" with associated code paths * `StreamingExecutor.runtime` * `StreamingExecutor.shuffle_method` * `StreamingExecutor.unique_fraction` * `StreamingExecutor.groupby_n_ary` * `StreamingExecutor.rapidsmpf_spill` * Removes the task runtime and associated tests * Some tests we modified to only test 1 specific test configuration because of NVIDIA#22346 to pass these tests for now. Planning on revisiting this once rapidsmpf becomes the default Ops-Bot-Merge-Barrier: true Authors: - Matthew Roeschke (https://github.com/mroeschke) Approvers: - Mads R. B. Kristensen (https://github.com/madsbk) - Bradley Dice (https://github.com/bdice) - Matthew Murray (https://github.com/Matt711) - Lawrence Mitchell (https://github.com/wence-) URL: NVIDIA#22281

madsbk self-assigned this Apr 20, 2026

madsbk added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 20, 2026

github-actions Bot added Python Affects Python cuDF API. cudf-polars Issues specific to cudf-polars labels Apr 20, 2026

github-project-automation Bot added this to cuDF Python Apr 20, 2026

GPUtester moved this to In Progress in cuDF Python Apr 20, 2026

madsbk force-pushed the refactor-tests branch 2 times, most recently from 5e26c86 to 5f9913e Compare April 20, 2026 14:40

madsbk removed this from cuDF Python Apr 20, 2026

madsbk force-pushed the refactor-tests branch 5 times, most recently from 1a38c35 to a464a61 Compare April 21, 2026 13:41

madsbk changed the title ~~[cudf-polar] Introduce engine pytest fixture~~ Refactor cudf-polars test suite onto pytest fixtures Apr 21, 2026

madsbk force-pushed the refactor-tests branch 3 times, most recently from 14fbfb3 to 15e9f36 Compare April 22, 2026 06:06

madsbk force-pushed the refactor-tests branch from 15e9f36 to da13742 Compare April 23, 2026 08:01

madsbk marked this pull request as ready for review April 23, 2026 13:04

madsbk requested review from a team as code owners April 23, 2026 13:04

madsbk requested review from gforsyth and nirandaperera April 23, 2026 13:04

NVIDIA deleted a comment from copy-pr-bot Bot Apr 23, 2026

gforsyth requested changes Apr 23, 2026

View reviewed changes

Comment thread ci/run_cudf_polars_experimental_pytests.sh Outdated

Comment thread ci/run_cudf_polars_experimental_pytests.sh

docs

d4d846a

gforsyth approved these changes Apr 23, 2026

View reviewed changes

Merge branch 'main' into refactor-tests

e7b0865

mroeschke reviewed Apr 23, 2026

View reviewed changes

mroeschke mentioned this pull request Apr 23, 2026

Make RapidsMPF the default runtime for cudf_polars streaming executor #22281

Merged

3 tasks

madsbk and others added 9 commits April 24, 2026 10:30

Merge branch 'refactor-tests' of github.com:madsbk/cudf into refactor…

469d0e0

…-tests

Merge branch 'main' of github.com:rapidsai/cudf into refactor-tests

171df3b

also test with small block sizes

973f1e6

Merge branch 'main' of github.com:rapidsai/cudf into refactor-tests

81c615b

Merge branch 'main' of github.com:rapidsai/cudf into refactor-tests

b2c5f8c

Update python/cudf_polars/tests/conftest.py

ee01a33

Co-authored-by: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com>

Merge branch 'refactor-tests' of github.com:madsbk/cudf into refactor…

6e3e7e3

…-tests

make ignore::ResourceWarning global

76e2c30

_skip_unless_spmd: use find_spec

1b21369

madsbk requested review from mroeschke and wence- April 24, 2026 13:57

TomAugspurger approved these changes Apr 24, 2026

View reviewed changes

pentschev approved these changes Apr 27, 2026

View reviewed changes

madsbk and others added 3 commits April 27, 2026 09:39

Update python/cudf_polars/tests/conftest.py

5be2c4f

Co-authored-by: Peter Andreas Entschev <peter@entschev.com>

Merge branch 'main' of github.com:rapidsai/cudf into refactor-tests

3d41fe9

Merge branch 'main' of github.com:rapidsai/cudf into refactor-tests

25c2e7d

wence- approved these changes Apr 27, 2026

View reviewed changes

rapids-bot Bot merged commit edc4876 into NVIDIA:main Apr 27, 2026
136 of 137 checks passed

madsbk deleted the refactor-tests branch April 27, 2026 10:18

mroeschke mentioned this pull request Apr 30, 2026

[FEA] Change cudf_polars testing assertion function to require an engine argument. #22346

Closed

Uh oh!

Conversation

madsbk commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Uh oh!

gforsyth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mroeschke left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TomAugspurger left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pentschev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

madsbk commented Apr 27, 2026

Uh oh!

madsbk commented Apr 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

madsbk commented Apr 20, 2026 •

edited

Loading