RFR: 8272398: Update DockerTestUtils.buildJdkDockerImage() [v2]
Harold Seigel
hseigel at openjdk.java.net
Tue Aug 17 20:35:23 UTC 2021
On Tue, 17 Aug 2021 14:58:48 GMT, Mikhailo Seledtsov <mseledtsov at 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()
>
> Mikhailo Seledtsov has updated the pull request incrementally with one additional commit since the last revision:
>
> Addressing review feedback
Other than that one obsolete comment the changes look good.
Thanks, Harold
test/lib/jdk/test/lib/containers/docker/DockerTestUtils.java line 175:
> 173: /**
> 174: * Build a container image based on given Dockerfile and image build directory.
> 175: *
This comment looks wrong because there is no longer a given Dockerfile.
-------------
Marked as reviewed by hseigel (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5134
More information about the serviceability-dev
mailing list