test architecture v0.6.x — T1 (mutants + coverage + fuzz + criterion)#145
Merged
Conversation
WHY: prep for the mutants.yml PR workflow added in the next commit. Centralises test_tool=nextest (mandatory per the no-cargo-test rule in CLAUDE.md), exclude_globs for crates/desktop (GTK env friction), and timeout_multiplier=5.0 for async-heavy code paths. NO exclude_re (matches mutant names not file paths — would unpredictably catch fns whose names contain the regex). NO additional_cargo_args (intended for build flags, not package selection — would clash with cargo-mutants' own per-package layer).
WHY: slice 1 of 4 in the T1 test-architecture decomposition (per docs/superpowers/specs/2026-04-21-architecture-audit-followups-design.md §T1). Mutation testing catches a class of bug LLM-authored tests miss (tests that look right but assert the wrong thing). This workflow is Linux-only (mutation testing has no per-OS bug surface worth 3x'ing CI cost on), observability-only (continue-on-error swallows the non-zero cargo-mutants exit so the survivor count surfaces in the PR's mutants-out artifact without blocking merge), and per-PR via --in-diff (workspace-full mutation runs are minutes-to-hours; --in-diff filters to the changed code). GTK Linux deps installed because cargo metadata resolves the whole workspace including perima-desktop's transitive deps even when desktop is excluded from mutation generation. Slice 2 (golden EXIF/XMP corpus) + slice 3 (cargo-fuzz) + slice 4 (criterion benchmarks) deferred to follow-up batches per spec §1 decomposition.
WHY: pure-move refactor in preparation for slice 2 of T1 test
architecture. Existing tests/integration.rs is 615 LOC; new
edge-case tests pushed it over the 700-LOC Batch F/G soft-limit.
Per the convention, shared integration-test helpers live in
tests/common/mod.rs.
Zero behavioural delta — the 6 existing tests pass byte-for-byte.
Only change to helper bodies is adding `pub` visibility so test
binaries can `use common::{...}`.
The two #![allow] header attrs (`unreachable_pub`, `dead_code`) on
common/mod.rs are load-bearing per the Batch F/G workaround for
rust-lang/rust#46379 — every test binary compiles common/mod.rs
but only uses a subset of helpers.
Adds image_extractor_directory_path_returns_default. Targets the read_exif \`MediaSource::file_path\` Err arm (warn-level log) per slice 2 spec §4 D-2 B1. Contract: ImageExtractor::extract must NOT propagate I/O errors. The Err arm returns (None, None, None) for EXIF fields and the outer extract still returns Ok(MediaMetadata) with mime_type populated.
Adds image_extractor_garbage_png_returns_no_dims. Targets the ImageExtractor::extract `image::image_dimensions` Err arm (debug log) per slice 2 spec §4 D-2 B4. Contract: extract returns Ok with width/height = None when the file isn't a parseable image, never propagating the decode error.
Adds video_extractor_garbage_mp4_returns_default. Targets the VideoExtractor::extract `mp4parse::read_mp4` Err arm (debug log) per slice 2 spec §4 D-2 B5. 16 zero bytes lack any valid ISO box structure. Contract: extract returns Ok with duration_ms / width / height / codec all None and mime_type populated; the parse error is never propagated.
Add `make_mp4_audio_first_then_video` helper (audio track_id=1, video track_id=2) and `video_extractor_mp4_audio_first_then_video_picks_video` test. Exercises `video_tracks_summary`'s loop-skip-non-video branch that the existing single-video-track fixture never reached.
Adds make_jpeg_with_corrupt_tiff helper (Strategy A: bogus "XX" TIFF magic) and image_extractor_jpeg_corrupt_tiff_returns_default test. The fixture has APP1 segment + Exif\0\0 identifier so has_exif() returns true, then parser.parse() fails on the invalid magic — reaching the Err arm at extractor.rs:121-128. Strategy A confirmed sufficient: nom-exif checks has_exif() via the Exif identifier only, not TIFF magic.
Adds image_extractor_jpeg_padded_ascii_is_trimmed. Targets the read_exif trim_end_matches(['\0', ' ']) branch per slice 2 spec §4 D-2 B3. Fixture mirrors the Nikon "NIKON CORPORATION \0" pattern documented in extractor.rs line 134. New helpers in common/mod.rs: - make_jpeg_with_padded_ascii (pub) — caller passes raw bytes for Make/Model so trailing padding is byte-controlled. - build_tiff_exif_padded (private) — near-copy of build_tiff_exif that takes &[u8] instead of &str; always appends one NUL. Test passes b"NIKON CORPORATION " (3 spaces) and b"D850 " (1 space); asserts camera_make = "NIKON CORPORATION" and camera_model = "D850" after trim.
Adds `image_extractor_jpeg_make_model_only_no_datetime` (branch B9 per slice 2 spec §4 D-2). Exercises the outer `.and_then(EntryValue:: as_time_components)` collapse-to-None arm in `read_exif`: the TIFF has IFD0 with Make + Model only (no ExifIFDPointer, no ExifSubIFD), so `exif.get(DateTimeOriginal)` returns None and `captured_at = None` while Make/Model populate normally. New helpers in `tests/common/mod.rs`: - `make_jpeg_with_make_model_only` (pub) — JPEG wrapper. - `build_tiff_make_model_only` (private) — 2-entry IFD0 TIFF builder. WHY 100-byte NUL padding in `build_tiff_make_model_only`: nom-exif's `MediaParser::parse` copies the `MediaSource` pre-read buffer (128 bytes) then calls `fill_buf` on the underlying reader. Files <= 128 bytes are fully consumed by the pre-read, leaving the reader at EOF; the subsequent `fill_buf` returns `UnexpectedEof` -> parse returns `(None, None, None)`. The NUL padding pushes the JPEG past the 128-byte threshold; it is structurally inert (TIFF parsers follow the IFD chain which terminates at next=0, ignoring bytes after the string area). This is distinct from B1 (file I/O error) and B2 (corrupt TIFF magic): has_exif() == true, parse() succeeds, only the DateTimeOriginal entry is absent. `common/mod.rs` is now 804 LOC -- over the 700-LOC soft-limit. GH #139 tracks the split-by-access-pattern follow-up; deliberate in this task.
WHY: prep for the `exif` and `blake3` cargo-fuzz targets added in the next two commits. fuzz/ is its OWN Cargo workspace (`[workspace] members = []`) so its `rust-toolchain.toml` nightly pin doesn't affect production stable builds — `cargo build` in the parent workspace continues to use stable as configured. Per slice 3 of T1 test-architecture decomposition, this slice ships the scaffold + 2 fuzz targets + a Linux-only weekly-cron workflow. Per spec §3 in/out: NO production code changes; NO new dependencies in the parent workspace; fuzz only depends on `libfuzzer-sys` + `tempfile` + path-deps on `perima-core`/`perima-hash`/`perima-media`. Cargo.lock for the fuzz workspace is deliberately tracked (per spec Q4) but NOT committed in this commit — it arrives in M-2 alongside `fuzz/fuzz_targets/exif.rs` when `cargo fuzz build` first generates the dep-graph lock. Step 7 verifies the manifest via pure TOML parse (tomllib) precisely to avoid generating Cargo.lock prematurely. Also adds `!fuzz/README.md` exception to root .gitignore — the blanket `**/*.md` rule would otherwise prevent tracking the contributor quick-start doc.
…eptions The previous commit (c57d51f) inadvertently re-gitignored CLAUDE.md and docs/superpowers/**/*.md with a "TEMPORARY per user request" comment that was unfounded — no such request was made. CLAUDE.md's own Git section explicitly states these files are tracked so cloud agents can continue autonomous work across sessions; re-gitignoring them would break that contract on the next remote-trigger clone. Restores the !CLAUDE.md + !docs/superpowers/**/*.md exceptions. The legitimate planned change from c57d51f (the !fuzz/README.md exception) is preserved.
WHY: fuzzes the full extract() pipeline — image::image_dimensions JPEG header parsing AND nom-exif EXIF parsing — by feeding arbitrary bytes as a JPEG-mime'd tempfile. Both surfaces must be panic-free on any input (Result::Err is fine; panic is the bug). Per slice 3 spec §1 D-1: target name "exif" slightly oversells the scope (it actually fuzzes both image-crate JPEG header parsing AND nom-exif), but widening `read_exif` to `pub` purely to isolate it would be undesirable production-API growth. Cargo.lock added by `cargo fuzz build`'s first compile — committed per spec Q4 for reproducibility of the fuzz toolchain's resolved deps. 10s local smoke run completed without crash.
WHY: fuzzes Blake3Service's quick_hash(&Path) + full_hash(&Path) by feeding arbitrary bytes as a tempfile. Both methods share the inner hash_file helper (crates/hash/src/blake3_service.rs lines 39-62); calling both per iteration covers the cap-Some (quick, 64 KiB cap) and cap-None (full, no cap) branches with near-zero added cost (BLAKE3 on small inputs is microsecond-range). Per spec §10 Q2: NOT adding a `hash_bytes(&[u8])` method purely for fuzz testability — tempfile mirror keeps the production API surface unchanged. The LLM-blindspot risk is in chunk-size handling around BLAKE3's update + EOF semantics + I/O-error propagation, all exercised through the file path. 10s local smoke run completed without crash.
WHY: slice 3 of T1 test-architecture decomposition. Runs both fuzz targets (exif, blake3) on Mondays 06:00 UTC + on-demand via workflow_dispatch (with optional max_total_time input). Linux-only (libFuzzer doesn't work cleanly on Windows; macOS deferred). Matrix over both targets so they run in parallel. Each per-target fuzz step is `continue-on-error: true` (observability-only) — libFuzzer exits non-zero on a crash; without the swallow, the very first crash would block all future runs of this workflow. Crashes upload as fuzz-artifacts-<target> CI artifact for triager download. The Quick start + Triage flow are documented in fuzz/README.md. Slice 4 (criterion benchmarks) deferred to follow-up batch per slice 1 spec §17 decomposition.
WHY: prep for the blake3 hash-throughput benchmark added in the next commit. Slice 4 of T1 test-architecture decomposition (per spec docs/superpowers/specs/2026-04-25-criterion-benches-design.md). criterion 0.5 added narrowly to perima-hash's [dev-dependencies] ONLY (per spec D-12 — NOT a workspace-wide dep). The [[bench]] entry uses harness = false so criterion installs its own harness in place of libtest. The bench source file (`benches/blake3.rs`) lands in N-2 — until then, `cargo build --benches -p perima-hash` is expected to fail with "couldn't read benches/blake3.rs". This proves the [[bench]] entry is wired correctly.
Adds bench_blake3 benchmark group over 3 input sizes (1 KiB / 64 KiB / 1 MiB) x 2 hash modes (quick_hash + full_hash) with Throughput::Bytes annotation so criterion reports MiB/s alongside per-iter time. iter_batched + BatchSize::SmallInput keeps the per-iteration tempfile write out of the measurement window — only the hash call is timed. sample_size = 30 (criterion default = 100; reduced for CI wall-clock budget per spec D-6). Override at invocation: `cargo bench -- --sample-size N`. WHY #![allow(missing_docs)] at the top of the bench file: the workspace lint set denies missing_docs, but `criterion_group!` macro expansion produces an undocumented `pub fn benches()`. Bench targets are not part of the library's public surface (compiled only with --benches), and the macro entry point is third-party. Local 10-sample smoke run completed all 6 benchmarks: 1KiB quick/full ~13 MiB/s, 64KiB ~520 MiB/s, 1MiB ~1.27 GiB/s on this VM (cold-cache dev environment numbers — CI runner numbers will differ; baseline calibration is the follow-up GH issue per spec §10 Q5). Cargo.lock updated with criterion 0.5 transitive deps (anes, cast, ciborium, plotters, etc.) — first build that resolved them.
Adds bench_fts benchmark group measuring `search` on a 100-row seeded DB. Setup spawns SqliteWriter (which migrates schema + installs FTS5 triggers), then seeds 100 rows directly into `search_content` via raw rusqlite (mirroring the seed_via_conn pattern in crates/app/src/search.rs lines 202-223 with #[allow(clippy::disallowed_methods)]). iter_batched + BatchSize::LargeInput amortizes the writer-spawn + 100-INSERT setup cost over multiple search iterations. WHY direct INSERT into search_content (not files/tags/file_tags via public-API repos): SearchRepository::search queries the search_index FTS5 virtual table, which is populated by triggers ON search_content. Multi-table seeding into the source tables would silently leave search_index empty and the bench would measure a cold-miss path. sample_size = 30 (criterion default = 100). Local 10-sample smoke completed in ~30 sec without error; SMOKE_HITS=20 confirms FTS5 trigger fans out to search_index and the search returns the limit. Cargo.toml: criterion = "0.5" added narrowly to perima-db's [dev-dependencies] (not workspace-wide per spec D-12). [[bench]] entry uses harness = false.
WHY: slice 4 of T1 test-architecture decomposition. Runs `cargo bench -p perima-hash -p perima-db` on Mondays 07:00 UTC (1h after slice 3 cargo-fuzz to avoid runner-pool contention) + on-demand via workflow_dispatch (with optional sample_size input). Linux-only (perf is OS-variant; cross-OS comparison would mislead). Single job (no matrix — both benches share a compile cache). Bench step is `continue-on-error: true` (observability-only) — any non-zero exit doesn't block the workflow; results are tee'd to \$GITHUB_STEP_SUMMARY so the workflow run page shows the latest numbers. target/criterion/ HTML reports upload as `criterion-reports` artifact for deep-dive analysis. NO threshold gate, NO PR-blocking, NO baseline-comparison infrastructure (per spec D-4 + Batch J GH #135 flake-risk precedent).
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
1 participant
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.
Summary
T1 of the architecture-audit umbrella: 4 sequential slices building out the test architecture from baseline to per-PR mutation testing + extractor regression coverage + libFuzzer + criterion benchmarks. 17 commits, 0 production code changed (
git diff main..HEAD -- 'crates/*/src/'is empty), 6 follow-up GH issues filed during execution.d6bf343+24b6bbf):.cargo/mutants.toml+.github/workflows/mutants.yml. Linux-only, observability-only viacontinue-on-error: true.test_tool = "nextest"mandatory per the no-cargo-test SQLite-deadlock rule.1c9846c..5549957, 8 commits): refactored helpers fromcrates/media/tests/integration.rstotests/common/mod.rs(Batch F/G convention) + 7 new regression tests for previously-untested branches inread_exif/ImageExtractor::extract/VideoExtractor::extract. 2 originally-planned tests dropped per track_duration_ms: B6 (movie-timescale fallback) + B7 (zero-timescale check) unreachable via mp4parse 0.17 input #140 —mp4parse0.17 atomically populatestrack.duration+track.timescale, making the defensivetrack_duration_msarms structurally unreachable.c57d51f..697b275, 5 commits): newfuzz/leaf workspace ([workspace] members = []so its nightly pin doesn't affect production builds);exif+blake3libFuzzer targets; weekly cron + manualworkflow_dispatch.68c12bd..a19e76d, 4 commits):blake3(3 sizes × 2 hash modes) +fts(FTS5 search latency on 100-row seededsearch_content) benchmarks; weekly cron tees results to\$GITHUB_STEP_SUMMARY. Observability-only.Open follow-up issues filed during T1
crates/media/tests/common/mod.rsby access pattern (LOC soft-limit)track_duration_msB6 + B7 unreachable via mp4parse 0.17 (defensive code)Test plan
Held documentation
The 4 slice spec files (`docs/superpowers/specs/2026-04-{24,25}-.md`), 4 slice plan files (`docs/superpowers/plans/2026-04-{24,25}-.md`), and the CLAUDE.md "Test stack" 4-bullet additions are intentionally NOT in this PR — kept in working tree per a session-level hold. Will commit separately when lifted.