Skip to content

Commit 366af65

Browse files
committed
test(abi): emit the cdylib under cargo test at all, then refuse a stale one
Two defects, both of which made this repo's only over-the-ABI coverage inert. 1. store-valkey-plugin declared crate-type = ["cdylib"] only. With cdylib-only, `cargo test` builds a "0 tests" harness for the package and NEVER produces the .dylib/.so, so tests/e2e.rs found nothing and silently skipped — green, with zero coverage of the durable valkey path. Verified: with cdylib-only, no deps/libbusbar_store_valkey_plugin.dylib appears even after touching src/lib.rs; with rlib added, cargo emits it on every build that recompiles the lib. The sibling sqlite/postgres/mysql plugin manifests already carry both crate-types for exactly this reason; this one had drifted. 2. plugin_path() looked in target/<profile>/, which only `cargo build` refreshes, and returned None -> "skip:" -> GREEN when absent. It now reads only deps/<name> (the output of the build graph that produced this test binary), panics instead of skipping, and asserts the artifact is no older than any workspace src/**/*.rs with a message that says STALE ARTIFACT rather than letting it read as a durability failure. Also: the persistence test reused a fixed key id and relied on delete_key to clean up. delete_key TOMBSTONES by contract, so get_key still answers afterwards — against a re-used Valkey the test passed with the plugin's put_key stubbed to Ok(()). Proven: that stub passed against a re-used instance and failed against a flushed one. A per-run key id makes every read answerable only by this run's writes.
1 parent e9518e1 commit 366af65

2 files changed

Lines changed: 138 additions & 32 deletions

File tree

store-valkey-plugin/Cargo.toml

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,16 @@ description = "The Valkey store as a droppable busbar plugin — a cdylib export
99
license = "Apache-2.0"
1010

1111
[lib]
12-
crate-type = ["cdylib"]
12+
# cdylib for the C ABI delivery; `rlib` is REQUIRED for `cargo test` to emit the cdylib artifact AT
13+
# ALL. With `cdylib`-only, cargo builds a trivial "0 tests" harness for this package under `cargo
14+
# test` and never produces the .dylib/.so — so `tests/e2e.rs`, the ONLY over-the-ABI coverage of the
15+
# durable valkey path, found nothing and silently skipped, reporting GREEN with zero coverage. The
16+
# sibling store-postgres-plugin/store-mysql-plugin/store-sqlite-plugin manifests already carry both
17+
# crate-types for exactly this reason; this one had drifted. Verified, not assumed: with `cdylib`
18+
# only, `cargo test -p busbar-store-valkey-plugin` leaves no
19+
# `target/debug/deps/libbusbar_store_valkey_plugin.dylib` even after touching src/lib.rs; with
20+
# `rlib` added, cargo emits it on every build that recompiles the lib.
21+
crate-type = ["cdylib", "rlib"]
1322

1423
[dependencies]
1524
# busbar-store-valkey is now a SAME-REPO sibling crate (this plugin brings 100% of what it needs — the

store-valkey-plugin/tests/e2e.rs

Lines changed: 128 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,106 @@ use std::time::{Duration, Instant};
3636
/// explicit signing key to mint virtual keys; busbar no longer auto-generates one.
3737
const TEST_SIGNING_KEY: &str = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef";
3838

