[8u] RFR(s): 8221342: [TESTBUG] Generate Dockerfile for docker testing

Andrew Hughes gnu.andrew at redhat.com
Wed Aug 26 03:38:40 UTC 2020


On 15:05 Tue 25 Aug     , Severin Gehwolf wrote:
> Hi!
> 
> Please review this 8u backport of this test bug which enhances
> container testing to automatically generate docker files. This is
> useful if the development platform is not Oracle Linux 7.X (which is
> the case for me).
> 
> The JDK 11 patch applies cleanly (after JDK-8220313 which is in the
> approval queue)

I don't see how this can be said to apply cleanly. Attempting to
shuffle the patch fails to find a mapping for
test/lib/jdk/test/lib/containers/docker/DockerTestUtils.java and
test/lib/jdk/test/lib/containers/docker/DockerfileConfig.java.

It is also beyond the scope of a script to know to duplicate these
changes between two repositories. So this alone justifies a review.

This is beyond the scope of this patch, but the proposed patch
for the JDK tests seems wrong. We seem to have duplication in the
JDK test tree, caused by the JFR backport.

The jdk/test/lib/testlibrary/jdk/testlibrary/containers directory
would be the correct location for these tests. The JFR backport seems
to have started an alternate tree at jdk/test/lib/jdk/test/lib with
duplication of some files in the former directory. This should be
resolved in the 8u282 lifecycle.

> but doesn't compile since Files.writeString() API is
> not available in JDK 8. Replaced it with:
> 
> byte[] bytes = dockerFileStr.getBytes(StandardCharsets.UTF_8);        
> Files.write(dockerfile, bytes);
>

Could this not be simplified to:

Files.write(dockerfile, dockerFileStr.getBytes(StandardCharsets.UTF_8));

I don't see the need for the intermediate variable.

Other changes look fine.

Thanks,
-- 
Andrew :)

Senior Free Java Software Engineer
OpenJDK Package Owner
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222


More information about the jdk8u-dev mailing list