On Fri, 22 Sep 2023 22:16:47 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
>> 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.,
Is it necessary to pass the wrapper argument if we are setting main.wrapper
property? Can't DebuggeeWrapper just pick up the wrapper type from the property
instead of the wrapper argument?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15874#discussion_r1334869054
