Skip to content

Skip test_string_agg_aliases on older SQLite versions - #22854

Closed
aryansri05 wants to merge 1 commit into
NVIDIA:mainfrom
aryansri05:fix-polars-tests-sqlite-version-3
Closed

Skip test_string_agg_aliases on older SQLite versions#22854
aryansri05 wants to merge 1 commit into
NVIDIA:mainfrom
aryansri05:fix-polars-tests-sqlite-version-3

Conversation

@aryansri05

Copy link
Copy Markdown
Contributor

Skips test_string_agg_aliases[STRING_AGG] and test_string_agg_aliases[GROUP_CONCAT]
in the cudf-polars-polars test suite by adding them to TESTS_TO_SKIP in
inject_gpu_engine.py.

These tests fail with sqlite3.OperationalError: near "ORDER": syntax error because
the CI environment's SQLite version predates 3.44.0, which is when support for
ORDER BY inside GROUP_CONCAT/STRING_AGG aggregates was introduced. This is a
CI environment constraint, not a bug in cuDF-polars logic.

TODO: remove the skip entries once CI upgrades to SQLite >= 3.44.0.

Closes #22850
Checklist:

I am familiar with the Contributing Guidelines
New or existing tests cover these changes — (N/A: this is a test skip for a CI env limitation)
The documentation is up to date — (N/A: no API/behavior changes)

@aryansri05
aryansri05 requested a review from a team as a code owner June 11, 2026 17:37
@aryansri05
aryansri05 requested a review from Matt711 June 11, 2026 17:37
@copy-pr-bot

copy-pr-bot Bot commented Jun 11, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added Python Affects Python cuDF API. cudf-polars Issues specific to cudf-polars labels Jun 11, 2026
@GPUtester GPUtester moved this to In Progress in cuDF Python Jun 11, 2026
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f173d74f-698d-4a7a-ae44-d23ff546c53f

📥 Commits

Reviewing files that changed from the base of the PR and between 25dbdf4 and 6787cfd.

📒 Files selected for processing (1)
  • python/cudf_polars/cudf_polars/testing/inject_gpu_engine.py

📝 Walkthrough

Summary by CodeRabbit

  • Tests
    • Configured additional test cases to skip during CI runs to ensure compatibility with current environment dependencies.

Walkthrough

The PR adds two SQL test case skip entries to address SQLite version incompatibility in CI. The tests test_string_agg_aliases[STRING_AGG] and test_string_agg_aliases[GROUP_CONCAT] are skipped because the CI SQLite version (< 3.44.0) does not support ORDER BY clauses within string aggregation functions.

Changes

SQL Test Skip Configuration

Layer / File(s) Summary
String aggregation test skip entries
python/cudf_polars/cudf_polars/testing/inject_gpu_engine.py
Two test_string_agg_aliases test variants (STRING_AGG and GROUP_CONCAT) are added to TESTS_TO_SKIP list to skip on CI due to SQLite < 3.44.0 not supporting ORDER BY in string aggregation.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

  • rapidsai/cudf#22605: Also modifies TESTS_TO_SKIP in the same file to exclude SQL test cases incompatible with CI test infrastructure.

Suggested labels

Python, improvement, non-breaking, cudf-polars

Suggested reviewers

  • vyasr
  • jameslamb
  • TomAugspurger
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: skipping test_string_agg_aliases tests on older SQLite versions.
Description check ✅ Passed The description clearly explains why the tests are being skipped, references the CI SQLite version limitation, and provides a TODO for future removal.
Linked Issues check ✅ Passed The PR directly addresses issue #22850 by implementing the requested temporary skip for failing tests due to SQLite version constraints.
Out of Scope Changes check ✅ Passed All changes are scoped to addressing the test failures in issue #22850; only the TESTS_TO_SKIP list is modified.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@TomAugspurger

Copy link
Copy Markdown
Contributor

Thanks @aryansri05. I'm going to handle this in #22870, which does things a bit differently to cover all the failing tests and makes the skip conditional on the version of python / sqlite.

@github-project-automation github-project-automation Bot moved this from In Progress to Done in cuDF Python Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cudf-polars Issues specific to cudf-polars Python Affects Python cuDF API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: test_string_agg_aliases test failure in cudf-polars-polars-tests

3 participants