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

[Hohensee, Paul]

Haven't run the test, but comparison with the JDK 13 patch looks fine to me.

Paul

On 10/9/20, 1:38 PM, "jdk8u-dev on behalf of Severin Gehwolf" <jdk8u-dev-retn at openjdk.java.net on behalf of sgehwolf at redhat.com> wrote:

    Hi Andrew,

    On Thu, 2020-09-24 at 11:39 +0200, Severin Gehwolf wrote:
    > On Mon, 2020-08-31 at 16:35 +0200, Severin Gehwolf wrote:
    > > 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!
    >
    > Ping?

    Ping v2? This is blocking some other docker test fixes which will
    enable better testing of to-come 8u container backports. Thanks!

    Cheers,
    Severin