RFR: 8315406: [REDO] serviceability/jdwp/AllModulesCommandTest.java ignores VM flags [v2]
Chris Plummer
cjplummer at openjdk.org
Fri Sep 1 21:53:12 UTC 2023
On Fri, 1 Sep 2023 21:47:53 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:
>> The fix changes test serviceability/jdwp/AllModulesCommandTest.java to accept VM flags.
>> 1) The 'ProcessTools.createTestJvm(JDWP_OPT, DEBUGGEE);' is used to start debugee
>> 2) The stderr is just logging and tests doesn't check if it is empty. stderr might contain some VM output which doesn't mean failure
>> 3) Some refactoring is done. Method ' public void onStringRead(StreamHandler handler, String line) {' doesn't need to check which handler is used and corresponding code and methods are deleted. The field 'inputHandler' and 'errorHandler' were moved into method.
>>
>> Tested with tier1-5 to ensure that test correctly work with CI options.
>>
>> The serviceability/jdwp/AllModulesCommandTest.java is the only one test in serviceability/jdwp directory so no other tests should be affected.
>>
>> I closed my previous PR https://github.com/openjdk/jdk/pull/15499 because GitHub incorrectly show changes after merge with backout if the first fix.
>
> Leonid Mesnik has updated the pull request incrementally with one additional commit since the last revision:
>
> Update test/hotspot/jtreg/serviceability/jdwp/DebuggeeLauncher.java
>
> Co-authored-by: Chris Plummer <chris.plummer at oracle.com>
test/hotspot/jtreg/serviceability/jdwp/DebuggeeLauncher.java line 66:
> 64: /**
> 65: * Starts the debuggee with the necessary JDWP options and handles the
> 66: * debuggee's stdout output. stderr migth contain jvm output and just printed to the log
Suggestion:
* debuggee's stdout output. stderr might contain jvm output, which is just printed to the log.
test/hotspot/jtreg/serviceability/jdwp/DebuggeeLauncher.java line 100:
> 98:
> 99: @Override
> 100: public void onStringRead(String line) {
Is there a reason not to also echo the stdout output? Seems it would be useful if the test ever failed. Doing so probably means adding back in the StreamHandler argument.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15544#discussion_r1313553274
PR Review Comment: https://git.openjdk.org/jdk/pull/15544#discussion_r1313560196
More information about the serviceability-dev
mailing list