Create separate test run scripts #722

bdice · 2025-12-16T22:28:24Z

Description

This PR creates separate test run scripts like ci/run_ctests.sh. This aligns with the pattern used in RAPIDS libraries. I am exploring adding devcontainers to cuOpt (rapidsai/devcontainers#637), and this change would help with allowing tests to run in both CI and devcontainer environments.

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
- Refactored continuous integration test execution workflow by introducing dedicated test runner scripts for C++ and Python test suites, improving test execution consistency and maintainability across different testing environments.

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

coderabbitai · 2025-12-16T22:33:07Z

📝 Walkthrough

Walkthrough

New Bash wrapper scripts are added to encapsulate test execution for Google Test binaries and Python test suites. Existing CI test scripts are refactored to delegate test execution to these wrapper scripts instead of performing inline execution, improving modularity and consistency across the CI pipeline.

Changes

Cohort / File(s)	Summary
New test runner scripts `ci/run_ctests.sh`, `ci/run_cuopt_pytests.sh`, `ci/run_cuopt_server_pytests.sh`	Adds three new Bash scripts to encapsulate test execution. `run_ctests.sh` discovers and runs Google Test binaries sequentially from INSTALL_PREFIX or CONDA_PREFIX. `run_cuopt_pytests.sh` and `run_cuopt_server_pytests.sh` execute pytest with cache clearing in their respective package directories. All enable strict shell options and forward arguments to underlying test commands.
C++ test CI runner `ci/test_cpp.sh`	Refactors to invoke `./ci/run_ctests.sh` instead of inline GTest loop execution. Sets `GTEST_OUTPUT=xml` environment variable. Increases timeout from 20m to 40m.
Python test CI runners `ci/test_python.sh`, `ci/test_wheel_cuopt.sh`, `ci/test_wheel_cuopt_server.sh`	Refactor to delegate test execution to wrapper scripts (`run_cuopt_pytests.sh` and `run_cuopt_server_pytests.sh`) instead of direct pytest invocations. Removes explicit directory changes and redundant flag specifications previously handled inline.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

New scripts are straightforward bash wrappers with consistent patterns (shebang, strict options, directory changes, command delegation).
Modifications across existing test scripts follow a homogeneous refactoring pattern (replacing inline execution with wrapper script calls).
All changes are in CI/shell scripting with minimal logic density and no complex control flow.
High repetition of similar changes reduces per-file review cognitive load.

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: introducing separate test run scripts (ci/run_ctests.sh, ci/run_cuopt_pytests.sh, ci/run_cuopt_server_pytests.sh) to consolidate test execution logic.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

ci/run_ctests.sh (1)
24-28: Consider adding glob failure handling for edge cases.

If GTEST_DIR exists but contains no *_TEST files, the glob won't expand and the loop will attempt to execute a literal *_TEST command, producing an unclear error message. This is unlikely in practice but could be handled for robustness.

Add this check before the loop:
+# Ensure test binaries exist
+shopt -s nullglob
 for gt in "${GTEST_DIR}"/*_TEST; do
     test_name=$(basename "${gt}")
     echo "Running gtest ${test_name}"
     "${gt}" "$@"
 done
+shopt -u nullglob
Or alternatively, add an explicit check:
+if ! compgen -G "${GTEST_DIR}/*_TEST" > /dev/null; then
+    echo "Error: No test binaries found matching ${GTEST_DIR}/*_TEST" >&2
+    exit 1
+fi
 for gt in "${GTEST_DIR}"/*_TEST; do
     test_name=$(basename "${gt}")
     echo "Running gtest ${test_name}"
     "${gt}" "$@"
 done

📜 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 f44e3ec and 21df01f.

📒 Files selected for processing (7)

ci/run_ctests.sh (1 hunks)
ci/run_cuopt_pytests.sh (1 hunks)
ci/run_cuopt_server_pytests.sh (1 hunks)
ci/test_cpp.sh (1 hunks)
ci/test_python.sh (1 hunks)
ci/test_wheel_cuopt.sh (1 hunks)
ci/test_wheel_cuopt_server.sh (1 hunks)

🧰 Additional context used

🧠 Learnings (7)

📓 Common learnings

Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*test*.{cpp,cu,py} : Ensure test isolation: prevent GPU state, cached memory, and global variables from leaking between test cases; verify each test independently initializes its environment