On Mon, 16 Aug 2021 23:47:04 GMT, Igor Ignatyev <iignat...@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()
>
> 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.

you also don't need to have `.` as a first argument, `Paths.get("x")` returns 
you a path to "x" in current directory.

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

PR: https://git.openjdk.java.net/jdk/pull/5134

Reply via email to