RFR: 8316693: Simplify at-requires checkDockerSupport()
Ioi Lam
iklam at openjdk.org
Thu Oct 12 02:03:20 UTC 2023
On Tue, 10 Oct 2023 00:06:12 GMT, Mikhailo Seledtsov <mseledtsov 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
LGTM
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),
-------------
Marked as reviewed by iklam (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/16111#pullrequestreview-1672873463
PR Review Comment: https://git.openjdk.org/jdk/pull/16111#discussion_r1355956973
More information about the hotspot-runtime-dev
mailing list