Skip to content

test(wiring): compile the 15 test files Cargo never built - #404

Merged
swackhamer merged 3 commits into
mainfrom
fix/revive-unreachable-tests-wiring
Aug 2, 2026
Merged

test(wiring): compile the 15 test files Cargo never built#404
swackhamer merged 3 commits into
mainfrom
fix/revive-unreachable-tests-wiring

Conversation

@swackhamer

Copy link
Copy Markdown
Collaborator

The defect

Cargo compiles only tests/*.rs as test roots. Files in subdirectories are reached solely through mod / #[path] declarations. 21 files under tests/ were declared by nothing, so 114 #[test] functions had never once compiled — let alone run.

tests/integration/mod.rs was itself unreachable: nothing declares mod integration;. Every module it listed was therefore dead too.

An undeclared test file is indistinguishable from a comment. This is how the fabricated registries/apple.rs survived — apple_makernotes_tests.rs asserted its invented tag names and never built.

This PR

Revives 15 of the 21 files (24 tests). The 5 vendor MakerNote files follow separately; tests/ffi/build.rs is a build script, not a test module.

file tests result
error_handling_tests.rs 9 10 pass (one added)
{mkv,webm,flv,avi,mts}_integration_tests.rs 5 pass
{mp3,flac,aac,wav,ogg,opus,ape}_integration_tests.rs 7 pass
unit/audio/mp3_tests.rs 2 pass

tests/integration/mod.rs is removed: it duplicated two entries already in tests/integration.rs, and its 12 container modules are now declared there like every other integration module.

Three defects the dead code was hiding

1. API drift. All 12 container tests called MetadataMap::from_file (no longer exists) and stringified via TagValue::to_string() (never existed — TagValue has no Display). Rewritten to the current BufferedReader + parse_<fmt>_metadata entrypoints and the explicit variant match already used by infiray_tests.rs.

2. The parity comparison compared nothing. The tests ran exiftool -json but looked up group-qualified keys like FLAC:SampleRate. Plain -json emits bare tag names, so every lookup missed. Eleven files hit a continue and passed vacuously; flac_integration_tests.rs lacks that guard and compared "44100" against the literal string "null". Fixed with -G0, which emits exactly the asserted keys. FLAC now genuinely compares SampleRate/Channels/BitsPerSample against ExifTool, and matches.

3. A test with a wrong premise. test_error_unsupported_format fed in "INVALID_FORMAT_MAGIC_BYTES_HERE" and asserted it was not a known format. But that is printable ASCII, and ExifTool 13.59 reports it as FileType: TXT, MIMEType: text/plain — oxidex answering TXT was correct. The test now uses genuinely unrecognized binary (ExifTool 13.59: Error: Unknown file type), and a new sibling test test_printable_ascii_is_txt_not_unknown pins the TXT behaviour so a future change to it reads as the regression it would be.

Separately, test_error_truncated_tiff required the error text to contain one of eof/truncate/unexpected/invalid. The parser correctly reports IFD offset 8 exceeds file size 8 — a precise truncation diagnosis matching no substring. Widened the accepted set rather than changing a correct parser.

Verification

  • No test deleted.
  • cargo test --all-features --test integration --test unit_tests554 passed, 17 passed, 0 failed.
  • cargo test --test integration -- --ignored *_integration_tests12 passed, 0 failed.
  • Ground truth: ExifTool 13.59 via /usr/bin/perl5.34 -I/tmp/oxidex-exiftool-cache/exiftool/lib.
  • cargo fmt --all and cargo clippy clean.

🤖 Generated with Claude Code

@swackhamer
swackhamer enabled auto-merge (squash) August 2, 2026 03:45
swackhamer added a commit that referenced this pull request Aug 2, 2026
cargo fmt --all only formats files reachable from the crate roots, so this
file was never formatted while it was undeclared. Declaring it in #404 made
it visible to the formatter for the first time, and CI's --check caught the
drift.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
swackhamer and others added 2 commits August 2, 2026 03:14
Cargo compiles only `tests/*.rs` as test roots; files in subdirectories are
reached solely through `mod` / `#[path]` declarations. 21 files under `tests/`
were declared by nothing, so 114 `#[test]` functions had never once compiled.

`tests/integration/mod.rs` was itself unreachable -- nothing declared
`mod integration;` -- so every module it listed was dead. It also duplicated
two entries already declared in `tests/integration.rs`, so it is removed and
its 12 container/audio modules are declared in `tests/integration.rs` like
every other integration module.

Revived here (15 files, 24 tests):
  error_handling_tests.rs        9 tests -> 10 pass
  {mkv,webm,flv,avi,mts}         5 tests -> pass
  {mp3,flac,aac,wav,ogg,opus,ape} 7 tests -> pass
  unit/audio/mp3_tests.rs        2 tests -> pass

Three defects the dead code was hiding:

1. API drift: all 12 container tests called `MetadataMap::from_file`, which no
   longer exists, and stringified via `TagValue::to_string()`, which never
   existed (`TagValue` has no `Display`). Rewritten to the current
   `BufferedReader` + `parse_<fmt>_metadata` entrypoints and the explicit
   variant match used by `infiray_tests.rs`.

2. The container tests ran `exiftool -json` but compared group-qualified keys
   ("FLAC:SampleRate"). Plain `-json` emits bare tag names, so every lookup
   missed. Eleven files then hit a `continue` and passed on nothing; the FLAC
   file lacks that guard and compared "44100" against the literal "null".
   Fixed with `-G0`, which emits exactly the keys asserted. FLAC now really
   does compare SampleRate/Channels/BitsPerSample against ExifTool, and matches.

3. `detect_format` returns `io::Result<FileFormat>` and reports an unrecognized
   signature as `Ok(FileFormat::Unknown)`, not `None`. `test_error_unsupported_format`
   also had a wrong premise: its "invalid magic bytes" were the printable string
   "INVALID_FORMAT_MAGIC_BYTES_HERE", and ExifTool 13.59 reports that as
   `FileType: TXT, MIMEType: text/plain` -- oxidex agreeing with TXT was correct.
   The test now feeds genuinely unrecognized binary (ExifTool 13.59: "Unknown
   file type") and a new sibling test pins the TXT behaviour so a future change
   to it reads as the regression it would be.

`test_error_truncated_tiff` asserted the error message contained one of
eof/truncate/unexpected/invalid. The parser correctly reports "IFD offset 8
exceeds file size 8" -- a precise truncation diagnosis that matched no
substring. Widened the accepted set rather than changing the parser.

No test deleted. 554 integration + 17 unit tests pass under --all-features.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…es (#407)

These three files were never declared to Cargo, so they had never compiled.
Compiling them was not the fix: every tag name they assert appears in **zero**
ExifTool 13.59 source files. Declaring them would pin invented data as
expected behaviour -- the exact mechanism that let the fabricated
`registries/apple.rs` survive.

Verified against ExifTool 13.59 (/tmp/oxidex-exiftool-cache/exiftool), using
`grep -a -r "Name => '<tag>'"` over lib/, i.e. ExifTool's own tag tables.

qualcomm_makernotes_tests.rs (17 tests)
  Tests `QualcommParser`, which production already deleted as a fabrication
  (see the note in makernotes::mod). Asserted ClearSight, ClearSightMode,
  ChromaFlash, OptiZoom, BokehMode, BokehLevel, ZoomLevel, HDRMode,
  NightMode, LowLightMode, SceneDetection, PhaseDetectAF, FrameMergeCount,
  MultiFrameNoiseReduction under "Qualcomm:" -- all absent from Qualcomm.pm.
  The file outlived the parser's deletion only because nothing compiled it.

google_makernotes_tests.rs (17 tests)
  Asserted Astrophotography, ColorPop, FaceRetouching, HDRPlusMode,
  MergedFrameCount, NightSight, SceneDetection, SuperResZoom -- 0 hits each.
  ExifTool's *only* Google MakerNote table is Google::HDRPlusMakerNote, and it
  is string-id keyed protobuf (ID_FMT => 'str', ids like '1-1', '9-36-1',
  base64 + encrypted + gzipped), not a numeric TIFF IFD. Its real tags are
  ImageName, ImageData, TimeLogText, SummaryText, FrameCount, CreateDate.

microsoft_makernotes_tests.rs (16 tests)
  Asserted AutoHDR, CreativeEffect, DynamicFlash, LensType,
  OpticalStabilization, PanoramaMode, PureViewMode, Refocus, RichCapture,
  RichCaptureMode, RichRecordingAudio, Video4K. Only LensType (10 other
  vendors' modules) and PanoramaMode (Kodak, Olympus) exist at all, and
  neither in Microsoft.pm. MakerNotes.pm has no MakerNoteMicrosoft dispatch;
  Microsoft.pm's only MakerNotes-group table is Microsoft::Stitch, binary data
  in EXIF tag 0x4748 with tags PanoramicStitchVersion / ...CameraMotion /
  ...MapType.

google and microsoft were *passing*. That is the finding, not a reassurance:
they pass because `makernotes/google.rs` and `makernotes/microsoft.rs` invent
exactly the names the tests assert. A test that mirrors a fabricated table
cannot detect the fabrication, and leaving it in place would make removing the
invented production tags look like a regression.

The production fabrication itself is NOT fixed here -- it is a behavioural
change across three vendor parsers and needs its own review. It is reported
separately.

50 tests deleted, 0 kept. No production code changed.
554 integration tests pass under --all-features.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
cargo fmt --all only formats files reachable from the crate roots, so this
file was never formatted while it was undeclared. Declaring it in #404 made
it visible to the formatter for the first time, and CI's --check caught the
drift.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@swackhamer
swackhamer force-pushed the fix/revive-unreachable-tests-wiring branch from b4d02e8 to f93a8cb Compare August 2, 2026 09:07
@swackhamer
swackhamer merged commit 5e3f00c into main Aug 2, 2026
6 of 7 checks passed
@swackhamer
swackhamer deleted the fix/revive-unreachable-tests-wiring branch August 2, 2026 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant