Integrated: 8338729: Retire the test jdk/java/util/zip/TestZipError.java

[Eirik Bjørsnøs]

On Wed, 21 Aug 2024 09:59:38 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:

> Please review this test-only, cleanup PR which proposes that we retire the `jdk/java/util/zip/TestZipError.java` test.
> 
> The test has fallen out of sync with its original and stated purpose described by its name, `@summary` tag and code comments. The error condition actually being exercised has existing coverage in the `ZipSourceCache` test.
> 
> Background:
> 
> * The test was initially added in JDK-4615343 (Java 6) to exercise code paths which caused `ZipFile` entry enumeration to throw a `ZipError` (a subclass of `InternalError`).
> * After the rewrite of the ZipFile API from native to Java in JDK-8145260 (JDK 9), the Java code now tested no longer throws `ZipError` (The `ZipError` class is no longer used in OpenJDK)
> * As part of JDK-8145260, `TestZipError.java` was updated to catch `ZipException` instead of `ZipError` for the expected/passing case. Interestingly, a line was added which caused the test to not simply enumerate the entries of the updated ZipFile, but to also consume the input stream of the entries.
> * The added line caused `ZipFileInputStream.initDataOffset` to throw a `ZipException` when the CEN local header offset does not contain the expected LOC header, resulting in a ZipException with the message "ZipFile invalid LOC header (bad signature)". This added line caused the test to (accidentally?) test something which is actually different than its original purpose. 
> * The "invalid LOC header" scenario is currently better tested by the `ZipSourceCache` JUnit test, which also runs on non-Windows platforms.
> * Note that JDK-8145260 did not update the copyright years in the license header of this file.
> 
> Considering all of the above, I suggest that we do not invest resources in bringing the naming, summary and code comments of this test in line with what is actually being tested. Instead, we should simply retire it in favor of the existing `ZipSourceCache` test.
> 
> Testing:
> 
> * No tests are updated in this PR, I added a `noreg-cleanup` label in the JBS
> * By collecting a stack trace, I verified that the `ZipException` caught by `TestZipError` is indeed caused by the LOC header validation logic in `ZipFileInputStream.initDataOffset`
> * Similarly, I also verified that `ZipSourceCache` throws on the same condition as `TestZipError`

This pull request has now been integrated.

Changeset: b6700095
Author:    Eirik Bjørsnøs <eirbjo at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/b6700095c018a67a55b746cd4eee763c68f538e0
Stats:     116 lines in 1 file changed: 0 ins; 116 del; 0 mod

8338729: Retire the test jdk/java/util/zip/TestZipError.java

Reviewed-by: lancea

-------------

PR: https://git.openjdk.org/jdk/pull/20660