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

[mikhailo]

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.

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


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


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