On Thu, 6 May 2021 21:37:35 GMT, Yumin Qi <mi...@openjdk.org> wrote: >> Hi, Please review >> When using jcmd to dump shared archive, if the archive name exists, it >> will be deleted first. If exception happens during dumping process, there is >> no new archive created. This PR changes to first dump the archive with a >> temporary file name. With successful dump, the temporary file will be rename >> to its given name. This way the old archive will not be touched on exception. >> The newly added test case skipped testing on Windows since File operation >> result is not same as on Linux. >> >> Tests: tier1,tier2,tier3,tier4 >> >> Thanks >> Yumin > > Yumin Qi has updated the pull request incrementally with one additional > commit since the last revision: > > Remove tab space
I have 2 questions on the test. test/hotspot/jtreg/runtime/cds/appcds/jcmd/JCmdTestFileSafety.java line 78: > 76: print2ln(test_count++ + " Set target dir not writable, do dynamic > dump"); > 77: setKeepArchive(true); > 78: outputDirFile.setWritable(true); Should the comment be `// Set target dir writable ...` ? (since you're setting the dir to writable at line 78) test/hotspot/jtreg/runtime/cds/appcds/jcmd/JCmdTestFileSafety.java line 89: > 87: outputDirFile.setWritable(false); > 88: test(localFileName, pid, noBoot, EXPECT_FAIL); > 89: outputDirFile.setWritable(true); Would the test fail without setting the dir not writable at line 87? ------------- PR: https://git.openjdk.java.net/jdk/pull/3886
