Skip to content

Add additional regex gtests for contains, count, findall, and replace - #22874

Merged
rapids-bot[bot] merged 2 commits into
NVIDIA:mainfrom
davidwendt:more-regex-tests
Jun 26, 2026
Merged

Add additional regex gtests for contains, count, findall, and replace#22874
rapids-bot[bot] merged 2 commits into
NVIDIA:mainfrom
davidwendt:more-regex-tests

Conversation

@davidwendt

Copy link
Copy Markdown
Contributor

Description

Adds some additional gtests for regex patterns to help validate optimizations in follow on PRs.

This is part of splitting out some of the work for #21936

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@davidwendt davidwendt self-assigned this Jun 12, 2026
@davidwendt davidwendt added 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jun 12, 2026
@copy-pr-bot

copy-pr-bot Bot commented Jun 12, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@davidwendt
davidwendt marked this pull request as ready for review June 12, 2026 18:30
@davidwendt
davidwendt requested a review from a team as a code owner June 12, 2026 18:30
@davidwendt
davidwendt requested review from bdice and mattgara June 12, 2026 18:30
@coderabbitai

coderabbitai Bot commented Jun 12, 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: c8ab7ad0-9610-4116-a022-f815a5b19210

📥 Commits

Reviewing files that changed from the base of the PR and between 0b8ef6a and 8954762.

📒 Files selected for processing (3)
  • cpp/tests/strings/contains_tests.cpp
  • cpp/tests/strings/findall_tests.cpp
  • cpp/tests/strings/replace_regex_tests.cpp

📝 Walkthrough

Summary by CodeRabbit

  • Tests
    • Extended test coverage for regex operations to validate alternation priority, bounded repetition limits, newline character handling, and lazy quantifier behavior across multiple string processing functions.

Walkthrough

This PR extends regex behavior coverage in CUDF string functions by adding unit tests across three test files. Tests validate nullable alternation branches, bounded repetition constraints, newline handling in dot patterns, alternation priority semantics, and lazy quantifier behavior in contains_re, count_re, findall, and replace_re operations.

Changes

String Regex Edge Case Coverage

Layer / File(s) Summary
Fundamental regex semantics in contains_re and count_re
cpp/tests/strings/contains_tests.cpp
Five new test cases validate core regex rules: alternation with nullable branches (a(bc|de|fg|)h), bounded repetition (ab{0,4}cv), dot behavior differences under default vs. EXT_NEWLINE modes, alternation leftmost-first priority (a|aa, foo|foobar), and lazy quantifier shortest-match semantics (x.*?x, x.+?x).
Alternation priority across string operations
cpp/tests/strings/findall_tests.cpp, cpp/tests/strings/replace_regex_tests.cpp
New tests in findall and replace_re verify that alternation leftmost-first matching is consistently applied when alternatives overlap at the same starting position, using patterns like foo|foobar and cat|catch.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~12 minutes


Possibly Related PRs

  • rapidsai/cudf#22429: The new contains_re/count_re test cases with explicitly empty alternation branches (|) exercise the same empty-match handling logic changed in this PR.
  • rapidsai/cudf#22800: The new tests with counted quantifiers at zero minimum (e.g., ab{0,4}cv) exercise the zero-repeat expansion logic fixed in regex_parser::expand_counted_items in regcomp.cpp.

Suggested Reviewers

  • igorpeshansky
  • vyasr
  • wence-
  • qbacpey
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding new regex tests for four string operations (contains, count, findall, and replace).
Description check ✅ Passed The description clearly explains the purpose of the PR—adding regex gtests to validate optimizations—and relates directly to the changeset of new test cases.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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.

@lamarrr lamarrr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@davidwendt

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit d51f715 into NVIDIA:main Jun 26, 2026
134 checks passed
@davidwendt
davidwendt deleted the more-regex-tests branch June 26, 2026 13:29
copy-pr-bot Bot pushed a commit that referenced this pull request Jun 29, 2026
…#22874)

Adds some additional gtests for regex patterns to help validate optimizations in follow on PRs.

This is part of splitting out some of the work for #21936

Authors:
  - David Wendt (https://github.com/davidwendt)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Basit Ayantunde (https://github.com/lamarrr)

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

Labels

3 - Ready for Review Ready for review by team improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants