Skip to content

test(makernotes): delete three suites that mirror fabricated tag tables - #407

Merged
swackhamer merged 1 commit into
fix/revive-unreachable-tests-wiringfrom
fix/revive-vendor-makernote-tests
Aug 2, 2026
Merged

test(makernotes): delete three suites that mirror fabricated tag tables#407
swackhamer merged 1 commit into
fix/revive-unreachable-tests-wiringfrom
fix/revive-vendor-makernote-tests

Conversation

@swackhamer

Copy link
Copy Markdown
Collaborator

Stacked on #404 (base retargets to main when #404 merges).

Summary

Three of the six undeclared vendor MakerNote suites are deleted rather than declared. Every tag name they assert appears in zero ExifTool 13.59 source files. Declaring them would have pinned invented data as expected behaviour — the exact mechanism that let the fabricated registries/apple.rs survive.

file tests disposition
qualcomm_makernotes_tests.rs 17 deleted — parser already removed as fabricated
google_makernotes_tests.rs 17 deleted — all 8 names fabricated
microsoft_makernotes_tests.rs 16 deleted — all 12 names fabricated

50 tests deleted, 0 kept. No production code changed.

Evidence

Ground truth: ExifTool 13.59 at /tmp/oxidex-exiftool-cache/exiftool, searched with /usr/bin/grep -a -r "Name => '<tag>'" lib/ — ExifTool's own tag tables, not the -listx documentation view.

Qualcomm — production already deleted QualcommParser as a fabrication (see the note in makernotes::mod). ClearSight, ClearSightMode, ChromaFlash, OptiZoom, BokehMode, BokehLevel, ZoomLevel, HDRMode, NightMode, LowLightMode, SceneDetection, PhaseDetectAF, FrameMergeCount: 0 hits each. The test file outlived the parser's deletion only because nothing ever compiled it.

GoogleAstrophotography, 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', stored base64-encoded/encrypted/gzipped) — not a numeric TIFF IFD. Its real tags are ImageName, ImageData, TimeLogText, SummaryText, FrameCount, CreateDate.

Microsoft — of the 12 asserted names only LensType (10 other vendors' modules) and PanoramaMode (Kodak, Olympus) exist anywhere, and neither in Microsoft.pm. MakerNotes.pm has no MakerNoteMicrosoft dispatch at all; Microsoft.pm's only GROUPS => { 0 => 'MakerNotes' } table is Microsoft::Stitch, binary data in EXIF tag 0x4748, with tags PanoramicStitchVersion / PanoramicStitchCameraMotion / PanoramicStitchMapType.

This is the identical shape to the already-documented Qualcomm finding: a string-keyed or binary table reached from somewhere other than a Make= MakerNote IFD, re-imagined as a numeric IFD with invented ids and names.

Why deleting passing tests is the right call

google (17/17) and microsoft (16/16) passed once declared. 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.

Not fixed here

makernotes/google.rs, makernotes/microsoft.rs and registries/samsung.rs still emit these fabricated tag names in production. That is a behavioural change across three vendor parsers and needs its own review; it is reported separately and is out of scope for a test-suite PR.

Verification

cargo test --all-features --test integration554 passed, 0 failed.

🤖 Generated with Claude Code

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>
@swackhamer
swackhamer merged commit c856863 into fix/revive-unreachable-tests-wiring Aug 2, 2026
2 of 3 checks passed
@swackhamer
swackhamer deleted the fix/revive-vendor-makernote-tests branch August 2, 2026 03:54
swackhamer added a commit that referenced this pull request Aug 2, 2026
…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>
swackhamer added a commit that referenced this pull request Aug 2, 2026
* test(wiring): compile the 15 test files Cargo never built

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>

* test(makernotes): delete three suites that mirror fabricated tag tables (#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>

* style: cargo fmt the newly-reachable error_handling_tests

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>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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