On Mon, 16 Aug 2021 23:31:41 GMT, Mikhailo Seledtsov <mseledt...@openjdk.org> 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