On Thu, 6 May 2021 23:59:21 GMT, Calvin Cheung <cche...@openjdk.org> wrote:
>> Yumin Qi has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove tab space > > 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) The comment is for the testing item --- that is consistent with println contents. The first set 'true' is for get the archive and keep the archive, the real test is after set it to 'false', the test will fail and we check the previous archive still available. > 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? See reply above. ------------- PR: https://git.openjdk.java.net/jdk/pull/3886
