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

[Severin Gehwolf]

Hi Andrew,

On Wed, 2020-08-26 at 04:38 +0100, Andrew Hughes wrote:
> 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.

OK.

> 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.

Yes, I've noticed it. Question is whether this is worth fixing. Some
tests rely on test/lib/testlibrary/jdk/testlibrary some others
on test/lib/jdk/test/lib.

> 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.

I'm not sure I agree but wouldn't object a patch which does this. It
seems a significant piece of work and would require some re-testing of
commonly run tests. Either way, this seems beyond the point of this
patch (as you said).

> > 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.

Sure. Fixed:
https://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8221342/jdk8/02/

> Other changes look fine.

Thanks for the review!

Cheers,
Severin