39-
/// Locate the built `busbar_store_valkey_plugin` cdylib in the target dir, derived from the test
40-
/// binary's own path (robust to a custom `CARGO_TARGET_DIR`). `None` if it hasn't been built —
41-
/// under `cargo test` (which builds the whole package including the cdylib target before running
42-
/// tests) it is always present, so this only guards against unusual invocations.
43-
fn plugin_path() -> Option<std::path::PathBuf> {
44-
let exe = std::env::current_exe().ok()?; // .../target/<profile>/deps/e2e-<hash>
45-
let profile_dir = exe.parent()?.parent()?; // .../target/<profile>
39+
/// Locate the cdylib THIS `cargo test` invocation just built — never a leftover artifact.
40+
///
41+
/// This looks ONLY in `target/<profile>/deps/`, never `target/<profile>/`, and that distinction is
42+
/// the whole point of this function.
43+
///
44+
/// `cargo` emits the lib target's cdylib into `deps/` as part of the very build graph that produces
45+
/// this test binary (this package's lib unit is compiled with BOTH declared crate-types — see
46+
/// `[lib] crate-type = ["cdylib", "rlib"]` in Cargo.toml), so `deps/libbusbar_store_valkey_plugin.dylib` is by construction up to
47+
/// date with the source tree under test. Cargo only *uplifts* a copy to `target/<profile>/` for
48+
/// `cargo build`, NEVER for `cargo test`. A lookup in `target/<profile>/` therefore reads an
49+
/// artifact that nothing in this test's dependency graph refreshes: whatever some earlier `cargo
50+
/// build` left there, from any commit — or nothing at all.
51+
///
52+
/// Both outcomes of that are lies about durability, and the second is the dangerous one:
53+
/// * NOTHING there -> the old code `return`ed with a "skip:" line and reported GREEN. That is how
54+
/// `cargo test` can pass with ZERO over-the-ABI coverage of the durable store path.
55+
/// * STALE artifact -> a cdylib built before an ABI change answers every write `Ok(())` and every
56+
/// read empty, which is BYTE-FOR-BYTE the signature of the unrelayed-seam defect this file
57+
/// exists to catch (that defect was real: `DynStore`'s `impl Store` overrode 24 methods, none of
58+
/// them the task/call-log methods, so `put_task` took the accept-and-keep-nothing trait
59+
/// default). RED on a stale artifact is indistinguishable from RED on the real bug — and an
60+
/// artifact NEWER than a regression reports GREEN while the shipped ABI is broken. Proven, not
61+
/// theorised: with a regressed plugin in the tree and a good cdylib in `target/debug/`, the old
62+
/// lookup passed and this one fails.
63+
///
64+
/// Same hazard, and the same reasoning, as the engine's `crates/busbar/Cargo.toml` dev-dependency on
65+
/// `busbar-store-example-plugin`: keep the cdylib in the build graph so no test can judge a stale
66+
/// one. Here the plugin's lib IS this package, so that graph edge already exists — what was missing
67+
/// was reading the artifact that edge actually produces.
68+
///
69+
/// Panics rather than skipping: a missing cdylib under `cargo test` means the build graph changed
70+
/// shape, and the only honest report of that is a failure, not a silent pass.
71+
/// The newest mtime across every workspace crate's `src/` — "how fresh must a cdylib be to be the
72+
/// one this source tree describes".
73+
///
74+
/// Deliberately ONLY `src/**/*.rs` of each workspace member: editing a `tests/` file or a
75+
/// `[dev-dependencies]` line recompiles the test binary but NOT the lib, so including those would
76+
/// fail a perfectly current cdylib.
77+
fn newest_source_mtime() -> std::time::SystemTime {
78+
fn walk(dir: &std::path::Path, newest: &mut std::time::SystemTime) {
79+
let Ok(rd) = std::fs::read_dir(dir) else { return };
80+
for e in rd.flatten() {
81+
let p = e.path();
82+
if p.is_dir() {
83+
walk(&p, newest);
84+
} else if p.extension().is_some_and(|x| x == "rs") {
85+
if let Ok(m) = e.metadata().and_then(|m| m.modified()) {
86+
if m > *newest {
87+
*newest = m;
88+
}
89+
}
90+
}
91+
}
92+
}
93+
let ws_root = std::path::Path::new(env!("CARGO_MANIFEST_DIR"))
94+
.parent()
95+
.expect("the plugin crate always sits under the workspace root");
96+
let mut newest = std::time::SystemTime::UNIX_EPOCH;
97+
for e in std::fs::read_dir(ws_root).into_iter().flatten().flatten() {
98+
let src = e.path().join("src");
99+
if src.is_dir() {
100+
walk(&src, &mut newest);
101+
}
102+
}
103+
newest
104+
}
105+
106+
fn plugin_path() -> PathBuf {
107+
let exe = std::env::current_exe().expect("current_exe"); // .../target/<profile>/deps/<test>-<hash>
108+
let deps_dir = exe.parent().expect("the test binary always lives in deps/");
46109
let name = plugin_library_filename("busbar_store_valkey_plugin");
47-
let candidate = profile_dir.join(&name);
48-
candidate.exists().then_some(candidate)
110+
let fresh = deps_dir.join(&name);
111+
assert!(
112+
fresh.exists(),
113+
"the store-valkey-plugin cdylib is not at {}, where cargo emits it for the same build that produced \
114+
this test binary. Refusing to fall back to target/<profile>/ (an artifact only `cargo \
115+
build` refreshes) or to skip: judging a stale cdylib is exactly how an unrelayed plugin \
116+
ABI reads as green.",
117+
fresh.display()
118+
);
119+
// FRESHNESS, ASSERTED — not assumed. Under `cargo test` the artifact above is rebuilt by the
120+
// same graph that built this binary (proven: delete it, re-run, cargo re-emits it). But this
121+
// test binary can also be executed DIRECTLY out of `deps/`, where nothing rebuilds anything,
122+
// and a stale cdylib there produces empty reads — indistinguishable from the unrelayed-ABI
123+
// defect. So compare it against the sources and fail with a message that says STALE ARTIFACT,
124+
// explicitly NOT a durability verdict.
125+
let built = std::fs::metadata(&fresh)
126+
.and_then(|m| m.modified())
127+
.expect("cdylib mtime");
128+
let newest_src = newest_source_mtime();
129+
assert!(
130+
built >= newest_src,
131+
"STALE ARTIFACT — THIS IS NOT A DURABILITY FAILURE. {} predates this workspace's sources, \
132+
so it cannot answer for the code in the tree; a pre-change cdylib returns empty for every \
133+
read, which reads exactly like an unrelayed plugin ABI. Run `cargo build -p {}` (or just \
134+
`cargo test`, which rebuilds it) and re-run.",
135+
fresh.display(),
136+
"busbar-store-valkey-plugin"
137+
);
138+
fresh
49139
}
50140

51141
/// The live `VALKEY_URL`, mirroring `busbar-store-valkey`'s own `live_store()` gating discipline
@@ -116,34 +206,47 @@ fn ledger() -> UsageLedger {
116206
/// This is the proof that `store: valkey` operations over the ABI aren't silently no-ops.
117207
#[test]
118208
fn load_and_exercise_valkey_plugin_persists_to_real_valkey_across_reopen() {
119-
let Some(path) = plugin_path() else {
120-
eprintln!("skip: valkey plugin cdylib not built (run under `cargo test`)");
121-
return;
122-
};
209+
let path = plugin_path();
123210
let Some(url) = valkey_url() else {
124211
return;
125212
};
126213
let cfg = serde_json::json!({ "url": url }).to_string();
127214

128215
// Isolate from any prior run against a persistent (non-CI) Valkey instance.
216+
//
217+
// A FRESH ID PER RUN, not a fixed one plus a `delete_key`: `delete_key` TOMBSTONES the row (it
218+
// is a soft delete by contract — see `Store::delete_key`), so `get_key` still answers with it
219+
// afterwards. Against a re-used Valkey that made this test pass even when the plugin's
220+
// `put_key` wrote NOTHING at all: every read below was satisfied by the previous run's row.
221+
// Proven, not theorised — a `put_key` stubbed to `Ok(())` passed this test against a re-used
222+
// instance and failed it against a flushed one. A per-run id makes every read here answerable
223+
// only by THIS run's writes.
129224
let direct = ValkeyStore::connect(&url).expect("connect directly to seed/clean up");
130-
let _ = Store::delete_key(&direct, "vk_e2e_dlopen");
225+
let vk_id = format!(
226+
"vk_e2e_dlopen_{}_{}",
227+
std::process::id(),
228+
std::time::SystemTime::now()
229+
.duration_since(std::time::UNIX_EPOCH)
230+
.unwrap()
231+
.as_nanos()
232+
);
233+
let vk_id = vk_id.as_str();
131234

132-
let vk = key("vk_e2e_dlopen");
235+
let vk = key(vk_id);
133236

134237
{
135238
let store = load_store(&path, &cfg).expect("load valkey plugin against a real Valkey");
136239
store.put_key(&vk).expect("put_key over the ABI");
137240
store
138-
.put_usage("vk_e2e_dlopen", 200, &ledger())
241+
.put_usage(vk_id, 200, &ledger())
139242
.expect("put_usage over the ABI");
140243
assert_eq!(
141244
store
142-
.get_key("vk_e2e_dlopen")
245+
.get_key(vk_id)
143246
.expect("get_key over the ABI")
144247
.expect("present in the same session")
145248
.id,
146-
"vk_e2e_dlopen"
249+
vk_id
147250
);
148251
// `store` (and the `RawPlugin` it wraps) drops here, running `busbar_close` and dropping
149252
// the plugin's own `ValkeyStore`/connection — the data must be durably in Valkey after
@@ -155,13 +258,13 @@ fn load_and_exercise_valkey_plugin_persists_to_real_valkey_across_reopen() {
155258
// on the first instance still being alive.
156259
let reopened = load_store(&path, &cfg).expect("re-load valkey plugin against the same URL");
157260
let got = reopened
158-
.get_key("vk_e2e_dlopen")
261+
.get_key(vk_id)
159262
.expect("get_key after reopen")
160263
.expect("the key must survive a full plugin close + reopen against the same Valkey");
161264
assert_eq!(got.group.as_deref(), Some("infra"));
162265
assert_eq!(got.labels.get("env").map(String::as_str), Some("e2e"));
163266
let usage = reopened
164-
.get_usage("vk_e2e_dlopen", 200)
267+
.get_usage(vk_id, 200)
165268
.expect("get_usage after reopen");
166269
assert_eq!(usage.requests, 5, "usage ledger must survive the reopen");
167270
let t = usage
@@ -175,22 +278,22 @@ fn load_and_exercise_valkey_plugin_persists_to_real_valkey_across_reopen() {
175278
// plugin's `put_key`/`put_usage` over the ABI were silent no-ops (or wrote somewhere other
176279
// than the configured Valkey), this independent reader would come back empty even though the
177280
// reopen-via-plugin check above passed.
178-
let direct_key = Store::get_key(&direct, "vk_e2e_dlopen")
281+
let direct_key = Store::get_key(&direct, vk_id)
179282
.expect("get_key via the direct connection")
180283
.expect("the key must be physically present in Valkey, bypassing the plugin");
181284
assert_eq!(direct_key.name, "e2e-dlopen-key");
182285
assert_eq!(
183286
direct_key.allowed_scopes,
184287
Some(vec![busbar_api::ScopeRef::pool("p")])
185288
);
186-
let direct_usage = Store::get_usage(&direct, "vk_e2e_dlopen", 200)
289+
let direct_usage = Store::get_usage(&direct, vk_id, 200)
187290
.expect("get_usage via the direct connection");
188291
assert_eq!(
189292
direct_usage.requests, 5,
190293
"usage must be physically present in Valkey, not just cached in-process by the plugin"
191294
);
192295

193-
let _ = Store::delete_key(&direct, "vk_e2e_dlopen");
296+
let _ = Store::delete_key(&direct, vk_id);
194297
}
195298

196299
/// END-TO-END FAILURE: an `open()` config that cannot produce a usable store — malformed JSON, a
@@ -199,10 +302,7 @@ fn load_and_exercise_valkey_plugin_persists_to_real_valkey_across_reopen() {
199302
/// case here fails before (or instead of) actually connecting.
200303
#[test]
201304
fn load_and_exercise_valkey_plugin_bad_config_fails_over_abi() {
202-
let Some(path) = plugin_path() else {
203-
eprintln!("skip: valkey plugin cdylib not built (run under `cargo test`)");
204-
return;
205-
};
305+
let path = plugin_path();
206306

207307
let err = load_store(&path, "{ not json")
208308
.err()
@@ -356,10 +456,7 @@ fn admin_test_valkey_url(base: &str) -> String {
356456
fn admin_api_installs_the_valkey_plugin_and_writes_land_in_real_valkey() {
357457
let Some(base_url) = valkey_url() else { return };
358458
let url = admin_test_valkey_url(&base_url);
359-
let Some(so_path) = plugin_path() else {
360-
eprintln!("skip: valkey plugin cdylib not built");
361-
return;
362-
};
459+
let so_path = plugin_path();
363460

364461
// Isolate from any prior run against a persistent (non-CI) Valkey instance.
365462
let direct = ValkeyStore::connect(&url).expect("connect directly to seed/clean up");

0 commit comments

Comments
 (0)