Skip to content

Commit 7988091

Browse files
committed
fix(nim-bindings): add all-endpoints test and fix installation_name ABI
Add nim-bindings/tests/test_all_endpoints.nim which imports bindings directly and calls every FFI proc, forcing the linker to include all symbols. This catches link-time and runtime issues that the pingpong example missed because unused symbols were optimised out. Running the new test revealed an ABI mismatch in installation_name: the Rust function used an explicit out-parameter but ReprCString has only flat fields, so Nim emits it as a C return value. CI now runs nimble test instead of nimble pingpong.
1 parent fa79b1c commit 7988091

6 files changed

Lines changed: 283 additions & 6 deletions

File tree

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,5 +52,5 @@ jobs:
5252
echo "$HOME/.nimble/bin" >> $GITHUB_PATH
5353
- run: nimble install -dy
5454
working-directory: nim-bindings
55-
- run: nimble pingpong
55+
- run: nimble test
5656
working-directory: nim-bindings

conversations/src/api.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,9 @@ pub fn create_context(name: repr_c::String) -> repr_c::Box<ContextHandle> {
5656

5757
/// Returns the friendly name of the contexts installation.
5858
///
59-
/// # ABI note
60-
/// The result is written through `out` (Nim's calling convention for large struct returns).
6159
#[ffi_export]
62-
pub fn installation_name(ctx: &ContextHandle, out: &mut repr_c::String) {
63-
*out = ctx.0.installation_name().to_string().into();
60+
pub fn installation_name(ctx: &ContextHandle) -> repr_c::String {
61+
ctx.0.installation_name().to_string().into()
6462
}
6563

6664
/// Destroys a conversation store and frees its memory
@@ -74,6 +72,17 @@ pub fn destroy_context(ctx: repr_c::Box<ContextHandle>) {
7472
drop(ctx);
7573
}
7674

75+
/// Destroys a repr_c::String and frees its memory
76+
///
77+
/// # Safety
78+
/// - s must be a valid pointer from an FFI function that returns repr_c::String
79+
/// - s must not be used after this call
80+
/// - s must not be freed twice
81+
#[ffi_export]
82+
pub fn destroy_string(s: repr_c::String) {
83+
drop(s);
84+
}
85+
7786
/// Creates an intro bundle for sharing with other users
7887
///
7988
/// # Returns
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,7 @@ before build:
2424
task pingpong, "Run pingpong example":
2525
buildRust()
2626
exec "nim c -r --path:src --passL:../target/release/liblibchat.a --passL:-lm examples/pingpong.nim"
27+
28+
task test, "Run comprehensive all-endpoints test":
29+
buildRust()
30+
exec "nim c -r --path:src --passL:../target/release/liblibchat.a --passL:-lm tests/test_all_endpoints.nim"

nim-bindings/src/bindings.nim

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,11 @@ proc installation_name*(ctx: ContextHandle): ReprCString {.importc.}
8484
## - handle must not be used after this call
8585
proc destroy_context*(ctx: ContextHandle) {.importc.}
8686

87+
## Free a ReprCString returned by any of the FFI functions
88+
## - s must be a valid pointer from an FFI function that returns repr_c::String
89+
## - s must not be used after this call
90+
proc destroy_string*(s: var ReprCString) {.importc.}
91+
8792
## Creates an intro bundle for sharing with other users
8893
## Returns: CreateIntroResult struct - check error_code field (0 = success, negative = error)
8994
## The result must be freed with destroy_intro_result()

nim-bindings/src/libchat.nim

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ proc newConversationsContext*(name: string): LibChat =
2424
proc getInstallationName*(ctx: LibChat): string =
2525
if ctx.handle == nil:
2626
return ""
27-
let name = installation_name(ctx.handle)
27+
var name = installation_name(ctx.handle)
28+
defer: destroy_string(name)
2829
result = $name
2930

3031
## Destroy the context and free resources
Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,258 @@
1+
# Comprehensive test for all FFI procs declared in bindings.nim.
2+
#
3+
# Design intent: By importing `bindings` directly and calling every importc
4+
# proc at least once, the linker is forced to include ALL symbol references.
5+
# This prevents link-time optimizations from stripping unused symbols and
6+
# catches both link-time crashes (missing symbols) and runtime crashes
7+
# (wrong ABI, segfaults on use).
8+
9+
import bindings
10+
11+
# ---------------------------------------------------------------------------
12+
# Assertion helper
13+
# ---------------------------------------------------------------------------
14+
15+
proc check(cond: bool, msg: string) =
16+
if not cond:
17+
echo "FAIL: ", msg
18+
quit(1)
19+
echo "OK: ", msg
20+
21+
# ---------------------------------------------------------------------------
22+
# Section 1: Helper proc coverage
23+
# ---------------------------------------------------------------------------
24+
25+
proc testHelperProcs() =
26+
echo "\n--- testHelperProcs ---"
27+
28+
# toSlice(string) — non-empty and empty branches
29+
let s = "hello"
30+
let sl = toSlice(s)
31+
check(sl.len == 5, "toSlice(string): correct len")
32+
check(sl.`ptr` != nil, "toSlice(non-empty string): non-nil ptr")
33+
34+
let emptySl = toSlice("")
35+
check(emptySl.len == 0, "toSlice(empty string): len == 0")
36+
check(emptySl.`ptr` == nil, "toSlice(empty string): ptr == nil")
37+
38+
# toSlice(seq[byte]) — non-empty and empty branches
39+
let b: seq[byte] = @[0x61'u8, 0x62'u8, 0x63'u8]
40+
let bSl = toSlice(b)
41+
check(bSl.len == 3, "toSlice(seq[byte]): correct len")
42+
check(bSl.`ptr` != nil, "toSlice(non-empty seq[byte]): non-nil ptr")
43+
44+
let emptyBSl = toSlice(newSeq[byte](0))
45+
check(emptyBSl.len == 0, "toSlice(empty seq[byte]): len == 0")
46+
check(emptyBSl.`ptr` == nil, "toSlice(empty seq[byte]): ptr == nil")
47+
48+
# toReprCString(string) and $(ReprCString) round-trip
49+
let name = "testname"
50+
let rcs = toReprCString(name)
51+
check(rcs.len == csize_t(name.len), "toReprCString: correct len")
52+
check(rcs.cap == 0, "toReprCString: cap == 0 (prevents Rust dealloc of Nim memory)")
53+
check(rcs.`ptr` != nil, "toReprCString: non-nil ptr")
54+
check($rcs == name, "$(ReprCString): round-trips to original string")
55+
56+
let emptyRcs = toReprCString("")
57+
check(emptyRcs.len == 0, "toReprCString(empty): len == 0")
58+
check($emptyRcs == "", "$(empty ReprCString): returns empty string")
59+
60+
# toBytes(string)
61+
let bs = toBytes("abc")
62+
check(bs.len == 3, "toBytes: correct length")
63+
check(bs[0] == 0x61'u8, "toBytes: correct first byte")
64+
65+
let emptyBs = toBytes("")
66+
check(emptyBs.len == 0, "toBytes(empty): empty seq")
67+
68+
# ---------------------------------------------------------------------------
69+
# Section 2: create_context / installation_name / destroy_context
70+
# ---------------------------------------------------------------------------
71+
72+
proc testContextLifecycle() =
73+
echo "\n--- testContextLifecycle ---"
74+
75+
let ctx = create_context(toReprCString("lifecycle-test"))
76+
check(ctx != nil, "create_context: returns non-nil handle")
77+
78+
var iname = installation_name(ctx)
79+
defer: destroy_string(iname)
80+
let inameStr = $iname
81+
check(inameStr.len > 0, "installation_name: returns non-empty name")
82+
echo " installation name: ", inameStr
83+
84+
destroy_context(ctx)
85+
echo " destroy_context: no crash"
86+
87+
# ---------------------------------------------------------------------------
88+
# Section 3: Full two-party conversation flow
89+
# ---------------------------------------------------------------------------
90+
# Exercises: create_intro_bundle, create_new_private_convo, handle_payload,
91+
# send_content, and all four destroy_* procs.
92+
# VecPayload helpers ([], len, items) are also exercised here.
93+
94+
proc testFullConversationFlow() =
95+
echo "\n--- testFullConversationFlow ---"
96+
97+
let aliceCtx = create_context(toReprCString("alice"))
98+
check(aliceCtx != nil, "Alice: create_context non-nil")
99+
100+
let bobCtx = create_context(toReprCString("bob"))
101+
check(bobCtx != nil, "Bob: create_context non-nil")
102+
103+
# --- create_intro_bundle ---
104+
var bobIntroRes = create_intro_bundle(bobCtx)
105+
check(bobIntroRes.error_code == ErrNone,
106+
"create_intro_bundle: error_code == ErrNone")
107+
check(bobIntroRes.intro_bytes.len > 0,
108+
"create_intro_bundle: intro_bytes non-empty")
109+
110+
# toSeq(VecUint8)
111+
let introBytes = toSeq(bobIntroRes.intro_bytes)
112+
check(introBytes.len > 0, "toSeq(VecUint8): produces non-empty seq")
113+
114+
# destroy_intro_result
115+
destroy_intro_result(bobIntroRes)
116+
echo " destroy_intro_result: no crash"
117+
118+
# --- create_new_private_convo ---
119+
var convoRes = create_new_private_convo(
120+
aliceCtx,
121+
toSlice(introBytes),
122+
toSlice("Hello, Bob!")
123+
)
124+
check(convoRes.error_code == ErrNone,
125+
"create_new_private_convo: error_code == ErrNone")
126+
127+
let aliceConvoId = $convoRes.convo_id
128+
check(aliceConvoId.len > 0, "create_new_private_convo: convo_id non-empty")
129+
echo " Alice-Bob convo_id: ", aliceConvoId
130+
131+
# len(VecPayload)
132+
let numPayloads = len(convoRes.payloads)
133+
check(numPayloads > 0, "len(VecPayload): > 0 payloads in new convo")
134+
135+
# [](VecPayload, int): subscript access
136+
let firstPayload = convoRes.payloads[0]
137+
check(firstPayload.data.len > 0, "VecPayload[0].data: non-empty")
138+
check(firstPayload.address.len > 0, "VecPayload[0].address: non-empty")
139+
echo " first payload address: ", $firstPayload.address
140+
141+
# items(VecPayload): collect bytes before destroy
142+
var payloadDatas: seq[seq[byte]] = @[]
143+
var iterCount = 0
144+
for p in convoRes.payloads:
145+
payloadDatas.add(toSeq(p.data))
146+
inc iterCount
147+
check(iterCount == numPayloads,
148+
"items(VecPayload): iterator yields all payloads")
149+
150+
# destroy_convo_result
151+
destroy_convo_result(convoRes)
152+
echo " destroy_convo_result: no crash"
153+
154+
# --- handle_payload ---
155+
var bobSawContent = false
156+
var bobConvoId = ""
157+
for pData in payloadDatas:
158+
var hp = handle_payload(bobCtx, toSlice(pData))
159+
check(hp.error_code == ErrNone, "handle_payload: error_code == ErrNone")
160+
161+
let content = toSeq(hp.content)
162+
if content.len > 0:
163+
bobConvoId = $hp.convo_id
164+
check(bobConvoId.len > 0,
165+
"handle_payload: convo_id non-empty when content present")
166+
if not bobSawContent:
167+
check(hp.is_new_convo,
168+
"handle_payload: is_new_convo == true on first contact")
169+
bobSawContent = true
170+
echo " Bob received content in convo: ", bobConvoId
171+
172+
destroy_handle_payload_result(hp)
173+
174+
check(bobSawContent, "handle_payload: Bob received Alice's opening message")
175+
echo " destroy_handle_payload_result: no crash"
176+
177+
# --- send_content ---
178+
var sendRes = send_content(
179+
aliceCtx,
180+
toReprCString(aliceConvoId),
181+
toSlice("How are you, Bob?")
182+
)
183+
check(sendRes.error_code == ErrNone,
184+
"send_content: error_code == ErrNone for valid convo_id")
185+
check(len(sendRes.payloads) > 0,
186+
"send_content: returns at least one payload")
187+
188+
var sendPayloadDatas: seq[seq[byte]] = @[]
189+
for p in sendRes.payloads:
190+
sendPayloadDatas.add(toSeq(p.data))
191+
192+
# destroy_send_content_result
193+
destroy_send_content_result(sendRes)
194+
echo " destroy_send_content_result: no crash"
195+
196+
# Bob handles follow-up payloads
197+
for pData in sendPayloadDatas:
198+
var hp2 = handle_payload(bobCtx, toSlice(pData))
199+
check(hp2.error_code == ErrNone,
200+
"handle_payload: Bob handles send_content payload without error")
201+
destroy_handle_payload_result(hp2)
202+
203+
destroy_context(aliceCtx)
204+
destroy_context(bobCtx)
205+
echo " both contexts destroyed: no crash"
206+
207+
# ---------------------------------------------------------------------------
208+
# Section 4: Error-case coverage
209+
# ---------------------------------------------------------------------------
210+
# Exercises destroy_* on error results (empty/null Vecs) to confirm they
211+
# do not crash.
212+
213+
proc testErrorCases() =
214+
echo "\n--- testErrorCases ---"
215+
216+
let ctx = create_context(toReprCString("error-tester"))
217+
check(ctx != nil, "error-tester: create_context non-nil")
218+
219+
# send_content with a nonexistent convo_id must fail
220+
var badSend = send_content(
221+
ctx,
222+
toReprCString("00000000-0000-0000-0000-nonexistent"),
223+
toSlice("payload")
224+
)
225+
check(badSend.error_code != ErrNone,
226+
"send_content(bad convo_id): error_code != ErrNone")
227+
echo " send_content(bad convo_id) error_code: ", badSend.error_code
228+
# Destroy error result to confirm destroy handles empty VecPayload
229+
destroy_send_content_result(badSend)
230+
echo " destroy_send_content_result(error result): no crash"
231+
232+
# create_new_private_convo with garbage bytes must fail with ErrBadIntro
233+
let badIntro: seq[byte] = @[0xDE'u8, 0xAD'u8, 0xBE'u8, 0xEF'u8]
234+
var badConvo = create_new_private_convo(
235+
ctx,
236+
toSlice(badIntro),
237+
toSlice("content")
238+
)
239+
check(badConvo.error_code == ErrBadIntro,
240+
"create_new_private_convo(bad intro): error_code == ErrBadIntro")
241+
destroy_convo_result(badConvo)
242+
echo " destroy_convo_result(error result): no crash"
243+
244+
destroy_context(ctx)
245+
246+
# ---------------------------------------------------------------------------
247+
# Entry point
248+
# ---------------------------------------------------------------------------
249+
250+
when isMainModule:
251+
echo "=== test_all_endpoints: begin ==="
252+
253+
testHelperProcs()
254+
testContextLifecycle()
255+
testFullConversationFlow()
256+
testErrorCases()
257+
258+
echo "\n=== ALL TESTS PASSED ==="

0 commit comments

Comments
 (0)