From 23c970a38fe29943bf710c7b70e7b17814e1160c Mon Sep 17 00:00:00 2001 From: Robert Maynard Date: Mon, 31 Aug 2026 11:31:41 -0400 Subject: [PATCH] Remove trailing new lines from GTest names This allows for people to use `ctest --tests-from-file` to run these tests since before the newline meant we never had an exact match. --- .coderabbit.yaml | 9 +++++++++ cpp/REVIEW_GUIDELINES.md | 2 ++ cpp/tests/neighbors/all_neighbors.cuh | 2 +- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 302e80599a..65cef37ad8 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -67,6 +67,15 @@ reviews: - Verify GPU availability checks before tests - Check for proper error handling and meaningful error messages + - path: "cpp/tests/**/*" + instructions: | + For GTest parameterized test parameter structs: + - Flag any `operator<<` overload or `PrintTo` function used to stringify test + parameters (i.e. feeds into a GTest test name) that emits `std::endl` or a + literal `"\n"`. GTest builds the parameterized test name from this stream + output, and a trailing newline breaks exact-match tooling such as + `ctest --tests-from-file`. Flag as HIGH priority. + knowledge_base: opt_out: false code_guidelines: diff --git a/cpp/REVIEW_GUIDELINES.md b/cpp/REVIEW_GUIDELINES.md index 1b85aa3fdc..388d261452 100644 --- a/cpp/REVIEW_GUIDELINES.md +++ b/cpp/REVIEW_GUIDELINES.md @@ -79,6 +79,7 @@ ### Test Quality - Missing validation of numerical correctness - **Using external datasets** (tests must not depend on external resources; use synthetic data or bundled datasets) +- **Trailing newline in GTest parameterized test names** (`operator<<` overloads or `PrintTo` functions used to stringify test parameters must not emit `std::endl` or `"\n"`; GTest uses the stream output to build the test name, and a trailing newline breaks exact-match tooling like `ctest --tests-from-file`) ## MEDIUM Issues (Comment Selectively) @@ -304,6 +305,7 @@ cudaStreamCreate(&per_device_stream); - [ ] Are all datasets synthetic or bundled (no external resource dependencies)? - [ ] Is numerical correctness validated? - [ ] Are edge cases tested (empty, single element, extreme values)? +- [ ] If an `operator<<`/`PrintTo` is used to name parameterized GTest cases, does it avoid `std::endl`/`"\n"` (a trailing newline in the generated name breaks `ctest --tests-from-file` exact matching)? --- diff --git a/cpp/tests/neighbors/all_neighbors.cuh b/cpp/tests/neighbors/all_neighbors.cuh index 0b43023eff..3674a52e61 100644 --- a/cpp/tests/neighbors/all_neighbors.cuh +++ b/cpp/tests/neighbors/all_neighbors.cuh @@ -53,7 +53,7 @@ inline ::std::ostream& operator<<(::std::ostream& os, const AllNeighborsInputs& << ", metric=" << static_cast(std::get<1>(p.build_algo_metric_recall)) << ", clusters=" << std::get<0>(p.cluster_nearestcluster) << ", overlap_factor=" << std::get<1>(p.cluster_nearestcluster) - << ", output_on_host=" << p.output_on_host << std::endl; + << ", output_on_host=" << p.output_on_host; return os; }