Skip to content

Commit 5fd9924

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 5fd9924

5 files changed

Lines changed: 263 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: 2 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

nim-bindings/conversations_example.nimble

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: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ type
7676
proc create_context*(name: ReprCString): ContextHandle {.importc.}
7777

7878
## Returns the friendly name of the context's identity
79-
## The result must be freed by the caller (repr_c::String ownership transfers)
8079
proc installation_name*(ctx: ContextHandle): ReprCString {.importc.}
8180

8281
## Destroys a context and frees its memory
Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
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+
let iname = installation_name(ctx)
79+
let inameStr = $iname
80+
check(inameStr.len > 0, "installation_name: returns non-empty name")
81+
echo " installation name: ", inameStr
82+
83+
destroy_context(ctx)
84+
echo " destroy_context: no crash"
85+
86+
# ---------------------------------------------------------------------------
87+
# Section 3: Full two-party conversation flow
88+
# ---------------------------------------------------------------------------
89+
# Exercises: create_intro_bundle, create_new_private_convo, handle_payload,
90+
# send_content, and all four destroy_* procs.
91+
# VecPayload helpers ([], len, items) are also exercised here.
92+
93+
proc testFullConversationFlow() =
94+
echo "\n--- testFullConversationFlow ---"
95+
96+
let aliceCtx = create_context(toReprCString("alice"))
97+
check(aliceCtx != nil, "Alice: create_context non-nil")
98+
99+
let bobCtx = create_context(toReprCString("bob"))
100+
check(bobCtx != nil, "Bob: create_context non-nil")
101+
102+
# --- create_intro_bundle ---
103+
var bobIntroRes = create_intro_bundle(bobCtx)
104+
check(bobIntroRes.error_code == ErrNone,
105+
"create_intro_bundle: error_code == ErrNone")
106+
check(bobIntroRes.intro_bytes.len > 0,
107+
"create_intro_bundle: intro_bytes non-empty")
108+
109+
# toSeq(VecUint8)
110+
let introBytes = toSeq(bobIntroRes.intro_bytes)
111+
check(introBytes.len > 0, "toSeq(VecUint8): produces non-empty seq")
112+
113+
# destroy_intro_result
114+
destroy_intro_result(bobIntroRes)
115+
echo " destroy_intro_result: no crash"
116+
117+
# --- create_new_private_convo ---
118+
var convoRes = create_new_private_convo(
119+
aliceCtx,
120+
toSlice(introBytes),
121+
toSlice("Hello, Bob!")
122+
)
123+
check(convoRes.error_code == ErrNone,
124+
"create_new_private_convo: error_code == ErrNone")
125+
126+
let aliceConvoId = $convoRes.convo_id
127+
check(aliceConvoId.len > 0, "create_new_private_convo: convo_id non-empty")
128+
echo " Alice-Bob convo_id: ", aliceConvoId
129+
130+
# len(VecPayload)
131+
let numPayloads = len(convoRes.payloads)
132+
check(numPayloads > 0, "len(VecPayload): > 0 payloads in new convo")
133+
134+
# [](VecPayload, int): subscript access
135+
let firstPayload = convoRes.payloads[0]
136+
check(firstPayload.data.len > 0, "VecPayload[0].data: non-empty")
137+
check(firstPayload.address.len > 0, "VecPayload[0].address: non-empty")
138+
echo " first payload address: ", $firstPayload.address
139+
140+
# items(VecPayload): collect bytes before destroy
141+
var payloadDatas: seq[seq[byte]] = @[]
142+
var iterCount = 0
143+
for p in convoRes.payloads:
144+
payloadDatas.add(toSeq(p.data))
145+
inc iterCount
146+
check(iterCount == numPayloads,
147+
"items(VecPayload): iterator yields all payloads")
148+
149+
# destroy_convo_result
150+
destroy_convo_result(convoRes)
151+
echo " destroy_convo_result: no crash"
152+
153+
# --- handle_payload ---
154+
var bobSawContent = false
155+
var bobConvoId = ""
156+
for pData in payloadDatas:
157+
var hp = handle_payload(bobCtx, toSlice(pData))
158+
check(hp.error_code == ErrNone, "handle_payload: error_code == ErrNone")
159+
160+
let content = toSeq(hp.content)
161+
if content.len > 0:
162+
bobConvoId = $hp.convo_id
163+
check(bobConvoId.len > 0,
164+
"handle_payload: convo_id non-empty when content present")
165+
check(hp.is_new_convo,
166+
"handle_payload: is_new_convo == true on first contact")
167+
bobSawContent = true
168+
echo " Bob received content in convo: ", bobConvoId
169+
170+
destroy_handle_payload_result(hp)
171+
172+
check(bobSawContent, "handle_payload: Bob received Alice's opening message")
173+
echo " destroy_handle_payload_result: no crash"
174+
175+
# --- send_content ---
176+
var sendRes = send_content(
177+
aliceCtx,
178+
toReprCString(aliceConvoId),
179+
toSlice("How are you, Bob?")
180+
)
181+
check(sendRes.error_code == ErrNone,
182+
"send_content: error_code == ErrNone for valid convo_id")
183+
check(len(sendRes.payloads) > 0,
184+
"send_content: returns at least one payload")
185+
186+
var sendPayloadDatas: seq[seq[byte]] = @[]
187+
for p in sendRes.payloads:
188+
sendPayloadDatas.add(toSeq(p.data))
189+
190+
# destroy_send_content_result
191+
destroy_send_content_result(sendRes)
192+
echo " destroy_send_content_result: no crash"
193+
194+
# Bob handles follow-up payloads
195+
for pData in sendPayloadDatas:
196+
var hp2 = handle_payload(bobCtx, toSlice(pData))
197+
check(hp2.error_code == ErrNone,
198+
"handle_payload: Bob handles send_content payload without error")
199+
destroy_handle_payload_result(hp2)
200+
201+
destroy_context(aliceCtx)
202+
destroy_context(bobCtx)
203+
echo " both contexts destroyed: no crash"
204+
205+
# ---------------------------------------------------------------------------
206+
# Section 4: Error-case coverage
207+
# ---------------------------------------------------------------------------
208+
# Exercises destroy_* on error results (empty/null Vecs) to confirm they
209+
# do not crash.
210+
211+
proc testErrorCases() =
212+
echo "\n--- testErrorCases ---"
213+
214+
let ctx = create_context(toReprCString("error-tester"))
215+
check(ctx != nil, "error-tester: create_context non-nil")
216+
217+
# send_content with a nonexistent convo_id must fail
218+
var badSend = send_content(
219+
ctx,
220+
toReprCString("00000000-0000-0000-0000-nonexistent"),
221+
toSlice("payload")
222+
)
223+
check(badSend.error_code != ErrNone,
224+
"send_content(bad convo_id): error_code != ErrNone")
225+
echo " send_content(bad convo_id) error_code: ", badSend.error_code
226+
# Destroy error result to confirm destroy handles empty VecPayload
227+
destroy_send_content_result(badSend)
228+
echo " destroy_send_content_result(error result): no crash"
229+
230+
# create_new_private_convo with garbage bytes must fail with ErrBadIntro
231+
let badIntro: seq[byte] = @[0xDE'u8, 0xAD'u8, 0xBE'u8, 0xEF'u8]
232+
var badConvo = create_new_private_convo(
233+
ctx,
234+
toSlice(badIntro),
235+
toSlice("content")
236+
)
237+
check(badConvo.error_code == ErrBadIntro,
238+
"create_new_private_convo(bad intro): error_code == ErrBadIntro")
239+
destroy_convo_result(badConvo)
240+
echo " destroy_convo_result(error result): no crash"
241+
242+
destroy_context(ctx)
243+
244+
# ---------------------------------------------------------------------------
245+
# Entry point
246+
# ---------------------------------------------------------------------------
247+
248+
when isMainModule:
249+
echo "=== test_all_endpoints: begin ==="
250+
251+
testHelperProcs()
252+
testContextLifecycle()
253+
testFullConversationFlow()
254+
testErrorCases()
255+
256+
echo "\n=== ALL TESTS PASSED ==="

0 commit comments

Comments
 (0)