Repository navigation
Conversation
Author
Local coverage measurement,
Files newly at or near 100% that weren't before: This is my own gcovr methodology, not codecov's exact algorithm, so codecov's reported numbers may differ - but the underlying coverage gain is real and verified, each commit checked in isolation to confirm it actually hits the specific line/branch it targets, not just returns correct output through some other path. |
soloturn
marked this pull request as ready for review
September 8, 2026 01:16
veloman-yunkan
requested changes
Sep 8, 2026
veloman-yunkan
left a comment
veloman-yunkan
left a comment
Collaborator
There was a problem hiding this comment.
Some commit messages mention "this PR" while obviously referring to a different PR.
soloturn
added a commit
to soloturn/libzim
that referenced
this pull request
Oct 4, 2026
FileCompound::locate(offset,size) queried the part map with a single wide Range covering the whole read. less_range's comparator isn't a strict weak ordering for that case: the wide query can compare "equivalent" to two disjoint stored part ranges at once, breaking transitivity. With 3+ parts this can land lower_bound on the wrong part, underflowing the local-offset computation in MultiPartFileReader (PR openzim#1126's expanded multiPartReads test hits this on libc++). Fixed by querying the two boundary points instead (locate(offset) above already does this safely for a single point). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
soloturn
force-pushed
the
test/misc-coverage-gaps
branch
from
October 4, 2026 17:36
19cdba3 to
c1c944e
Compare
soloturn
added a commit
to soloturn/libzim
that referenced
this pull request
Oct 4, 2026
FileCompound::locate(offset,size) queried the part map with a single wide Range covering the whole read. less_range's comparator isn't a strict weak ordering for that case: the wide query can compare "equivalent" to two disjoint stored part ranges at once, breaking transitivity. With 3+ parts this can land lower_bound on the wrong part, underflowing the local-offset computation in MultiPartFileReader (PR openzim#1126's expanded multiPartReads test hits this on libc++). Fixed by querying the two boundary points instead (locate(offset) above already does this safely for a single point). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
soloturn
force-pushed
the
test/misc-coverage-gaps
branch
from
October 4, 2026 17:47
c1c944e to
64e764e
Compare
soloturn
force-pushed
the
test/misc-coverage-gaps
branch
from
October 5, 2026 05:23
64e764e to
a5b214a
Compare
Contributor
|
@veloman-yunkan I would like to consider closing this PR, I see no opportunity to get a full fix within a few iterations. What do you think? |
FileCompound::locate(offset,size) queried the part map with a single wide Range covering the whole read. less_range's comparator isn't a strict weak ordering for that case: the wide query can compare "equivalent" to two disjoint stored part ranges at once, breaking transitivity. With 3+ parts this can land lower_bound on the wrong part, underflowing the local-offset computation in MultiPartFileReader (PR openzim#1126's expanded multiPartReads test hits this on libc++). Fixed by querying the two boundary points instead (locate(offset) above already does this safely for a single point). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Scanned overall project coverage for easy, self-contained gaps and added tests for each, checking real content/behavior rather than shape (hex-string length, non-empty, substring) wherever a reviewer flagged that distinction: - Archive::getChecksum(): checks the archive's actual checksum value, one expected hex digest per zim-testing-suite category. - Archive::getIllustrationItem(size): the delegating overload and the not-found EntryNotFound throw, both previously uncalled. - SearchIterator::getDbData(): the uninitialized-guard path. - Reader::getMemorySize(): across all three Reader implementations, branching on real type (dynamic_cast) rather than factory identity. - MultiPartFileReader: small/whole/boundary/large reads, aligned or not, against a synthetic 3-part compound (all 8 access patterns). - Entry::getItem()'s redirect guard and multi-hop redirect chains. - zim::getVersions()/printVersions(): full expected content, built the same way src/version.cpp does, not substring/non-empty checks. - StringItem/FileItem round-trip, Item::getHints()'s base default, and getAmendedHints()'s FRONT_ARTICLE/COMPRESS mimetype guesses (both the zero and non-zero cases for each). Verified: full local build + `meson test`, all passing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
soloturn
force-pushed
the
test/misc-coverage-gaps
branch
from
October 5, 2026 16:50
a5b214a to
b0ddd96
Compare
Author
addressed @veloman-yunkan comments. |
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
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.
Split out of #1119 - these tests aren't related to that PR's fd-based Xapian fix, they cover pre-existing gaps found while looking at overall project coverage.
Three small, targeted additions, each amending an existing test rather than adding new fixtures where one already existed:
Archive::getChecksum()had zero test coverage anywhere - added a check that it returns a valid 32-char hex digest.Archive::getIllustrationItem(size)(the size-only overload, and its not-foundEntryNotFoundthrow) was never called by any test, despiteopenCreatedArchivealready building an archive with two real illustrations to exercise it on.SearchIterator::getDbData()'s uninitialized-guard path was the one method missing from the otherwise-thoroughsearch_iterator.uninitializedtest.(
getIllustrationSizes()andSearchIterator::getSize()are both deprecated - left alone rather than fighting-Werrorfor soon-to-be-removed APIs.)Verified: full local build +
meson test, 31/31 passing, on top of plainmain.