Skip to content

Commit 86187a6

Browse files
committed
Added tests for the FFI library, addressing PR comments
Signed-off-by: barshaul <[email protected]>
1 parent 2f1a4ae commit 86187a6

File tree

9 files changed

+297
-99
lines changed

9 files changed

+297
-99
lines changed

.github/workflows/ffi.yml

-45
This file was deleted.

.github/workflows/rust.yml

+7-3
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,17 @@ jobs:
9999

100100
- uses: Swatinem/rust-cache@v2
101101

102-
- name: Run tests
102+
- name: Run glide-core tests
103103
working-directory: ./glide-core
104-
run: cargo test --all-features --release -- --test-threads=1 # TODO remove the concurrency limit after we fix test flakyness.
104+
run: cargo test --all-features --release
105+
106+
- name: Run glide-ffi tests
107+
working-directory: ./ffi
108+
run: cargo test
105109

106110
- name: Run logger tests
107111
working-directory: ./logger_core
108-
run: cargo test --all-features -- --test-threads=1
112+
run: cargo test --all-features
109113

110114
- name: Check features
111115
working-directory: ./glide-core

.vscode/settings.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
"glide-core/redis-rs/Cargo.toml",
1818
"benchmarks/rust/Cargo.toml",
1919
"java/Cargo.toml",
20-
"go/Cargo.toml",
20+
"ffi/Cargo.toml",
2121
],
2222
"rust-analyzer.runnableEnv": {
2323
"REDISRS_SERVER_TYPE": "tcp"

ffi/Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ redis = { path = "../glide-core/redis-rs/redis", features = ["aio", "tokio-comp"
1414
glide-core = { path = "../glide-core", features = ["proto"] }
1515
tokio = { version = "^1", features = ["rt", "macros", "rt-multi-thread", "time"] }
1616

17+
[dev-dependencies]
18+
glide-ffi = { path = "." }
19+
1720
[profile.release]
1821
lto = true
1922
debug = true

ffi/README.md

+9-4
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,22 @@ The glide_ffi crate provides a C-compatible Foreign Function Interface (FFI) for
1212
## Building the Library
1313

1414
To build the FFI library:
15-
```
15+
```bash
1616
cargo build --release
1717
```
18-
19-
To generate the C header file:
18+
To run tests:
19+
```bash
20+
cargo test
2021
```
22+
23+
## Generating the C Header File
24+
25+
```bash
2126
cargo install cbindgen
2227
cbindgen --config cbindgen.toml --crate glide-ffi --output lib.h --lang c
2328
```
2429

25-
## Running the linters
30+
## Running Linters and Formatting
2631

2732
```bash
2833
rustup component add clippy rustfmt

ffi/src/lib.rs

+16-16
Original file line numberDiff line numberDiff line change
@@ -38,36 +38,36 @@ use tokio::runtime::Runtime;
3838
/// The struct is freed by the external caller by using `free_command_response` to avoid memory leaks.
3939
/// TODO: Add a type enum to validate what type of response is being sent in the CommandResponse.
4040
#[repr(C)]
41-
#[derive(Debug)]
41+
#[derive(Debug, Clone)]
4242
pub struct CommandResponse {
43-
response_type: ResponseType,
44-
int_value: i64,
45-
float_value: c_double,
46-
bool_value: bool,
43+
pub response_type: ResponseType,
44+
pub int_value: i64,
45+
pub float_value: c_double,
46+
pub bool_value: bool,
4747

4848
/// Below two values are related to each other.
4949
/// `string_value` represents the string.
5050
/// `string_value_len` represents the length of the string.
51-
string_value: *mut c_char,
52-
string_value_len: c_long,
51+
pub string_value: *mut c_char,
52+
pub string_value_len: c_long,
5353

5454
/// Below two values are related to each other.
5555
/// `array_value` represents the array of CommandResponse.
5656
/// `array_value_len` represents the length of the array.
57-
array_value: *mut CommandResponse,
58-
array_value_len: c_long,
57+
pub array_value: *mut CommandResponse,
58+
pub array_value_len: c_long,
5959

6060
/// Below two values represent the Map structure inside CommandResponse.
6161
/// The map is transformed into an array of (map_key: CommandResponse, map_value: CommandResponse) and passed to Go.
6262
/// These are represented as pointers as the map can be null (optionally present).
63-
map_key: *mut CommandResponse,
64-
map_value: *mut CommandResponse,
63+
pub map_key: *mut CommandResponse,
64+
pub map_value: *mut CommandResponse,
6565

6666
/// Below two values are related to each other.
6767
/// `sets_value` represents the set of CommandResponse.
6868
/// `sets_value_len` represents the length of the set.
69-
sets_value: *mut CommandResponse,
70-
sets_value_len: c_long,
69+
pub sets_value: *mut CommandResponse,
70+
pub sets_value_len: c_long,
7171
}
7272

7373
impl Default for CommandResponse {
@@ -90,7 +90,7 @@ impl Default for CommandResponse {
9090
}
9191

9292
#[repr(C)]
93-
#[derive(Debug, Default)]
93+
#[derive(Debug, Default, Clone)]
9494
pub enum ResponseType {
9595
#[default]
9696
Null = 0,
@@ -132,8 +132,8 @@ pub type FailureCallback = unsafe extern "C" fn(
132132
/// The struct is freed by the external caller by using `free_connection_response` to avoid memory leaks.
133133
#[repr(C)]
134134
pub struct ConnectionResponse {
135-
conn_ptr: *const c_void,
136-
connection_error_message: *const c_char,
135+
pub conn_ptr: *const c_void,
136+
pub connection_error_message: *const c_char,
137137
}
138138

139139
/// A `GlideClient` adapter.

0 commit comments

Comments
 (0)