RFR: 8316693: Simplify at-requires checkDockerSupport()

Daniel D. Daugherty dcubed at openjdk.org
Thu Oct 12 16:58:53 UTC 2023


On Thu, 12 Oct 2023 02:00:11 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Please review this simple change.
>> 
>> Currently JTReg @requires extension test/jtreg-ext/requires/VMProps.java checkDockerSupport() uses "<CONTAINER_ENGINE> ps" command to check whether test host/environment allows for container testing. With this change proposing to switch it to "which <CONTAINER_ENGINE>":
>>   - the "ps" command could be heavy-weight in some environments, taking longer time then expected
>>   - it may not apply to all container engine types
>
> test/jtreg-ext/requires/VMProps.java line 607:
> 
>> 605:         ProcessBuilder pb = new ProcessBuilder("which", Container.ENGINE_COMMAND);
>> 606:         Map<String, String> logFileNames =
>> 607:             redirectOutputToLogFile("checkDockerSupport(): which <container-engine>",
> 
> Maybe it's better for the message to include the actual name of the engine?
> 
> 
> redirectOutputToLogFile("checkDockerSupport(): which " + Container.ENGINE_COMMAND),

I don't see how this comment was resolved. Did I miss something?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16111#discussion_r1357121400


More information about the hotspot-runtime-dev mailing list