Make no-JHDF tests opt-in - #4467
Open
Darlokt wants to merge 1 commit into
Open
Conversation
Move the dependency-excluded JHDF service test into an explicit Maven profile and remove it from the default Ant test target. Keep CI coverage by running the profile in Maven builds while preventing focused normal tests from inheriting the stripped classpath.
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.
Hej,
While investigating the
MemoizerTestfailures mentioned in #4464, I found that the problem was caused by the Maven test configuration rather than a problem in the normal Memoizer tests.formats-bsdhad two Surefire executions in the default lifecycle: the regular suite and a second execution with JHDF removed from the classpath. When-Dtest=MemoizerTestwas used, Surefire applied that filter to both executions, so Memoizer tests were unexpectedly run without JHDF.The normal tests passed with JHDF available. On the stripped classpath, the default
Memoizercreates anImageReader, which eagerly instantiates all registered readers, including HDF readers. Kryo then inspected the complete reader graph and attempted to resolveHDF5CompoundDataMap, causing memo serialization to fail.saveMemo()caught the failure and returnedfalse, producing the two reported assertion failures.As far as I can tell the no-JHDF suite was originally intended only to verify that JHDF service discovery fails cleanly when the dependency is absent, but not intended to run arbitrary tests without JHDF.
Changes
Move the no-JHDF compatibility test into an explicit
no-jhdfMaven profile.The changes:
no-jhdfprofile.test-no-jhdffrom the default Ant test target while retaining the explicit Ant target.mvn install -Pno-jhdf, preserving coverage of both the normal and missing-JHDF suites.MissingJHDFServiceTestunchanged.This change does not alter Memoizer serialization or claim to add Memoizer support for missing JHDF. It isolates the existing compatibility test and makes the test contract explicit. It mostly improves dev experience.
The serializer should still be hardened separately. Even with the no-JHDF suite isolated, users running Bio-Formats without optional HDF dependencies or similar can still lose memoization for otherwise supported non-HDF etc. files because Kryo serializes the entire eagerly discovered ImageReader graph. Unused HDF readers should not make an unrelated memo fail. A future fix should serialize only the active reader and wrapper chain, reconstruct discovery from the current runtime classpath, record or validate required optional dependencies, bump the memo format version, and report unsupported memoization clearly instead of silently returning false. No matter if the separate no-JHDF suite is still needed, this is a central problem with the memoizer.