Skip to content

Conversation

@bdice
Copy link
Contributor

@bdice bdice commented Dec 18, 2025

Description

This switches cuOpt to fetch GTest with CPM and statically link it into the test executables. This removes the conda dependency on gtest and aligns with what we do in cuDF and other RAPIDS libraries.

Checklist

  • I am familiar with the Contributing Guidelines.
  • Testing
    • New or existing tests cover these changes
    • Added tests
    • Created an issue to follow-up
    • NA
  • Documentation
    • The documentation is up to date with these changes
    • Added new documentation
    • NA

Summary by CodeRabbit

  • Chores
    • Removed gtest/gmock testing framework dependencies from Conda environments, recipes, and shared dependency manifests.
    • Updated GTest integration to build as a static library and removed the external test-version pinning.

✏️ Tip: You can customize this high-level summary in your review settings.

@bdice bdice requested review from a team as code owners December 18, 2025 02:40
@bdice bdice requested review from gforsyth and tmckayus December 18, 2025 02:40
@bdice bdice added non-breaking Introduces a non-breaking change improvement Improves an existing functionality labels Dec 18, 2025
@bdice bdice self-assigned this Dec 18, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 18, 2025

📝 Walkthrough

Walkthrough

Removed gmock and gtest from multiple conda environment and recipe files, removed gtest_version from a conda build config, and changed the GTest CMake hook to call rapids_cpm_gtest() with BUILD_STATIC.

Changes

Cohort / File(s) Summary
Conda environment configurations
conda/environments/all_cuda-129_arch-aarch64.yaml, conda/environments/all_cuda-129_arch-x86_64.yaml, conda/environments/all_cuda-130_arch-aarch64.yaml, conda/environments/all_cuda-130_arch-x86_64.yaml
Removed gmock and gtest from the dependencies lists in all four environment files.
Conda recipe configuration
conda/recipes/libcuopt/conda_build_config.yaml
Removed the gtest_version entry from the conda build configuration.
Conda recipe specification
conda/recipes/libcuopt/recipe.yaml
Removed gmock and gtest entries from host requirements, outputs, run, and ignore_run_exports sections across recipe outputs.
Build configuration (CMake)
cpp/cmake/thirdparty/get_gtest.cmake
Updated call to rapids_cpm_gtest() to pass BUILD_STATIC, changing the GTest build mode to static.
Dependency manifest
dependencies.yaml
Removed gmock and gtest from dependencies/test_cpp/common.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Check that removing gtest_version doesn't break references in other recipes or CI scripts.
  • Verify downstream build/test behavior where gmock/gtest were previously expected (CI, local dev, packaging).
  • Confirm the BUILD_STATIC change in get_gtest.cmake yields intended linkage and doesn't affect platforms differently (ARM vs x86_64).

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: switching to static GTest linking and removing conda dependencies on gtest/gmock.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5351bb2 and c3bc13c.

📒 Files selected for processing (5)
  • conda/environments/all_cuda-129_arch-aarch64.yaml
  • conda/environments/all_cuda-129_arch-x86_64.yaml
  • conda/environments/all_cuda-130_arch-aarch64.yaml
  • conda/environments/all_cuda-130_arch-x86_64.yaml
  • dependencies.yaml
💤 Files with no reviewable changes (5)
  • conda/environments/all_cuda-130_arch-aarch64.yaml
  • conda/environments/all_cuda-129_arch-x86_64.yaml
  • conda/environments/all_cuda-129_arch-aarch64.yaml
  • dependencies.yaml
  • conda/environments/all_cuda-130_arch-x86_64.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.12, arm64, rockylinux8
  • GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.13, arm64, rockylinux8
  • GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.12, amd64, rockylinux8
  • GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.11, amd64, rockylinux8
  • GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.11, arm64, rockylinux8
  • GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.10, amd64, rockylinux8
  • GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.10, arm64, rockylinux8
  • GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.13, amd64, rockylinux8

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions
Copy link

🔔 Hi @anandhkb @bdice, this pull request has had no activity for 7 days. Please update or let us know if it can be closed. Thank you!

If this is an "epic" issue, then please add the "epic" label to this issue.
If it is a PR and not ready for review, then please convert this to draft.
If you just want to switch off this notification, then use the "skip inactivity reminder" label.

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

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants