Skip to content

Commit 6efc235

Browse files
authored
Testing FFI from C side (#7126)
Add Catch2 target and first tests for C API --------- Signed-off-by: Mikhail Kot <mikhail@spiraldb.com>
1 parent 8fd83ab commit 6efc235

File tree

8 files changed

+258
-9
lines changed

8 files changed

+258
-9
lines changed

.github/workflows/ci.yml

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -708,7 +708,7 @@ jobs:
708708
mkdir -p vortex-cxx/build
709709
cmake -S vortex-cxx -B vortex-cxx/build -DVORTEX_ENABLE_TESTING=ON -DVORTEX_ENABLE_ASAN=ON
710710
cmake --build vortex-cxx/build --parallel $(nproc)
711-
ctest --test-dir vortex-cxx/build -V
711+
ctest --test-dir vortex-cxx/build -j $(nproc) -V
712712
- name: Build and run the example in release mode
713713
run: |
714714
cmake -S vortex-cxx/examples -B vortex-cxx/examples/build -DCMAKE_BUILD_TYPE=Release
@@ -821,6 +821,31 @@ jobs:
821821
run: |
822822
find flatbuffers/ -type f -name "*.fbs" | sed 's/^flatbuffers\///' | xargs -I{} -n1 flatc -I flatbuffers.HEAD --conform-includes flatbuffers --conform flatbuffers/{} flatbuffers.HEAD/{}
823823
824+
ffi-c-test:
825+
name: "C API test build"
826+
timeout-minutes: 10
827+
runs-on: >-
828+
${{ github.repository == 'vortex-data/vortex'
829+
&& format('runs-on={0}/runner=amd64-medium/image=ubuntu24-full-x64-pre-v2/tag=cxx-build', github.run_id)
830+
|| 'ubuntu-latest' }}
831+
steps:
832+
- uses: runs-on/action@v2
833+
if: github.repository == 'vortex-data/vortex'
834+
with:
835+
sccache: s3
836+
- uses: actions/checkout@v6
837+
- uses: ./.github/actions/setup-prebuild
838+
- name: "regenerate FFI header file"
839+
run: |
840+
cargo +$NIGHTLY_TOOLCHAIN build -p vortex-ffi
841+
- name: Build and run C++ unit tests
842+
run: |
843+
cd vortex-ffi
844+
mkdir build
845+
cmake -Bbuild
846+
cmake --build build -j $(nproc)
847+
ctest --test-dir build -j $(nproc)
848+
824849
check-java-publish-build:
825850
runs-on: ${{ matrix.target.runs-on }}
826851
container:

vortex-ffi/CMakeLists.txt

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# SPDX-License-Identifier: Apache-2.0
2+
# SPDX-FileCopyrightText: Copyright the Vortex contributors
3+
cmake_minimum_required(VERSION 3.10)
4+
5+
project(VortexFFI
6+
VERSION 0.0.1
7+
LANGUAGES C)
8+
set(CMAKE_C_STANDARD 17)
9+
set(CMAKE_CXX_STANDARD 20)
10+
set(CMAKE_CXX_STANDARD_REQUIRED ON)
11+
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
12+
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Werror -Wextra -Wpedantic")
13+
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Werror -Wextra -Wpedantic")
14+
15+
option(BUILD_TESTING "Build tests" ON)
16+
17+
if (NOT CMAKE_BUILD_TYPE)
18+
set(CMAKE_BUILD_TYPE Debug)
19+
endif()
20+
21+
message(NOTICE "Build type: ${CMAKE_BUILD_TYPE}")
22+
if (NOT CMAKE_BUILD_TYPE STREQUAL "Debug" AND
23+
NOT CMAKE_BUILD_TYPE STREQUAL "Release" AND
24+
NOT CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo")
25+
message(FATAL_ERROR "vortex-ffi can only be built in Release, Debug, or RelWithDebInfo mode")
26+
endif()
27+
28+
if (CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo")
29+
set(LIBRARY_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../target/release_debug")
30+
else()
31+
string(TOLOWER ${CMAKE_BUILD_TYPE} CMAKE_BUILD_TYPE_LOWER)
32+
set(LIBRARY_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../target/${CMAKE_BUILD_TYPE_LOWER}")
33+
endif()
34+
35+
if(WIN32)
36+
set(LIBRARY_PATH "${LIBRARY_DIR}/libvortex_ffi.lib")
37+
set(LIBRARY_PATH_SHARED "${LIBRARY_DIR}/libvortex_ffi.dll")
38+
elseif(APPLE)
39+
set(LIBRARY_PATH "${LIBRARY_DIR}/libvortex_ffi.a")
40+
set(LIBRARY_PATH_SHARED "${LIBRARY_DIR}/libvortex_ffi.dylib")
41+
else()
42+
set(LIBRARY_PATH "${LIBRARY_DIR}/libvortex_ffi.a")
43+
set(LIBRARY_PATH_SHARED "${LIBRARY_DIR}/libvortex_ffi.so")
44+
endif()
45+
46+
set(LIBRARY_HEADERS "${CMAKE_CURRENT_SOURCE_DIR}/cinclude")
47+
48+
message(NOTICE "
49+
Library dir ${LIBRARY_DIR}
50+
Shared library path ${LIBRARY_PATH_SHARED}
51+
Static library path ${LIBRARY_PATH}
52+
Headers path ${LIBRARY_HEADERS}")
53+
54+
if (NOT EXISTS "${LIBRARY_PATH_SHARED}")
55+
message(FATAL_ERROR "Shared library not found")
56+
endif()
57+
if (NOT EXISTS "${LIBRARY_PATH}")
58+
message(FATAL_ERROR "Static library not found")
59+
endif()
60+
61+
add_library(vortex_ffi STATIC IMPORTED)
62+
set_target_properties(vortex_ffi PROPERTIES
63+
IMPORTED_LOCATION "${LIBRARY_PATH}"
64+
INTERFACE_INCLUDE_DIRECTORIES "${LIBRARY_HEADERS}"
65+
)
66+
67+
add_library(vortex_ffi_shared SHARED IMPORTED)
68+
set_target_properties(vortex_ffi_shared PROPERTIES
69+
IMPORTED_LOCATION "${LIBRARY_PATH_SHARED}"
70+
INTERFACE_INCLUDE_DIRECTORIES "${LIBRARY_HEADERS}"
71+
INTERFACE_LINK_OPTIONS "LINKER:-rpath,${LIBRARY_DIR}"
72+
)
73+
74+
if (BUILD_TESTING)
75+
enable_language(CXX)
76+
enable_testing()
77+
add_subdirectory(test)
78+
endif()

vortex-ffi/README.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,18 @@ Stable builds use the checked-in header file at `cinclude/vortex.h`.
4040
- **For header changes**: Use nightly toolchain to regenerate headers after modifying FFI code
4141
- **For regular development**: Stable toolchain builds work with existing checked-in headers
4242
- **CI validation**: Automated checks verify header freshness using nightly toolchain
43+
44+
### Testing
45+
46+
Build the test library
47+
48+
```
49+
cmake -Bbuild
50+
cmake --build build -j $(nproc)
51+
```
52+
53+
Run the tests
54+
55+
```
56+
ctest --test-dir build -j $(nproc)
57+
```

vortex-ffi/cinclude/vortex.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -747,6 +747,13 @@ void vx_session_free(vx_session *ptr);
747747
*/
748748
vx_session *vx_session_new(void);
749749

750+
/**
751+
* Clone a Vortex session, returning an owned copy.
752+
*
753+
* The caller is responsible for freeing the session with [`vx_session_free`].
754+
*/
755+
vx_session *vx_session_clone(const vx_session *session);
756+
750757
/**
751758
* Opens a writable array stream, where sink is used to push values into the stream.
752759
* To close the stream close the sink with `vx_array_sink_close`.

vortex-ffi/src/dtype.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -206,10 +206,9 @@ pub unsafe extern "C-unwind" fn vx_dtype_decimal_scale(dtype: *const vx_dtype) -
206206
pub unsafe extern "C-unwind" fn vx_dtype_struct_dtype(
207207
dtype: *const vx_dtype,
208208
) -> *const vx_struct_fields {
209-
// TODO(joe): propagate this error up instead of expecting
210-
let struct_dtype = vx_dtype::as_ref(dtype)
211-
.as_struct_fields_opt()
212-
.vortex_expect("not a struct dtype");
209+
let Some(struct_dtype) = vx_dtype::as_ref(dtype).as_struct_fields_opt() else {
210+
return ptr::null();
211+
};
213212
vx_struct_fields::new_ref(struct_dtype)
214213
}
215214

@@ -219,10 +218,9 @@ pub unsafe extern "C-unwind" fn vx_dtype_struct_dtype(
219218
/// Do NOT free the returned dtype pointer - it shares the lifetime of the list dtype.
220219
#[unsafe(no_mangle)]
221220
pub unsafe extern "C-unwind" fn vx_dtype_list_element(dtype: *const vx_dtype) -> *const vx_dtype {
222-
// TODO(joe): propagate this error up instead of expecting
223-
let element_dtype = vx_dtype::as_ref(dtype)
224-
.as_list_element_opt()
225-
.vortex_expect("not a list dtype");
221+
let Some(element_dtype) = vx_dtype::as_ref(dtype).as_list_element_opt() else {
222+
return ptr::null();
223+
};
226224
vx_dtype::new_ref(element_dtype)
227225
}
228226

vortex-ffi/src/session.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,12 @@ pub unsafe extern "C-unwind" fn vx_session_new() -> *mut vx_session {
2424
VortexSession::default().with_handle(RUNTIME.handle()),
2525
))
2626
}
27+
28+
/// Clone a Vortex session, returning an owned copy.
29+
///
30+
/// The caller is responsible for freeing the session with [`vx_session_free`].
31+
#[unsafe(no_mangle)]
32+
pub unsafe extern "C-unwind" fn vx_session_clone(session: *const vx_session) -> *mut vx_session {
33+
let session = vx_session::as_ref(session);
34+
vx_session::new(Box::new(session.clone()))
35+
}

vortex-ffi/test/CMakeLists.txt

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# SPDX-License-Identifier: Apache-2.0
2+
# SPDX-FileCopyrightText: Copyright the Vortex contributors
3+
include(CTest)
4+
include(FetchContent)
5+
6+
FetchContent_Declare(
7+
Catch
8+
GIT_REPOSITORY https://github.com/catchorg/Catch2.git
9+
GIT_TAG v3.8.1
10+
)
11+
FetchContent_MakeAvailable(Catch)
12+
include(Catch)
13+
14+
file(GLOB TEST_FILES "${CMAKE_CURRENT_SOURCE_DIR}/*.cpp")
15+
add_executable(vortex_ffi_test ${TEST_FILES})
16+
target_link_libraries(vortex_ffi_test PRIVATE vortex_ffi_shared Catch2::Catch2WithMain)
17+
catch_discover_tests(vortex_ffi_test)

vortex-ffi/test/main.cpp

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
// SPDX-FileCopyrightText: Copyright the Vortex contributors
3+
#include <catch2/catch_test_macros.hpp>
4+
5+
#include "vortex.h"
6+
7+
using namespace std::string_literals;
8+
using namespace std::string_view_literals;
9+
10+
TEST_CASE("Session creation", "[session]") {
11+
vx_session *session = vx_session_new();
12+
REQUIRE(session != nullptr);
13+
vx_session *session2 = vx_session_clone(session);
14+
REQUIRE(session2 != nullptr);
15+
REQUIRE(session != session2);
16+
vx_session_free(session);
17+
vx_session_free(session2);
18+
}
19+
20+
TEST_CASE("Creating and iterating binaries", "[binary]") {
21+
for (std::string_view str : {"ololo"sv, "Широкая строка"sv, "مرحبا بالعالم"sv}) {
22+
const vx_binary *binary = vx_binary_new(str.data(), str.size());
23+
24+
REQUIRE(binary != nullptr);
25+
const size_t len = vx_binary_len(binary);
26+
REQUIRE(len == str.size());
27+
28+
const char *ptr = vx_binary_ptr(binary);
29+
REQUIRE(std::string_view {ptr, len} == str);
30+
31+
const vx_binary *binary2 = vx_binary_clone(binary);
32+
vx_binary_free(binary);
33+
34+
ptr = vx_binary_ptr(binary2);
35+
REQUIRE(std::string_view {ptr, len} == str);
36+
37+
vx_binary_free(binary2);
38+
}
39+
}
40+
41+
TEST_CASE("Creating dtypes", "[dtype]") {
42+
const vx_dtype *dtype = vx_dtype_new_null();
43+
REQUIRE(dtype != nullptr);
44+
CHECK(vx_dtype_get_variant(dtype) == DTYPE_NULL);
45+
CHECK(vx_dtype_is_nullable(dtype));
46+
vx_dtype_free(dtype);
47+
48+
dtype = vx_dtype_new_decimal(5, 2, false);
49+
REQUIRE(dtype != nullptr);
50+
CHECK(vx_dtype_get_variant(dtype) == DTYPE_DECIMAL);
51+
CHECK(vx_dtype_decimal_precision(dtype) == 5);
52+
CHECK(vx_dtype_decimal_scale(dtype) == 2);
53+
CHECK_FALSE(vx_dtype_is_nullable(dtype));
54+
55+
CHECK(vx_dtype_struct_dtype(dtype) == nullptr);
56+
CHECK(vx_dtype_list_element(dtype) == nullptr);
57+
58+
vx_dtype_free(dtype);
59+
}
60+
61+
constexpr size_t STRUCT_LEN = 10;
62+
TEST_CASE("Creating structs", "[struct]") {
63+
vx_struct_fields_builder *builder = vx_struct_fields_builder_new();
64+
REQUIRE(builder != nullptr);
65+
66+
for (size_t i = 0; i < STRUCT_LEN; ++i) {
67+
const std::string target_name = "name"s + std::to_string(i);
68+
const vx_string *name = vx_string_new(target_name.data(), target_name.size());
69+
const vx_dtype *dtype = i % 2 ? vx_dtype_new_binary(false) : vx_dtype_new_primitive(PTYPE_F32, true);
70+
vx_struct_fields_builder_add_field(builder, name, dtype);
71+
}
72+
const vx_struct_fields *fields = vx_struct_fields_builder_finalize(builder);
73+
REQUIRE(fields != nullptr);
74+
75+
const size_t len = vx_struct_fields_nfields(fields);
76+
CHECK(len == STRUCT_LEN);
77+
for (size_t i = 0; i < len; ++i) {
78+
// borrowed
79+
const vx_string *name = vx_struct_fields_field_name(fields, i);
80+
// owned TODO(myrrc): that's weird API
81+
const vx_dtype *dtype = vx_struct_fields_field_dtype(fields, i);
82+
83+
std::string_view name_view {vx_string_ptr(name), vx_string_len(name)};
84+
std::string target_name = "name"s + std::to_string(i);
85+
86+
CHECK(name_view == target_name);
87+
88+
if (i % 2) {
89+
CHECK_FALSE(vx_dtype_is_nullable(dtype));
90+
CHECK(vx_dtype_get_variant(dtype) == DTYPE_BINARY);
91+
} else {
92+
CHECK(vx_dtype_is_nullable(dtype));
93+
CHECK(vx_dtype_get_variant(dtype) == DTYPE_PRIMITIVE);
94+
}
95+
96+
vx_dtype_free(dtype);
97+
}
98+
99+
vx_struct_fields_free(fields);
100+
}

0 commit comments

Comments
 (0)