On Thu, 10 Aug 2023 09:41:29 GMT, Yi Yang <[email protected]> wrote:
>> This is a follow-up patch of #13667. verifyHeapDump is duplicated in several
>> tests, this patch tries to consolidate them into one method.
>
> Yi Yang has updated the pull request incrementally with one additional commit
> since the last revision:
>
> fix TestHeapDumpForInvokDynamic
Thanks for reviews!
----------
@kevinjwalls
> One question on whether we need to change those "throws IOException" to
> "throws Exception", which is really me thinking if the existing HProfParser
> parse method really needs to throw Exception? Maybe it only needs
> IOException? (snapshot.resolve doesn't throw anything) But looks good anyway
> and good to share this code. 8-)
Yes, we must change it to `throws Exception` otherwise we won't compile
/jdk/test/lib/jdk/test/lib/hprof/HprofParser.java:99: error: unreported
exception Exception; must be caught or declared to be thrown
try (Snapshot snapshot = Reader.readFile(dump.getAbsolutePath(),
callStack, debugLevel)) {
^
exception thrown from implicit call to close() on resource variable 'snapshot'
I noticed another optional enhancement where I found that essentially all calls
to HprofParser.parse should now be replaced with HprofParser.parseAndVerify or
make it the standard parsing logic, but this is unrelated to my current patch.
----------
@plummercj
> Can you clarify what testing you did?
All involved tests work in Linux.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/15202#issuecomment-1674139274
