RFR: 7903781: Report the process id of the agent or other VM that was used for a jtreg action [v2]

[Jonathan Gibbons]

On Fri, 26 Jul 2024 06:54:56 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Can I please get a review of this change which proposes to repor the process id of the process that was used to run a jtreg action? This change addresses https://bugs.openjdk.org/browse/CODETOOLS-7903781.
>> 
>> As noted in that linked enhancement request, this change will report the process id  the `section` messages of each action. This will now look like:
>> 
>> 
>> #section:compile
>> ----------messages:(7/297)----------
>> command: compile test/streams/FileDescriptorTest.java
>> reason: .class file out of date or does not exist
>> started: Fri Jul 26 00:56:01 UTC 2024
>> Mode: othervm
>> Process id: 88469
>> finished: Fri Jul 26 00:56:01 UTC 2024
>> elapsed time (seconds): 0.254
>> 
>> ....
>> 
>> #section:main
>> ----------messages:(7/232)----------
>> command: main FileDescriptorTest
>> reason: User specified action: run main FileDescriptorTest 
>> started: Fri Jul 26 00:56:01 UTC 2024
>> Mode: othervm
>> Process id: 88470
>> finished: Fri Jul 26 00:56:01 UTC 2024
>> elapsed time (seconds): 0.042
>> 
>> Notice the new "Process id: ..." line in those sections.
>
> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
> 
>   include a self test for verifying the process id in the generated report

Mostly good; some minor comments/suggestions.

src/share/classes/com/sun/javatest/regtest/exec/Agent.java line 581:

> 579:     /**
> 580:      * Returns the process id of the {@code AgentServer} with which this {@code Agent}
> 581:      * communicates. Returns {@code -1} if the process id of the {@code AgentServer}

As a matter of style, it is better if the first sentence is "complete", so I suggest changing ". Returns" to " or ".


      * Returns the process id of the {@code AgentServer} with which this {@code Agent}
      * communicates or {@code -1} if the process id of the {@code AgentServer}
      * couldn't be determined.

src/share/classes/com/sun/javatest/regtest/util/ProcessUtils.java line 58:

> 56:             pidMethod = null;
> 57:         }
> 58:         PID_METHOD = pidMethod;

These two blocks of code are functionally similar but stylistically different in the names used.
I suggest making them similar, perhaps changing to use DESTROY_FORCIBLY_METHOD.

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

PR Review: https://git.openjdk.org/jtreg/pull/215#pullrequestreview-2241426761
PR Review Comment: https://git.openjdk.org/jtreg/pull/215#discussion_r1719056393
PR Review Comment: https://git.openjdk.org/jtreg/pull/215#discussion_r1719060481