RFR: 8304839: Move TestScaffold.main() to the separate class DebugeeWrapper [v2]

Leonid Mesnik lmesnik at openjdk.org
Fri Sep 22 22:19:11 UTC 2023


On Fri, 22 Sep 2023 21:44:28 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> Leonid Mesnik has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fixed comment
>
> test/jdk/com/sun/jdi/TestScaffold.java line 555:
> 
>> 553:         if (mainWrapper != null && !argInfo.targetAppCommandLine.isEmpty()) {
>> 554:             argInfo.targetVMArgs += "-D" + DebuggeeWrapper.PROPERTY_NAME + "=" + mainWrapper;
>> 555:             argInfo.targetAppCommandLine = DebuggeeWrapper.class.getName() + ' '
> 
> I know this is a pre-existing issue, but it just seems strange that we have to both set the main.wrapper property to "Virtual" and also pass "Virtual" as the first argument to DebuggeeWrapper.main(). Could we leave it up to DebuggeeWrapper.main() to set main.wrapper when the first arg is "Virtual"?

It may break the tests that check this property in static initializers of debugee. In such case method could be called before main() and return an empty value if a property is not set yet.

Probably the DebuggerWrapper requires more documentation about it's usage. I could add it separately with refactoring.,

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15874#discussion_r1334851533


More information about the serviceability-dev mailing list