RFR: 8228434: jdk/net/Sockets/Test.java fails after JDK-8227642

Mandy Chung mandy.chung at oracle.com
Fri Jul 19 17:32:06 UTC 2019



On 7/19/19 10:13 AM, Severin Gehwolf wrote:
> Hi Mandy,
>
> On Fri, 2019-07-19 at 09:58 -0700, Mandy Chung wrote:
>> Hi Severin,
>>
>> On 7/19/19 9:55 AM, Severin Gehwolf wrote:
>>>> There might be other tests with policy files where this is not the case.
>>> My issue is with finding those tests :-/ If we know the set of *all*
>>> tests affected by the breakage we could do approach 2. Approach 1 (or
>>> 3) seems safer.
>>>
>>>> Severin - how about a combination of the two approaches, meaning add
>>>> Docker.DOCKER_COMMAND as per the first version but use
>>>> privilegedGetProperty to read the value. That way only container tests
>>>> using a SM and their own policy files will need to grant the permission
>>>> to read this property.
>>> Sure, fine with me. Here you go:
>>> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8228434/02/webrev/
>>>
>>>
>> It seems better to define Docker as a new top-level class as Platform
>> doesn't seem the right place to define DOCKER_COMMAND.
> Note that Platform.java is already on the boot library path for jtreg
> and moving it to a separate class would need another entry here (in
> TEST.ROOT):
>
> requires.extraPropDefns.bootlibs = ../lib/sun ../lib/jdk/test/lib/Platform.java
>
> The reason I've moved it to Platform for JDK-8227642 was that
> VMProps.java previously hard-coded the docker command (it needs to use
> the property value instead).

It accepts multiple files, doesn't it?

> This seems work for little gain. Note that the intention is to change
> "Docker" and DOCKER_COMMAND to something more neutral, like Container
> or ContainerEngine. In light of that, Platform.ContainerEngine.COMMAND
> makes sense to me.

This is a test library specific to containers.  "Container" sounds fine. 
   Why not doing it now?

> Could I perhaps convince you to accept this version?
>
> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8228434/02/webrev/

I still think separating container-specific APIs in its own class will 
prevent running similar kind of issue in the future.

Mandy


More information about the core-libs-dev mailing list