RFR: 8311775: [TEST] duplicate verifyHeapDump in several tests [v2]

Yi Yang yyang at openjdk.org
Fri Aug 11 02:46:59 UTC 2023


On Thu, 10 Aug 2023 09:41:29 GMT, Yi Yang <yyang at openjdk.org> 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


More information about the serviceability-dev mailing list