Excercise assert_collect_raises with engine fixture#22563
Conversation
📝 WalkthroughWalkthroughThe PR updates test assertions and execution patterns across four test modules to standardize behavior when running under streaming versus non-streaming GPU engines. Boolean and string tests that were previously skipped on streaming engines are now enabled, while casting and string validation tests are updated to branch error assertions based on engine type. ChangesTest execution parity across engine types
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/cudf_polars/tests/expressions/test_stringfunction.py`:
- Line 595: Wrap the engine-backed call q.collect(engine=engine) in the same
pytest.raises(...) assertion used for the non-engine negative `fill` path so the
test verifies the expected exception for engine execution; locate the test that
exercises `q` and `engine`, replace the bare q.collect(engine=engine) with a
with pytest.raises(ExpectedError): q.collect(engine=engine) (use the same
exception type/class used elsewhere in this test file for the negative `fill`
path).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d0f423c9-991c-4700-ac61-de60cba6ef2f
📒 Files selected for processing (4)
python/cudf_polars/tests/expressions/test_booleanfunction.pypython/cudf_polars/tests/expressions/test_casting.pypython/cudf_polars/tests/expressions/test_datetime_basic.pypython/cudf_polars/tests/expressions/test_stringfunction.py
💤 Files with no reviewable changes (1)
- python/cudf_polars/tests/expressions/test_booleanfunction.py
…lect_raises_engines
…lect_raises_engines
…lect_raises_engines
|
/merge |
Description
The pattern that was replaced in #22535 would only test that collection errors were raised with the in-memory engine.
As apart of #22346, we should ensure that our other engines also exhibit a collection error.
Also checks if tests that were skipped in #21828 no longer segfault
Checklist