Skip to content

fix(apple): HDRImageType printed invented values; guard test reachability - #412

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

fix(apple): HDRImageType printed invented values; guard test reachability#412
swackhamer merged 1 commit into
fix/revive-unreachable-tests-wiringfrom
fix/salvage-apple-samsung-makernote-tests

Conversation

@swackhamer

Copy link
Copy Markdown
Collaborator

Stacked on #404 (base retargets to main when #404 merges). Completes the revival of all 21 unreachable test files.

The production bug the dead tests were hiding

DECODE_HDR_TYPE (Apple.pm 0x000a HDRImageType) carried nine invented values:

0 Off | 1 HDR | 2 HDR (Original) | 3 Auto HDR | 4 Smart HDR | 5..8 Smart HDR 2 .. Smart HDR 5

ExifTool 13.59 Apple.pm declares exactly two:

0x000a => {
    Name => 'HDRImageType',
    PrintConv => { 3 => 'HDR Image', 4 => 'Original Image' },
},

So on a real iPhone file oxidex printed "Auto HDR" where ExifTool prints "HDR Image", and "Smart HDR" where ExifTool prints "Original Image" — under the real tag name Apple:HDRImageType. Per AGENTS.md, a plausible-but-wrong value under a real tag name is worse than an absent tag: it does not crash, and nothing downstream can tell.

The undeclared test asserted 0 => "Off" and 4 => "Smart HDR" — so simply declaring it would have passed and pinned the wrong values. That is the whole thesis of this work in one tag.

Table trimmed to ExifTool's two entries; unmapped values fall through to "Unknown (N)", matching ExifTool. Three in-repo assertions that encoded the old values are corrected, and a new test pins 0 => "Unknown (0)" so the invented labels cannot come back.

apple_makernotes_tests.rs: 18 → 9

Deleted 10 tests asserting names with 0 hits in ExifTool 13.59: FacingCamera, LensModel, NightMode, PortraitMode, SceneDetection. Two more asserted SemanticStyle as a SHORT enum at 0x2E — wrong twice over: ExifTool puts SemanticStyle at 0x0040 and decodes it with ConvertPLIST (a property list, not an enum), and 0x002e is CameraType.

Rewrote the HDR tests to ExifTool's values, added the unmapped-value guard, and rebuilt test_apple_multiple_tags on two verified tags (0x000a HDRImageType, 0x0005 AETarget).

samsung_makernotes_tests.rs: 23 → 6

Deleted 17 tests asserting names with 0 hits: SceneOptimizer, SceneType, SingleTake, ExpertRAW, NightMode, GalaxyLensType, ZoomLevel, PortraitEffect, DirectorsView, ProMode, SuperSteady, FoodMode, ObjectTracking, MultiFrameNoiseReduction (Sony.pm only).

Two near-misses are recorded in the file because they look like hits:

  • %Image::ExifTool::Samsung::PortraitEffect is a real table name, but it is JSON-processed with string keys and yields PortraitEffectID, PortraitEffectLevel, … — there is no Samsung:PortraitEffect tag, and it is not a numeric MakerNote IFD entry.
  • SceneType exists in FlashPix.pm / Exif.pm / DICOM.pm, not Samsung.pm.

Kept 4 passing: parser_trait, MakerNoteVersion (0x0001), DeviceType (0x0002 → 0x2000 'High-end NX Camera'), and SamsungModelID retargeted from 0x100123a => "NX1" (absent from ExifTool — its only NX1 is commented out at 0x5001038) to 0x100101c => 'NX10', the one mapping where oxidex's MODEL_ID_DECODER and ExifTool agree.

Two ColorSpace tests are #[ignore]d, not deleted. The PrintConv is right, but SAMSUNG_COLOR_SPACE is 0x0221 and 0x0221 appears nowhere in Samsung.pm — ExifTool puts ColorSpace at 0xa011 in Samsung::Type2. Consequence on a real file: ColorSpace is never emitted, and whatever really lives at 0x0221 would be misread as ColorSpace. The tests encode ExifTool's id so they fail until the constant is fixed. Bending them to 0x0221 would pin the defect; deleting them would lose it.

The guard

