RFR: 8347840: Fix testlibrary compilation warnings

David Holmes dholmes at openjdk.org
Thu Jan 16 06:09:46 UTC 2025


On Wed, 15 Jan 2025 23:48:33 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:

> There few compiler warning disabled in the testlibary build.
> They should be fixed or localized and removed from build to prevent new possible issues.
> 
> The main goal is to avoid new such issues in the testlibrary. 
> Tested with tier1-5 to ensure that all tests were passed.

Overall looks good - sometimes a bit of a puzzler understanding what warning is being addressed. :)

Only one thing I'm concerned may be an issue. Also a couple of suggestions.

Thanks

test/lib/jdk/test/lib/format/ArrayDiff.java line 110:

> 108:      * @return an ArrayDiff instance for the two arrays and formatting parameters provided
> 109:      */
> 110:     @SuppressWarnings({"rawtypes", "unchecked"})

Just wondering where the unchecked warning arises in the code?

test/lib/jdk/test/lib/hprof/model/JavaHeapObject.java line 42:

> 40:  *
> 41:  * @author      Bill Foote
> 42:  */

I would suggest deleting any comment blocks that just have the @author tag, and deleting the @author elsewhere. All these files (lib/hprof) already have an author attribution comment.

test/lib/jdk/test/lib/hprof/parser/ReadBuffer.java line 46:

> 44:     public int   getInt(long pos) throws IOException;
> 45:     public long  getLong(long pos) throws IOException;
> 46:     public void  close() throws IOException;

Why was this redefined to throw IOException rather than just Exception?

test/lib/jdk/test/lib/hprof/parser/Reader.java line 96:

> 94:             } else if ((i >>> 8) == GZIP_HEADER_MAGIC) {
> 95:                 // Possible gziped file, try decompress it and get the stack trace.
> 96:                 in.close();

It is not obvious to me that there may not be a reason for closing all of the streams before opening new ones below.

test/lib/jdk/test/lib/hprof/parser/Reader.java line 169:

> 167:             } else if ((i >>> 8) == GZIP_HEADER_MAGIC) {
> 168:                 // Possible gziped file, try decompress it and get the stack trace.
> 169:                 in.close();

It is not obvious to me that there may not be a reason for closing all of the streams before opening new ones below.

test/lib/jdk/test/lib/thread/VThreadRunner.java line 100:

> 98:             if ((X) ex == ex) {
> 99:                 throw (X) ex;
> 100:             }

This doesn't make sense to me.

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

PR Review: https://git.openjdk.org/jdk/pull/23143#pullrequestreview-2554831705
PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1917773365
PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1917775999
PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1917780287
PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1917795069
PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1917795554
PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1917787207


More information about the build-dev mailing list