Add C API tests for error handling, indexing, and search parameters by rfsaliev · Pull Request #306 · intel/ScalableVectorSearch

rfsaliev · 2026-03-27T16:11:21Z

This pull request introduces a comprehensive C API test suite for the SVS project, leveraging the Catch2 testing framework. It adds new test files covering all major C API functionalities, integrates automated test building and execution into the CMake build system, and improves error handling and testability for dynamic index operations.

C API Test Infrastructure and Test Coverage:

Added a new directory of C API tests using Catch2, with individual test files for error handling, algorithm configuration, storage, search parameters, index building, and dynamic index operations. Each test file is organized by functionality and follows consistent patterns for assertions and resource cleanup. [1] [2] [3]
Introduced a CMakeLists.txt for the test suite that automatically fetches Catch2 if not found, builds the test executable, and integrates with CTest for automated test discovery and execution.
Added a detailed README.md explaining test structure, coverage, build/run instructions, and contribution guidelines for new tests.

Build System Improvements:

Enabled building of C API tests by default with a new SVS_BUILD_C_API_TESTS CMake option, and added logic to include the tests subdirectory conditionally.

Dynamic Index Error Handling:

Refactored svs_index_dynamic_delete_points to improve error handling: split the operation into two exception-wrapped phases, ensuring correct error reporting when deleting points from a dynamic index.

…rch parameters, and storage - Introduced tests for error handling in the C API, ensuring proper creation, cleanup, and state management of error handles. - Implemented tests for index building and searching, covering various scenarios including different metrics, storage types, and threadpool configurations. - Added tests for search parameters creation and validation, including handling of invalid sizes. - Developed tests for storage creation, validating various data types and ensuring proper error handling for invalid arguments. - Ensured all tests utilize the Catch2 framework for consistency and clarity.

Copilot

Pull request overview

This PR adds a new Catch2-based test suite for the SVS C API, wires it into the CMake/CTest build, and refactors dynamic-index deletion to improve error reporting.

Changes:

Added multiple Catch2 test files covering C API error handling, algorithms, storage, search params, index building/search, and dynamic index operations.
Added bindings/c/tests/CMakeLists.txt and bindings/c/tests/README.md to build and run the new C API tests via CTest.
Refactored svs_index_dynamic_delete_points to split validation and deletion into separate exception-wrapped phases.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 15 comments.

Show a summary per file

File	Description
bindings/c/tests/c_api_storage.cpp	Adds storage-handle tests (Simple/LeanVec/LVQ/SQ).
bindings/c/tests/c_api_search_params.cpp	Adds search-parameter creation tests and invalid-size coverage.
bindings/c/tests/c_api_index_builder.cpp	Adds index-builder creation/config tests (metric/storage/threadpool).
bindings/c/tests/c_api_index.cpp	Adds build/search tests plus distance/reconstruct and threadpool management checks.
bindings/c/tests/c_api_error.cpp	Adds basic error-handle lifecycle and invalid-call behavior tests.
bindings/c/tests/c_api_dynamic_index.cpp	Adds dynamic index build/add/delete/consolidate/compact/search tests.
bindings/c/tests/README.md	Documents test structure and build/run instructions.
bindings/c/tests/CMakeLists.txt	Builds the C API test executable and integrates with CTest/Catch discovery.
bindings/c/src/svs_c.cpp	Refactors `svs_index_dynamic_delete_points` exception handling.
bindings/c/CMakeLists.txt	Introduces `SVS_BUILD_C_API_TESTS` option and conditionally adds the tests subdir.

Copilot · 2026-03-27T16:23:46Z

bindings/c/tests/c_api_index.cpp

+        // Verify distance is approximately correct
+        float expected_distance =
+            euclidean_distance(data.data(), queries.data(), DIMENSION);
+        CATCH_REQUIRE(std::abs(distance - expected_distance) < 0.1f);


The expected value in the “Index Get Distance” section is computed as a true Euclidean distance (includes sqrt), but the SVS L2 metric is squared-L2 (see svs::distance::DistanceL2 / DistanceType::L2). This will make the test fail (or, worse, encode the wrong contract). Compute the expected distance using squared L2 (no sqrt) or rename the helper to reflect squared distance and compare accordingly.

Copilot · 2026-03-27T16:23:46Z

bindings/c/tests/c_api_storage.cpp

+        CATCH_REQUIRE(storage != nullptr);
+        CATCH_REQUIRE(svs_error_ok(error) == true);


This LeanVec(UINT4/UINT4) case also needs to handle the optional/unsupported scenarios: svs_storage_create_leanvec can legitimately fail with SVS_ERROR_NOT_IMPLEMENTED or SVS_ERROR_UNSUPPORTED_HW depending on build flags and CPU support. As written, the test will fail those configurations instead of skipping/validating the expected error code.

Suggested change

CATCH_REQUIRE(storage != nullptr);

