On Mon, 27 Feb 2023 21:13:47 GMT, Lance Andersen <lan...@openjdk.org> wrote:
>> Eirik Bjorsnos has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains ten additional >> commits since the last revision: >> >> - Replace the u8, u16, u32 methods with using a little-endian ByteBuffer. >> Collapse the checkZipException and checkZipExceptionInGetInputStream into >> one method by always consuming the InputStream. Add a block comment for the >> assertZipException method. >> - Use block comments instead of javadoc comments >> - Merge branch 'master' into corrupted-zip-files-ng >> - Merge branch 'master' into corrupted-zip-files-ng >> - Give the @BeforeMethod and @AfterMethod more meaningful names >> - Improve comments and method names to help future maintainers understand >> what these tests verify. >> - Merge branch 'master' into corrupted-zip-files-ng >> - Trim whitespace and fix some spelling >> - Convert test CorruptedZipFiles to testNG and delete ZipFile constants >> copies which are now obsolete. > > Hi Eirik, > > Thank you for your suggested changes to this test. > > I think if we are going to re-work this test, we should go further including > improving the comments for future maintainers > > I made a quick pass and some initial thoughts are below > > Best > Lance > @LanceAndersen Do you have any opinion on junit/testNG for tests like this? Here's a junit version for consideration: https://github.com/eirbjo/jdk/blob/corrupted-zip-files-ng-junit/test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java ------------- PR Comment: https://git.openjdk.org/jdk/pull/12563#issuecomment-1488678652
