Skip to content

Commit 19f91e2

Browse files
committed
ci: the destructive-wipe job could go green having run nothing
Two independent ways this job's only real assertion could vanish silently. 1. `cargo test <filter>` is a SUBSTRING match with NO floor. Rename the test -- dropping just `the_` from `wipes_the_entire_namespace_destructively` is enough -- and the filter matches zero tests: running 0 tests test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out exit status 0 The job goes green, having stood up a dedicated valkey/valkey:8 service container to run nothing at all, and because the test is `#[ignore]`d no other job covers it either. The step now asserts `test result: ok. 1 passed`, so a rename fails loudly instead of quietly deleting the coverage. Deliberately NOT `--exact`: the `1 passed` floor is the load-bearing half and cannot break on a guess about the module path. 2. `plugin_path()` in the plugin's e2e tests was the only one of the ten sibling plugin repos without the `is_none() && CI -> panic!()` guard -- while `valkey_url()` seventeen lines below it in the same file has it. Demonstrated rather than assumed, with the cdylib absent: running 3 tests test admin_api_installs_the_valkey_plugin_and_writes_land_in_real_valkey ... ok test load_and_exercise_valkey_plugin_bad_config_fails_over_abi ... ok test load_and_exercise_valkey_plugin_persists_to_real_valkey_across_reopen ... ok test result: ok. 3 passed; ... finished in 0.00s Three tests "passed" in 0.00s without their subject existing. It now checks both the uplifted target dir and target/deps (a bare `cargo test` uplifts to neither for this crate), and refuses to skip under CI.
1 parent fa9ead1 commit 19f91e2

3 files changed

Lines changed: 60 additions & 8 deletions

File tree

.github/workflows/ci.yml

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,4 +84,19 @@ jobs:
8484
working-directory: store-valkey-repo/store-valkey
8585
env:
8686
VALKEY_URL: redis://localhost:6379/0
87-
run: cargo test -- --ignored --test-threads=1 wipes_the_entire_namespace_destructively
87+
run: |
88+
set -euo pipefail
89+
# FLOOR ASSERTION on the number of tests that actually ran. `cargo test <filter>` is a
90+
# SUBSTRING match with no minimum: rename the test (dropping "the_" is enough) and the
91+
# filter matches ZERO tests, cargo prints "running 0 tests / test result: ok" and EXITS 0.
92+
# This whole job — and the dedicated valkey/valkey:8 service container it stands up — then
93+
# goes green having run nothing, and because the test is #[ignore]'d no other job covers
94+
# it either. Requiring "1 passed" is what makes the rename fail loudly instead.
95+
out="$(cargo test -- --ignored --test-threads=1 \
96+
wipes_the_entire_namespace_destructively 2>&1 | tee /dev/stderr)"
97+
grep -qE 'test result: ok\. 1 passed' <<<"$out" || {
98+
echo "::error::the destructive-wipe test did not run (renamed, moved, or filtered out)." \
99+
"The filter matched no tests and cargo exited 0. Re-point the filter at the test's" \
100+
"current name — do NOT relax this assertion." >&2
101+
exit 1
102+
}

Cargo.lock

Lines changed: 11 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

store-valkey-plugin/tests/e2e.rs

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,39 @@ const TEST_SIGNING_KEY: &str = "0123456789abcdef0123456789abcdef0123456789abcdef
4141
/// under `cargo test` (which builds the whole package including the cdylib target before running
4242
/// tests) it is always present, so this only guards against unusual invocations.
4343
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>
46-
let name = plugin_library_filename("busbar_store_valkey_plugin");
47-
let candidate = profile_dir.join(&name);
48-
candidate.exists().then_some(candidate)
44+
let candidate = (|| {
45+
let exe = std::env::current_exe().ok()?; // .../target/<profile>/deps/e2e-<hash>
46+
let profile_dir = exe.parent()?.parent()?; // .../target/<profile>
47+
let name = plugin_library_filename("busbar_store_valkey_plugin");
48+
// Check BOTH the "uplifted" <profile>/<name> copy and the raw <profile>/deps/<name>
49+
// compiler output, newest wins: a bare `cargo test` does NOT uplift the cdylib to the
50+
// top-level profile dir, only to deps/ (same fix already applied to store-postgres's,
51+
// store-mysql's, auth-oidc's and webrequest-hook's equivalent helpers).
52+
let uplifted = profile_dir.join(&name);
53+
let raw = profile_dir.join("deps").join(&name);
54+
[uplifted, raw]
55+
.into_iter()
56+
.filter_map(|p| {
57+
std::fs::metadata(&p)
58+
.and_then(|m| m.modified())
59+
.ok()
60+
.map(|mtime| (p, mtime))
61+
})
62+
.max_by_key(|(_, mtime)| *mtime)
63+
.map(|(p, _)| p)
64+
})();
65+
// Under CI a missing cdylib is a HARD FAILURE, never a silent skip — the same discipline
66+
// `valkey_url()` below already applies, and the one every sibling plugin repo applies here.
67+
// Without it the three tests gated on this helper skip to green in 0.00s when their subject
68+
// disappears, which is the only over-the-ABI coverage of the valkey store path.
69+
if candidate.is_none() && std::env::var_os("CI").is_some() {
70+
panic!(
71+
"the store-valkey plugin cdylib is not built under CI: `cargo test` must build it \
72+
(checked both the uplifted target dir and target/deps). Refusing to silently skip \
73+
the only over-the-ABI coverage of the durable Valkey store path."
74+
);
75+
}
76+
candidate
4977
}
5078

5179
/// The live `VALKEY_URL`, mirroring `busbar-store-valkey`'s own `live_store()` gating discipline

0 commit comments

Comments
 (0)