CATCH_REQUIRE(svs_error_ok(error) == true);

if (storage != nullptr) {

CATCH_REQUIRE(svs_error_ok(error) == true);

} else {

CATCH_REQUIRE_FALSE(svs_error_ok(error));

auto code = svs_error_get_code(error);

CATCH_REQUIRE(

code == SVS_ERROR_NOT_IMPLEMENTED || code == SVS_ERROR_UNSUPPORTED_HW

);

}

Copilot · 2026-03-27T16:23:47Z

bindings/c/tests/CMakeLists.txt

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set(TARGET_NAME svs_c_api_test)


The test executable target is named svs_c_api_test, but the README/build instructions in this PR refer to svs_c_api_tests (plural). This mismatch will break the documented make/run commands and CTest -R filtering. Consider renaming the target (and add_test name) to match the documented name, or update the README consistently.

Suggested change

set(TARGET_NAME svs_c_api_test)

set(TARGET_NAME svs_c_api_tests)

Copilot · 2026-03-27T16:23:47Z

bindings/c/src/svs_c.cpp

+    return wrap_exceptions(
+        [&]() { return dynamic_index_ptr->delete_points(std::span(ids, num_ids)); },
+        out_err,
+        0


svs_index_dynamic_delete_points now uses a second wrap_exceptions(..., 0) call. If out_err is NULL and delete_points() throws, the function will return 0 (which is also the valid “deleted 0 points” result), violating the header contract that failures return (size_t)-1 and potentially hiding errors from callers that don’t pass an error handle. Use an error sentinel (e.g., (size_t)-1) for the second wrap_exceptions as well (or revert to a single wrap_exceptions call) so failures remain distinguishable even when out_err == NULL.

Suggested change

0

static_cast<size_t>(-1)

Copilot · 2026-03-27T16:23:47Z

bindings/c/tests/c_api_index_builder.cpp

+        // Try to create with 0 dimension
+        svs_index_builder_h builder =
+            svs_index_builder_create(SVS_DISTANCE_METRIC_EUCLIDEAN, 0, algorithm, error);
+        // Behavior depends on implementation
+        if (builder != nullptr) {
+            svs_index_builder_free(builder);
+        }
+


The “Invalid Parameters” section treats svs_index_builder_create(..., dimension=0, ...) as implementation-defined, but the implementation explicitly requires dimension > 0 and should return nullptr with SVS_ERROR_INVALID_ARGUMENT. Update the test to assert the failure path and verify the error code/message so regressions are caught.

Suggested change

// Try to create with 0 dimension

svs_index_builder_h builder =

svs_index_builder_create(SVS_DISTANCE_METRIC_EUCLIDEAN, 0, algorithm, error);

// Behavior depends on implementation

if (builder != nullptr) {

svs_index_builder_free(builder);

}

// Try to create with 0 dimension: this must fail with INVALID_ARGUMENT

svs_index_builder_h builder =

svs_index_builder_create(SVS_DISTANCE_METRIC_EUCLIDEAN, 0, algorithm, error);

CATCH_REQUIRE(builder == nullptr);

CATCH_REQUIRE(svs_error_ok(error) == false);

auto code = svs_error_get_code(error);

CATCH_REQUIRE(code == SVS_ERROR_INVALID_ARGUMENT);

const char* msg = svs_error_message(error);

CATCH_REQUIRE(msg != nullptr);

CATCH_REQUIRE(msg[0] != '\0');

Copilot · 2026-03-27T16:23:49Z

bindings/c/tests/README.md

+The tests are built as part of the C API build process. To build them:
+
+```bash
+# From the build directory
+cmake -DSVS_BUILD_C_API_TESTS=ON ..
+make svs_c_api_tests
+```


The build/run commands reference svs_c_api_tests (plural), but bindings/c/tests/CMakeLists.txt currently creates svs_c_api_test (singular). Update the README commands to match the actual target/executable name (or rename the target) so the instructions work as-written.

Copilot · 2026-03-27T16:23:49Z

bindings/c/tests/c_api_dynamic_index.cpp

+        size_t non_existing_id = NUM_VECTORS + 1000;
+        size_t deleted_count =
+            svs_index_dynamic_delete_points(index, &non_existing_id, 1, error);
+        // Should return 0 for non-existing ID


In “Dynamic Index Delete Non-existing ID”, the test asserts only deleted_count == 0 but doesn’t verify svs_error_ok(error). Since 0 is also a plausible fallback value in some error paths (and the implementation of svs_index_dynamic_delete_points was recently refactored), add an explicit svs_error_ok(error) / error-code check here to ensure this is a successful “not found” deletion rather than a silent failure.

Suggested change

// Should return 0 for non-existing ID

// Should return 0 for non-existing ID and no error

CATCH_REQUIRE(svs_error_ok(error));

Copilot · 2026-03-27T16:23:50Z

bindings/c/tests/c_api_storage.cpp

+    CATCH_SECTION("LVQ Storage UINT8") {
+        svs_error_h error = svs_error_create();
+
+        svs_storage_h storage =
+            svs_storage_create_lvq(SVS_DATA_TYPE_UINT8, SVS_DATA_TYPE_VOID, error);
+        CATCH_REQUIRE(storage != nullptr);
+        CATCH_REQUIRE(svs_error_ok(error) == true);
+


Like the other LVQ tests, this section assumes svs_storage_create_lvq always succeeds. LVQ can legitimately be unavailable (SVS_ERROR_NOT_IMPLEMENTED) or hardware-gated (SVS_ERROR_UNSUPPORTED_HW), so the test should skip or assert the expected error code in those cases instead of hard-failing on storage == nullptr.

Copilot · 2026-03-27T16:23:50Z

bindings/c/tests/c_api_storage.cpp

+        CATCH_REQUIRE(storage != nullptr);
+        CATCH_REQUIRE(svs_error_ok(error) == true);
+


This LVQ-with-residual section should also account for builds/CPUs where LVQ is not implemented or supported. Otherwise, a configuration that correctly returns SVS_ERROR_NOT_IMPLEMENTED / SVS_ERROR_UNSUPPORTED_HW will incorrectly fail the test suite.

Suggested change

CATCH_REQUIRE(storage != nullptr);

CATCH_REQUIRE(svs_error_ok(error) == true);

if (storage == nullptr) {

CATCH_REQUIRE_FALSE(svs_error_ok(error));

auto code = svs_error_get_code(error);

CATCH_REQUIRE(

code == SVS_ERROR_NOT_IMPLEMENTED || code == SVS_ERROR_UNSUPPORTED_HW

);

} else {

CATCH_REQUIRE(svs_error_ok(error) == true);

}

Copilot · 2026-03-27T16:23:50Z

bindings/c/CMakeLists.txt

-# if(SVS_BUILD_C_API_TESTS)
-#     add_subdirectory(tests)
-# endif()
+option(SVS_BUILD_C_API_TESTS "Build C API tests" ON)


SVS_BUILD_C_API_TESTS defaults to ON, which means building the C API will build the test executable (and potentially FetchContent-download Catch2) by default. This differs from the repo’s other test knobs (e.g., SVS_BUILD_TESTS / SVS_BUILD_RUNTIME_TESTS, which are opt-in) and can be surprising for consumers embedding bindings/c. Consider defaulting this option to OFF (or tying it to the parent project’s test option) so tests are only built when explicitly requested.

Suggested change

option(SVS_BUILD_C_API_TESTS "Build C API tests" ON)

if(DEFINED SVS_BUILD_TESTS)

option(SVS_BUILD_C_API_TESTS "Build C API tests" ${SVS_BUILD_TESTS})

else()

option(SVS_BUILD_C_API_TESTS "Build C API tests" OFF)

endif()

rfsaliev added 2 commits March 27, 2026 16:07

Refactor CMake configuration for C API tests

46038d6

rfsaliev requested a review from Copilot March 27, 2026 16:11

Copilot started reviewing on behalf of rfsaliev March 27, 2026 16:13 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add C API tests for error handling, indexing, and search parameters#306

Add C API tests for error handling, indexing, and search parameters#306
rfsaliev wants to merge 2 commits intorfsaliev/c-api-index-num-threadsfrom
rfsaliev/c-api-tests

rfsaliev commented Mar 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		CATCH_REQUIRE(storage != nullptr);
		CATCH_REQUIRE(svs_error_ok(error) == true);

-        CATCH_REQUIRE(storage != nullptr);
-        CATCH_REQUIRE(svs_error_ok(error) == true);
+        if (storage != nullptr) {
+            CATCH_REQUIRE(svs_error_ok(error) == true);
+        } else {
+            CATCH_REQUIRE_FALSE(svs_error_ok(error));
+            auto code = svs_error_get_code(error);
+            CATCH_REQUIRE(
+                code == SVS_ERROR_NOT_IMPLEMENTED || code == SVS_ERROR_UNSUPPORTED_HW
+            );
+        }

	set(TARGET_NAME svs_c_api_test)
	set(TARGET_NAME svs_c_api_tests)

	// Should return 0 for non-existing ID
	// Should return 0 for non-existing ID and no error
	CATCH_REQUIRE(svs_error_ok(error));

-option(SVS_BUILD_C_API_TESTS "Build C API tests" ON)
+if(DEFINED SVS_BUILD_TESTS)
+    option(SVS_BUILD_C_API_TESTS "Build C API tests" ${SVS_BUILD_TESTS})
+else()
+    option(SVS_BUILD_C_API_TESTS "Build C API tests" OFF)
+endif()

Conversation

rfsaliev commented Mar 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants