RFR: 8227642: [TESTBUG] Make docker tests podman compatible

[Severin Gehwolf]

Hi Igor,

On Wed, 2019-07-17 at 11:37 -0700, Igor Ignatyev wrote:
> 
> Hi Severin,
> 
> the updated webrev looks good to me, please see a couple comments below.

Thanks. More below.

> Cheers,
> -- Igor
> 
> > On Jul 17, 2019, at 10:34 AM, Severin Gehwolf <sgehwolf at redhat.com> wrote:
> > 
> > Hi Misha,
> > 
> > On Wed, 2019-07-17 at 10:22 -0700, mikhailo.seledtsov at oracle.com wrote:
> > > Hi Severin,
> > > 
> > > On 7/17/19 5:44 AM, Severin Gehwolf wrote:
> > > > Hi Igor, Misha,
> > > > 
> > > > On Tue, 2019-07-16 at 11:49 -0700, Igor Ignatyev wrote:
> > > > > Hi Severin,
> > > > > 
> > > > > I don't think that tests (or test libraries for that matter) should
> > > > > be responsible for setting correct PATH value, it should be a part of
> > > > > host configuration procedure (tests can/should check that all
> > > > > required bins are available though). in other words, I'd prefer if
> > > > > you remove 'env.put("PATH", ...)' lines from both DockerTestUtils and
> > > > > TestJFREvents. the rest looks good to me.
> > > > Updated webrev:
> > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8227642/02/webrev/
> > > > 
> > > > No more additions to PATH are being done.
> > > > 
> > > > I've discovered that VMProps.java which defines "docker.required", used
> > > > the "docker" binary even for podman test runs. This ended up not
> > > > running most of the tests even with -Djdk.test.docker.command=podman
> > > > specified.
> > > Good catch.
> should we rename docker.support and DOCKER_COMMAND to something more abstract?

container.support and CONTAINER_ENGINE_COMMAND perhaps?

> > > > I've fixed that by moving DOCKER_COMMAND to Platform.java so
> > > > that it can be used in both places.
> > > 
> > > Sounds good to me.
> > > 
> > > (of course, the alternative would be to import 
> > > jdk.test.lib.containers.docker.DockerTestUtils into VMProps.java -- not 
> > > sure if there are any potential problems doing it this way)
> > 
> > I've tried that but for some reason this was a problem and VMProps
> > failed to compile. I don't know exactly how those jtreg extensions work
> > and went with the Platform approach. Hope that's OK.
> 
> all files needed for VMProps (or other @requires expression class)
> have to be listed in requires.extraPropDefns or
> requires.extraPropDefns.bootlibs property in TEST.ROOT file in all
> the test suites which use these extensions. we are trying to be very
> cautious in what is used by VMProps (directly and indirectly) so
> these lists won't grow and we won't require any modules other than
> java.base, given DockerTestUtils has dependencies on a number of
> other library classes, the Platform approach is much better from that
> point of view.

I had a feeling it was something like that. Thanks for the explanation!

Cheers,
Severin