On Wed, 15 Jan 2025 23:48:33 GMT, Leonid Mesnik <lmes...@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
