On Mon, 16 Aug 2021 23:31:41 GMT, Mikhailo Seledtsov <[email protected]>
wrote:
> Please review this change that updates the buildJdkDockerImage() test library
> API.
>
> This work originated while working on "8195809: [TESTBUG] jps and jcmd -l
> support for containers is not tested".
> The initial intent was to extend the buildJdkDockerImage() API of
> DockerTestUtils to accept custom Dockerfile content.
> As I analyzed the usage of buildJdkDockerImage() I realized that:
> - 2nd argument "dockerfile" is always the same: "Dockerfile-BasicTest"
> its use has been obsolete for some time, in favor of Dockerfile
> generated by DockerTestUtils
> - 3rd argument "buildDirName" is also always the same: "jdk-docker"
>
> Hence I thought it would be a good idea to simplify this API and make it
> up-to-date.
>
> Also, since the method signature is being updated, I thought it would be a
> good idea to also change the name to use more generic container terminology:
> buildJdkDockerImage() --> buildJdkContainerImage()
Changes requested by iignatyev (Reviewer).
test/lib/jdk/test/lib/containers/docker/DockerTestUtils.java line 145:
> 143:
> 144: // Create an image build/staging directory
> 145: Path buildDir = Paths.get(".", "image-build");
to make it more robust and possible to have more than one container within a
test, I'd use `imagName` + "image-build" as build dir name.
test/lib/jdk/test/lib/containers/docker/DockerTestUtils.java line 179:
> 177: * @throws Exception
> 178: */
> 179: public static void buildImage(String imageName, Path buildDir)
> throws Exception {
it seems there are no users of this method, do we need to keep it public?
-------------
PR: https://git.openjdk.java.net/jdk/pull/5134
