On Fri, 10 Mar 2023 08:28:37 GMT, Eirik Bjorsnos <d...@openjdk.org> wrote:
>> Please review this PR which brings the DataDescriptorSignatureMissing test >> back to life. >> >> This test currently calls out to Python to create a test vector ZIP with a >> Data Descriptor without the recommended but optional signature. The Python >> dependency has turned out to be very brittle, so the test is currently >> marked with `@ignore` >> >> The PR replaces Python callouts with directly creating the test vector ZIP >> in the test itself. We can then remove the `@ignore`tag and run this useful >> test automatically. > > Eirik Bjorsnos has updated the pull request incrementally with one additional > commit since the last revision: > > Revert change to Google copyright line, add an Oracle copyright line > instead. It's good that this test is being revived and no longer relies on an external tool/program to generate a zip which was triggering the issue in https://bugs.openjdk.org/browse/JDK-8056934. The changes look good to me. In order to verify that without the fix in https://bugs.openjdk.org/browse/JDK-8056934, this test fails (i.e. reproduces the issue), I reverted the fix that was done in that issue (`git revert 3951dda4cf06c6e61e19d3df26a792022c1701b9`) and then built the JDK and ran this updated test. It runs into a NullPointerException within the test because the second entry in the zip is missing after the zip is read through the `ZipInputStream`. Could you add this following patch to your test: diff --git a/test/jdk/java/util/zip/DataDescriptorSignatureMissing.java b/test/jdk/java/util/zip/DataDescriptorSignatureMissing.java index 5efdc59de63..636cecb4851 100644 --- a/test/jdk/java/util/zip/DataDescriptorSignatureMissing.java +++ b/test/jdk/java/util/zip/DataDescriptorSignatureMissing.java @@ -39,6 +39,7 @@ import java.nio.charset.StandardCharsets; import java.util.zip.*; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; public class DataDescriptorSignatureMissing { @@ -55,10 +56,12 @@ public class DataDescriptorSignatureMissing { try (ZipInputStream in = new ZipInputStream( new ByteArrayInputStream(zip))) { ZipEntry first = in.getNextEntry(); + assertNotNull(first, "Zip file is unexpectedly missing first entry"); assertEquals(first.getName(), "first"); assertEquals(in.readAllBytes(), "first".getBytes(StandardCharsets.UTF_8)); ZipEntry second = in.getNextEntry(); + assertNotNull(second, "Zip file is unexpectedly missing second entry"); assertEquals(second.getName(), "second"); assertEquals(in.readAllBytes(), "second".getBytes(StandardCharsets.UTF_8)); } so that instead of running into a NullPointerException, the test will (rightly) reproduce and report the missing second entry? ------------- PR: https://git.openjdk.org/jdk/pull/12959
