On Mon, 21 Sep 2020 12:18:40 GMT, Alan Bateman <al...@openjdk.org> wrote:
> 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. I have logged JDK-8253444 to add more coverage for setDictionary. I would prefer to put this change back to address the offset not being used and improve the coverage afterwards. > 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. I updated the test name(s). > > 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. Addressed the issues above. I have pushed the updated changes for review to the PR. ------------- PR: https://git.openjdk.java.net/jdk/pull/269
