RFR: 8304834: Fix wrapper insertion in TestScaffold.parseArgs(String args[]) [v4]
Chris Plummer
cjplummer at openjdk.org
Tue Apr 11 17:43:41 UTC 2023
On Tue, 11 Apr 2023 16:55:42 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:
>> The TestScaffold incorrectly parse options, it should insert wrapper class between VM options and applications classame.
>
> Leonid Mesnik has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains eight commits:
>
> - updated after David's comments.
> - Merge branch 'master' of https://github.com/openjdk/jdk into 8289546
> - updated parsing.
> - added comments and trim arguments
> - parsing updted
> - problemlist fixed
> - comments
> - fix
Changes requested by cjplummer (Reviewer).
test/jdk/com/sun/jdi/TestScaffold.java line 471:
> 469: // Parse arguments, like java/j* tools command-line arguments
> 470: // the first argument not-starting with '-' is treated as a classname
> 471: // the other arguments are split to targetVMArgs targetAppCommandLine correspondingly
Can you make these proper sentences with the first letter uppercase and a period at the end.
test/jdk/com/sun/jdi/TestScaffold.java line 475:
> 473: // The result without any wrapper enabled:
> 474: // argInfo.targetAppCommandLine : Frames2Targ
> 475: // argInfo.targetVMArgs : -Xss4M
The 2 argInfo comments would be easier to read if indented 2 or 4 spaces (after the //, not before).
test/jdk/com/sun/jdi/TestScaffold.java line 478:
> 476: // The result with wrapper enabled:
> 477: // argInfo.targetAppCommandLine : TestScaffold Virtual Frames2Targ
> 478: // argInfo.targetVMArgs : -Xss4M
The 2 argInfo comments would be easier to read if indented 2 or 4 spaces (after the //, not before).
test/jdk/com/sun/jdi/TestScaffold.java line 500:
> 498: redefineAsynchronously = true;
> 499: } else if (arg.startsWith("-J")) {
> 500: throw new RuntimeException("-J-option format is not supported. Incorrect arg: " + args[i]);
David pointed out that you can use arg instead of args[i] here.
-------------
PR Review: https://git.openjdk.org/jdk/pull/13170#pullrequestreview-1379784054
PR Review Comment: https://git.openjdk.org/jdk/pull/13170#discussion_r1163138251
PR Review Comment: https://git.openjdk.org/jdk/pull/13170#discussion_r1163138841
PR Review Comment: https://git.openjdk.org/jdk/pull/13170#discussion_r1163138984
PR Review Comment: https://git.openjdk.org/jdk/pull/13170#discussion_r1163137029
More information about the serviceability-dev
mailing list