Fix java/util/zip/DeflateIn_InflateOut.skipBytes Test #559
Conversation
|
@keithc-ca / @pshipton I have marked this as WIP, till we test it through grinder and figure out delivery process (First in OpenJDK master or external). Would appreciate your review to polish it off. |
|
I don't have any concerns accepting a test change before submitting it to OpenJDK. There should be comments included in the code that explain the changes. |
r30shah
force-pushed
the
testRestSize
branch
2 times, most recently
from
9da3031 to
9bb20ea
Compare
r30shah
changed the title
r30shah
force-pushed
the
testRestSize
branch
3 times, most recently
from
3eeedd1 to
6d92003
Compare
|
@pshipton I have added a comment in the code to mention why input data is fixed for checking skipping of different number of bytes in Also wanted to check with @backwaterred if it is possible to contribute this to OpenJDK repo? |
|
@keithc-ca I recall you reviewed a similar change elsewhere, if you want to look at this one. |
java/util/zip/DeflateIn_InflateOut.skipBytes verifies functionality of DeflaterInputStream.skipBytes. While testing skipping of bytes, test uses different input data with same data length, and compares the compressed sizes of these data to verify skipping bytes. As per zlib specification which is what is used by Deflater in java, length of compressed data is highly dependable on input data contents, so it will be incorrect to assume that compression ratio would be constant for same length input data. For more details please see explaination in eclipse-openj9/openj9#14948 (comment). Signed-off-by: Rahil Shah <[email protected]>
r30shah
force-pushed
the
testRestSize
branch
from
6d92003 to
ee1497e
Compare
|
Pls port it to jdk17, 19, next. |
|
also 8 |
|
@pshipton Looking at the source code for same test in JDK17,JDK19 and JDKNext [1][2][3], seems like they have already fixed this. Though it keeps the data constant (One of the suggestion @keithc-ca had was to perform different test with different sets of data. I did port the change back in ibmruntimes/openj9-openjdk-jdk8#609 though. Let me know what do you think. Use the fix in this PR for JDK8 and JDK11 which would perform test with different input data (Only keep data consistent for skipBytes test which was failing) or want to port the changes from JDKnext. |
|
Perhaps we should be applying fixes closer to the change made in jdk17? |
|
Backporting the changes from jdk17+ is preferred. |
|
The only difference this fix and the one JDK17 has is use of different input data for different tests in this fix. If this is OK, I agree, it is preferable to have changes backported from JDK17. |
java/util/zip/DeflateIn_InflateOut.skipBytes verifies functionality of
DeflaterInputStream.skipBytes. While testing skipping of bytes, test
uses different input data with same data length, and compares the
compressed sizes of these data to verify skipping bytes. As per zlib
specification which is what is used by Deflater in java, length of
compressed data is highly dependable on input data contents, so it will
be incorrect to assume that compression ratio would be constant for same
length input data. For more details please see explaination in
eclipse-openj9/openj9#14948 (comment).
Signed-off-by: Rahil Shah [email protected]