test(enrich,model): close coverage gaps in sbom.rs and kev.rs#40
Merged
Conversation
Pre-fix coverage from CI's lcov:
src/model/sbom.rs 0.0% lines 0.0% fns
src/enrich/kev.rs 39.6% lines 33.3% fns
src/model/sbom.rs was 12 lines of pure match-arm impls (as_str,
Display, Serialize) with zero I/O and zero tests. Adding three
small unit tests pins the canonical string form ("CycloneDX",
"SPDX", "Syft") for each variant across all three impls. The
string is load-bearing for the diff-comment metadata footer, the
SARIF run.tool description, and the JSON output's format field,
so a typo in any branch silently mislabels every diff for that
format. 0% to 100%.
src/enrich/kev.rs already had the right shape for testability —
an injection-point signature `enrich_with_url(e, url, timeout,
ttl_hours)` and pure-function read_cache/write_cache that take
a path parameter — but no tests exercised any of it. The 91
uncovered lines were almost entirely network/disk plumbing that
is in fact unit-testable; they just weren't being tested.
Adding eight tests:
- enrich short-circuit when vulns is empty (covers the public
entry point through to enrich_with_url's first branch)
- enrich_with_ttl short-circuit (same path via the ttl-aware
entry point used by --cache-ttl-hours)
- enrich_with_url with a refused-connect URL + 500ms timeout
pins the best-effort contract: network failure must NOT flag
refs as KEV. Uses port 1 (TCPMUX, never bound on GitHub-
hosted runners) for a deterministic refuse-connect
- read_cache: missing file, fresh file (within TTL), corrupt
body. Covers the cache-hit and cache-miss recovery paths
- write_cache: round-trip through read_cache + parent-dir
creation. Pins the atomic-write-via-tmp-rename contract
load_or_fetch relies on for cache hits
- cache_path: suffix assertion, platform-agnostic so it works
on Linux + macOS + Windows runners
Excluded: the strict ttl-expiry branch (`age > ttl_secs`) needs
either a 1+ second sleep or a filetime crate dep to test
reliably; both costs outweigh the single-line coverage gain.
Test count: 432 -> 444. Release-mode test suite passes; clippy
and fmt clean.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Coverage reportLine coverage: 86.6% (9349 / 10797 lines) Full lcov report available as workflow artifact coverage-lcov: download from this run. v0.9.8 introduces this report; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Maintainer reviewed v0.9.8's lcov artifact and flagged three files with low coverage. Decomposing the data:
src/model/sbom.rssrc/enrich/kev.rssrc/enrich/osv.rsThis PR closes the first two.
osv.rsis left as-is — its 58% is defensibly low because the uncovered surface is OSV's/v1/querybatchHTTP path (wide, would need a mock server in CI), and the pure-compute helpers are already covered.What
src/model/sbom.rs— three tests, 0% → 100%12 lines of pure match-arm impls (
SbomFormat::as_str,Display::fmt,Serialize::serialize) with no I/O and no tests. The string output is load-bearing for:run.tooldescriptionformatfieldA typo in any branch silently mislabels every diff for that format, so pinning the canonical strings (
"CycloneDX","SPDX","Syft") across all three impls is high-leverage for a 10-line test cost.src/enrich/kev.rs— eight tests, ~40% → ~80% (estimated)The file was already written for testability — the
enrich_with_url(e, url, timeout, ttl_hours)injection-point signature plus pure-functionread_cache(path, ttl)/write_cache(path, body)give clean test seams. Nothing was using them. Adding tests for:enrichandenrich_with_ttlshort-circuit whenvulnsis empty (covers the public entry points through toenrich_with_url's first branch)enrich_with_urlwith a refused-connect URL + 500ms timeout pins the best-effort contract: network failure must NOT flag refs as KEV. Uses port 1 (TCPMUX, never bound on GitHub-hosted runners) for a deterministic refuse-connectread_cache: missing file, fresh file within TTL, corrupt body — covers cache-hit and cache-miss recovery pathswrite_cache: round-trip throughread_cache+ parent-dir auto-creation. Pins the atomic-write-via-.tmp-rename contract thatload_or_fetchrelies on for cache hitscache_pathsuffix assertion, platform-agnostic for Linux/macOS/Windows runnersExcluded: the strict TTL-expiry branch
read_cacheusesif age > ttl_secs { return None }(strict greater-than). Forcing the expiry branch reliably needs either a 1+ second sleep (slows CI) or afiletimecrate dep (out of scope). Single-line coverage gain doesn't justify either cost.Validation
cargo fmt --all --checkcleancargo clippy --all-targets --all-features -- -D warningscleancargo test --release: 391 lib + 36 cli + 9 integration + 7 real-world + 1 doctest = 444 tests pass (was 432 → +12 net)Note on what isn't in scope
This PR is purely additive — no production code touched. The asymmetry I noticed in
read_cache's expiry semantics (age > ttl_secsvs the more common>=) is intentionally left alone; changing prod behavior in a coverage-only PR would be a scope violation.