RFR(XS): 8189213: [TESTBUG] Running jtreg tests on machine without docker shows extra message

[David Holmes]

Hi Misha,

On 3/11/2017 10:22 AM, mikhailo wrote:
> Hi David,
> 
>    Thank you for taking a look at this change.
> 
> 
> On 11/02/2017 04:47 PM, David Holmes wrote:
>> Hi Misha,
>>
>> I don't see anything else in VMProps that writes anything out so to me 
>> the fix is to delete the System.err.println completely.
> Removal is an option, and seems a simpler one. However, we will loose 
> the diagnostic information.
> On the other hand, if an issue arises on a specific host or system, I 
> can patch this file with extra trace statements, and diagnose the 
> failure that way.
> If you think removing the print statement is a better option, and I do 
> not hear objections or other opinions, I can just remove the print 
> statement.

Delete please :)

>> That said I did not look at this in the past but I feel the whole 
>> docker test here is not really appropriate/suitable to always be run 
>> on linux-x64. It has to exec another process and wait up to 10 seconds 
>> to get a result! Can't this actually be done via a WhiteBox check once 
>> Bob's container support updates are in place?
> Actually, WhiteBox is not necessary for such check. The ability to run 
> docker on a given host/node can be determined in a body of a jtreg test, 
> via a test utility. In fact, that was my original intent. When this 
> change was presented for a discussion, this aspect was debated and 
> reviewers have reached a consensus to do such check in VMProps.java. We 
> mainly discussed within VM SQE team. The main argument was that using 
> @requires is a more proper way of "skipping" the test(s) on unsupported 
> environments, rather then checking this in the body of the test and then 
> returning. When using @requires, the test execution system will report 
> the test as "not run". When skipping the test using "return" from the 
> body of a test, the execution system will report test as "ran" and 
> passed; jtreg does not yet support the "skipped" state.
> 
> Unfortunately, the way our "@requires" mechanism works is that it runs 
> each time prior to starting a test run, even if the test being run is 
> not concerned with a specific property. It was my concern that this 
> could be a burden on the test run startup, but I eventually agreed to 
> the opinion of other reviewers.
> 
> If you have a strong opinion or concern about this, let me know. 
> However, I am not sure what is the best process to use to overturn a 
> decision on a prior review or design discussion. I guess one option is 
> to file a bug or an RFE, and bring it up for a discussion.

Yes let's file a bug/RFE. VMProps can use WhiteBox and I think using 
WhiteBox connected to Bob's new container support code is better than 
exec'ing a separate process (which I'm not even sure what it does).

> 
> For this change under review, if I do not hear opinions from other 
> reviewers, I can simply delete the print statement.

Sounds good to me.

Thanks,
David

> 
> Thank you,
> Misha
>>
>> Thanks,
>> David
>>
>> On 3/11/2017 7:03 AM, mikhailo wrote:
>>> Please review this simple change that makes the error message in 
>>> jtreg-ext/requires/VMProps.java conditional
>>> on java property "vmprops.trace.verbose". W/o this condition an error 
>>> message is printed each time any jtreg
>>> test is ran, including non-docker tests.
>>>
>>>      JBS: https://bugs.openjdk.java.net/browse/JDK-8189213
>>>      Webrev: http://cr.openjdk.java.net/~mseledtsov/8189213.00/
>>>
>>>
>>>
>>> ======= Background and details:
>>> VMProps.java is called for each test run of jtreg (be it a run 
>>> containing a singe test or multiple tests).
>>> VMProps evaluates the @requires properties. Therefore, in case of 
>>> this bug, any time a user runs any jtreg VM test
>>> on a machine w/o an operational docker engine, this error will pop up.
>>> The fix makes printing of this error message conditional, off by 
>>> default. The message can be turend on
>>> via a property vmprops.trace.verbose when detailed diagnostic info is 
>>> desired.
>>>
>>>
>>>
>>> ======== Testing:
>>> Local testing:
>>>    W/o docker present:
>>>      jtreg <arbitrary-jtreg-test>
>>>        No error message
>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>        Detailed error message
>>>      jtreg DockerBasicTest.java
>>>        Test skipped
>>>
>>>    With docker installed but no permissions:
>>>      jtreg <arbitrary-jtreg-test>
>>>        No error message
>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>        Detailed error message
>>>      jtreg DockerBasicTest.java
>>>        Test skipped
>>>
>>>    With docker installed and operational:
>>>      jtreg <arbitrary-jtreg-test>
>>>        No error message
>>>      jtreg -Dvmprops.trace.verbose=true <arbitrary-jtreg-test>
>>>        No error message
>>>      jtreg DockerBasicTest.java
>>>        Test passed
>>>
>>> Testing via automated distributed test system:
>>>    - tier1
>>>    - docker tests
>>>    In progress
>>>
>>>
>>> Thank you,
>>> Misha
>>>
>