On Tue, 4 Apr 2023 23:52:45 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
>> test/jdk/com/sun/jdi/VMConnection.java line 54: >> >>> 52: // When we run under jtreg, test.classes contains the pathname >>> of >>> 53: // the dir in which the .class files will be placed. >>> 54: // When we run jtreg with plugin, the TestScaffold should be >>> also in classpath >> >> A stupid question about this part of comment: `the TestScaffold should be >> also in classpath`. >> If it should be ALSO in classpath then why in the `Virtual` case we use >> "test.class.path" instead of "test.classes" but not both. It'd be good to >> clarify this in the comment. > > The "test.class.path" is complete classpath required to run test. It included > test.classes as well as testlibrary and some other classes. The tests located > in com/sun/jdi/<subdirectory> uses 'com/sun/jdi' as a testlibrary. > The better comment would be > // When we run jtreg with plugin, the full classpath including testlibrary is > required to use TestScaffold is required > > However, before fixing this comment I would like to see if there are any > objections against using "test.class.path" in all cases. It would minimize > difference in virtual and standard mode. > Is there are any reason to use "test.classes" at all? I'd prefer to always use the "test.class.path" as it is a complete classpath required to run test. > The better comment would be > // When we run jtreg with plugin, the full classpath including testlibrary is > required to use TestScaffold is required I guess, you are going to remove this duplication with "is required". :) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13339#discussion_r1157911621