tests/test_file_reachability.rs re-derives the reachable set from the tests/*.rs roots by following mod / #[path] edges transitively, and fails listing any orphan. It independently reproduced the original 21-file set while this work was in progress. tests/ffi/build.rs is exempt — it is a Cargo build script, not a test module.

Also found, reported separately (not fixed here)

Five more Apple decoders disagree with ExifTool. Worst is ImageCaptureType (0x0014), where every shared key is wrong — value 10, very common on modern iPhones, prints "Screenshot" instead of ExifTool's "Photo". OISMode, GreenGhostMitigationStatus and SignalToNoiseRatioType invent labels for tags where ExifTool has no PrintConv at all and prints a raw integer.

Verification

  • 3354 lib + 567 integration + 17 unit + 1 reachability tests pass under --all-features.
  • cargo clippy --all-features -- -D warnings clean; cargo fmt --all -- --check clean.
  • Ground truth: ExifTool 13.59, searched with /usr/bin/grep -a (the shell grep is ugrep with --ignore-files and gives false negatives).

🤖 Generated with Claude Code

…lity

Completes the revival of the 21 unreachable test files. Apple and Samsung were
the two suites left; both were part fabrication, part real.

## The production bug the dead tests were hiding

`DECODE_HDR_TYPE` (Apple.pm 0x000a HDRImageType) carried nine invented values:

    0 Off | 1 HDR | 2 HDR (Original) | 3 Auto HDR | 4 Smart HDR
    5..8 Smart HDR 2 .. Smart HDR 5

ExifTool 13.59 Apple.pm declares exactly two:

    3 => 'HDR Image'
    4 => 'Original Image'

So on a real iPhone file oxidex printed **"Auto HDR" where ExifTool prints
"HDR Image"**, and **"Smart HDR" where ExifTool prints "Original Image"** --
under the real tag name `Apple:HDRImageType`. Per AGENTS.md, a
plausible-but-wrong value under a real tag name is worse than an absent tag: it
does not crash and nothing downstream can tell.

The undeclared test asserted `0 => "Off"` and `4 => "Smart HDR"`, so declaring
it would have *passed* and pinned the wrong values. Table trimmed to ExifTool's
two entries; unmapped values now fall through to "Unknown (N)", matching
ExifTool. Three in-repo assertions that encoded the old values are corrected,
and a new test pins `0 => "Unknown (0)"` so the invented labels cannot return.

## apple_makernotes_tests.rs: 18 -> 9

Deleted 10 tests asserting names with 0 hits in ExifTool 13.59:
FacingCamera, LensModel, NightMode, PortraitMode, SceneDetection. Two more
asserted SemanticStyle as a SHORT enum at 0x2E -- wrong twice: ExifTool puts
SemanticStyle at 0x0040 and decodes it with ConvertPLIST, and 0x002e is
CameraType. Rewrote the HDR tests to ExifTool's values, added the unmapped
guard, and rebuilt test_apple_multiple_tags on two verified tags
(0x000a HDRImageType, 0x0005 AETarget).

## samsung_makernotes_tests.rs: 23 -> 6

Deleted 17 tests asserting names with 0 hits: SceneOptimizer, SceneType,
SingleTake, ExpertRAW, NightMode, GalaxyLensType, ZoomLevel, PortraitEffect,
DirectorsView, ProMode, SuperSteady, FoodMode, ObjectTracking,
MultiFrameNoiseReduction (Sony.pm only). Two near-misses recorded in the file:
Samsung::PortraitEffect is a real *table* but JSON/string-keyed, yielding
PortraitEffectID etc., not a `Samsung:PortraitEffect` MakerNote tag; SceneType
lives in FlashPix/Exif/DICOM.

Kept 4 passing: parser_trait, MakerNoteVersion (0x0001), DeviceType
(0x0002 => 0x2000 'High-end NX Camera'), and SamsungModelID retargeted from
0x100123a => "NX1" (absent from ExifTool; its only NX1 is commented out at
0x5001038) to 0x100101c => 'NX10', the one mapping where oxidex's
MODEL_ID_DECODER and ExifTool agree.

Two ColorSpace tests are #[ignore]d, not deleted: the PrintConv is right but
SAMSUNG_COLOR_SPACE is 0x0221, and 0x0221 appears nowhere in Samsung.pm --
ExifTool puts ColorSpace at 0xa011 in Samsung::Type2. They encode ExifTool's id
so they fail until the constant is fixed; bending them to 0x0221 would pin the
defect and deleting them would lose it.

## The guard

tests/test_file_reachability.rs re-derives the reachable set from the
`tests/*.rs` roots by following mod / #[path] edges, and fails listing any
orphan. It independently reproduced the original 21-file set while the work was
in progress. tests/ffi/build.rs is exempt: it is a Cargo build script, not a
test module.

3354 lib + 567 integration + 17 unit + 1 reachability tests pass under
--all-features. cargo clippy --all-features -- -D warnings clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@swackhamer
swackhamer merged commit b4d02e8 into fix/revive-unreachable-tests-wiring Aug 2, 2026
3 checks passed
@swackhamer
swackhamer deleted the fix/salvage-apple-samsung-makernote-tests branch August 2, 2026 04: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