On Sun, 20 Sep 2020 22:22:47 GMT, Lance Andersen <lan...@openjdk.org> wrote:
>> Hi all, >> >> Please review the fix for JDK-8252739 which addresses an issue introduced by >> https://bugs.openjdk.java.net/browse/JDK-8225189, where Deflater.c ignored >> the offset specified by >> Deflater.setDictionary. Mach5 jdk-tier1, jdk-tier2, jdk-tier3 runs cleanly >> as well as the java/util/zip and >> java/util/jar JCK tests. > > Lance Andersen has updated the pull request incrementally with one additional > commit since the last revision: > > More updates to testByteBufferDirect in DeflaterDictionaryTests.java Update to Deflater.c looks good. DeflaterDictionaryTests looks like is a shaping up to be a good test for setDictionary. Are there other assertions that should be checked, e.g. setDictionary(ByteBuffer) is specified to consume all bytes and it would be good to check that the position is set to the limit. Also can the 2-arg setDictionary be tests, also corner cases such no bytes remaining or invoked with null. The naming of the tests is a bit inconsistent, e.g. ByteArrayTest vs. testByteBufferDirect. In the naming then I would use "Heap" instead of "Wrap", as in testHeapByteBuffer. In passing: The tests can use try-finally to ensure that Deflater::end is invoked even when the test fails. String repeat(int) could be used to avoid duplicating "Welcome to Us Open;". Missing space in several "if(...)" usages. ------------- PR: https://git.openjdk.java.net/jdk/pull/269
