Refactor unit tests: hoist imports, add EXIF read/write coverage, split exiftool tests#821
Merged
Conversation
Move imports that were nested inside test functions/methods up to the top of their modules, and let usort normalize the import blocks. - test_camm_parser: lift mp4_sample_parser into the mapillary_tools.mp4 import - test_exifread: hoist ExifReadFromEXIF/ExifReadFromXMP/XMP_NAMESPACES, ExifToolRead, ExifToolReadVideo and xml.etree; alias the two clashing EXIFTOOL_NAMESPACES as EXIFTOOL_READ_NAMESPACES / EXIFTOOL_READ_VIDEO_NAMESPACES - test_geo: hoist math and telemetry CAMMGPSPoint/GPSFix/GPSPoint - test_gpmf_parser: hoist struct - remaining files: usort import-block normalization only
Raises coverage of exif_read.py 64%->84% and exif_write.py 71%->87%. test_exifread.py: - TestExifReadFromXMPMetadata: the ExifReadFromXMP metadata extractors (altitude, lon/lat incl. Adobe format, make/model, width/height incl. GPano fallbacks, orientation, direction, EXIF/GPS datetime, capture_time) - TestExtractXmpEfficiently: the JPEG APP1/XMP segment scanner, incl. no-SOI, no-XMP, and non-XMP-segment-skipped cases - TestExifReadXmpFallback: the ExifRead EXIF->XMP fallback path driven through a real BytesIO stream test_exifedit.py: - add_make/add_model round-trip and empty-string ValueError - add_orientation out-of-range ValueError - write() on bytes-backed edit without filename raises - _safe_dump recovery: untrusted wrong-type tag stripped, trusted wrong-type tag re-raised, AsShotNeutral workaround (issue #662)
The three _safe_dump recovery tests injected malformed tags via the private ExifEdit._ef dict. There is no public API to trigger those branches, so remove the tests rather than depend on internals.
This reverts commit 616eaed.
Replace the two ExifEdit._safe_dump tests that injected malformed tags through the private _ef dict with file-based equivalents driven by real JPEG fixtures whose EXIF piexif can load but cannot re-dump. - tests/unit/generate_corrupt_exif_image.py: hand-assembles the EXIF (PIL + raw TIFF bytes), since neither PIL nor piexif.dump can emit a malformed-but-loadable tag. - tests/data/corrupt_exif_wrong_type.jpg: Software (non-trusted) stored as SHORT -> _safe_dump strips it and retries. - tests/data/corrupt_exif_trusted_wrong_type.jpg: ImageDescription (trusted) stored as SHORT -> _safe_dump re-raises. The AsShotNeutral workaround branch is dropped: it can't be reproduced from a file (piexif decodes it to a dumpable value) without _ef access.
Relocate the undumpable-EXIF fixtures alongside the other unit-test fixtures and reference them via the test's data_dir.
Point test_exifedit.py at the fixtures via data_dir and update the generator's output path accordingly.
Replace the last ExifEdit._ef access in test_exifedit.py. The large thumbnail is no longer injected into the private _ef dict; instead a committed fixture carries a thumbnail above piexif's 64000-byte dump limit (yet within a JPEG APP1 segment), so ExifEdit loads it and _safe_dump drops the thumbnail/1st IFD on dump and retries. tests/unit/data/corrupt_exif_large_thumbnail.jpg added; generator extended to build it.
Reword test docstrings/comments and rename tests so they describe observable behavior (unsavable tags, oversized thumbnails) rather than implementation details (private helpers, library specifics, IFD layout).
The fixtures are committed binaries under tests/unit/data; the generator is no longer needed.
Move ExifToolRead tests to tests/unit/test_exiftool_read.py and ExifToolReadVideo camera-uuid tests to tests/unit/test_exiftool_read_video.py so that test_exifread.py only covers exif_read.
ptpt
changed the title
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.
Refactor and expand the unit-test suite around EXIF reading/writing and
import hygiene. No production code changes —
mapillary_tools/is untouched.Import hygiene
let usort normalize the import blocks across the unit-test suite.
test_exifread: alias the two clashingEXIFTOOL_NAMESPACESasEXIFTOOL_READ_NAMESPACES/EXIFTOOL_READ_VIDEO_NAMESPACES.test_exifread: drop thegeoimport in favor of an inlinedas_unix_timehelper; test_parse_coordvia the publicextract_lon_lat.Coverage for exif_read / exif_write
Raises coverage of
exif_read.py64%→84% andexif_write.py71%→87%.test_exifread: ExifReadFromXMP metadata extractors (altitude, lon/latincl. Adobe format, make/model, width/height incl. GPano fallbacks,
orientation, direction, EXIF/GPS datetime, capture_time); the JPEG
APP1/XMP segment scanner; the ExifRead EXIF→XMP fallback path.
test_exifedit: add_make/add_model round-trip and empty-stringValueError; add_orientation out-of-range ValueError; write() on a
bytes-backed edit without filename raises.
_safe_dump recovery via committed fixtures
Cover the
_safe_dumprecovery branches with real JPEG fixtures (undertests/unit/data/) instead of reaching into the privateExifEdit._efdict:
corrupt_exif_wrong_type.jpg: untrusted tag with wrong type → strippedand retried.
corrupt_exif_trusted_wrong_type.jpg: trusted tag with wrong type →re-raised.
corrupt_exif_large_thumbnail.jpg: oversized thumbnail → dropped ondump and retried.
Test-file organization
Split the exiftool reader tests out of
test_exifread.pyso it onlycovers
exif_read:test_exiftool_read.py— ExifToolRead teststest_exiftool_read_video.py— ExifToolReadVideo camera-uuid tests