Skip to content

[Story]: Trim cudf-polars's own test suite down to what upstream polars can't cover #23963

Description

@Matt711

cudf_polars/tests/ (plan node tests), tests/expressions/, and tests/streaming/ together run roughly 2,600 tests, and a large share of them duplicate coverage that already exists in the upstream polars suite (run against the GPU engine in CI). A test that builds a LazyFrame, applies one ordinary polars operation on unremarkable data, and checks GPU output against CPU is already covered upstream, engine-agnostically. What's left after filtering those out is a smaller, genuinely local set: cudf-polars's own unsupported-op/fallback surface, GPU-kernel numerics edge cases (NaN/inf/overflow, decimal precision, dtype boundaries), and our own config/engine/optimizer internals (GPUEngine config, StreamingOptions, join-filter-pushdown, partitioning-request propagation, engine backends). None of this is testable upstream, since polars has no notion of any of it.

A chunk of the streaming-specific tests are also introspective in a way that isn't justified. They assert on hand-built IR node fields or repr strings instead of observable behavior, when a public-API test would cover the same ground just as well.

Goal: prune the duplicated tests, keep (or rewrite to be behavioral) the ones testing something genuinely cudf-polars-specific, and where a test covers real public-API behavior upstream doesn't test and has no cudf-polars dependency, contribute it upstream instead of keeping it local. Running the upstream suite with small partition sizes (--inject-gpu-engine-blocksize=small) already exists as a mechanism and can absorb most multi-partition-correctness tests directly. The exception is scan/IO tests, which are explicitly skipped there for being too slow, a gap that needs its own fix before those tests can be removed. Also worth resolving along the way: the upstream polars job currently runs with raise_on_fail=False, so it doesn't catch CPU-fallback regressions the way local tests do. Deleting local fallback tests without adding fallback tracking to the upstream run would be a net loss of coverage, not a cleanup.

Before deleting anything based on judgment calls about "this looks like standard polars behavior," we should back it with actual coverage data. pytest-cov supports dynamic contexts (--cov-context=test), which records which test covers which line. As a first pass, even a plain line-coverage diff would help: run the polars test job and the cudf-polars test job separately, combine the two coverage reports, and look for cudf-polars lines that only the local job hits. Those are the parts of cudf-polars the local suite is actually covering better, and they're the strongest signal for what should stay local versus what's safe to prune.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    cudf-polarsIssues specific to cudf-polarsimprovementImprovement / enhancement to an existing function

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions