RFR: 8146435: [TESTBUG] Logging tests are failing intermittently on windows when executed by JPRT

[Ioi Lam]

Hi Rachel,

The new code looks good. Thanks for being patient with my requests :-)

- Ioi

On 1/19/16 9:51 AM, Rachel Protacio wrote:
> Hello, again!
>
> A call for brand new reviews; I've corrected the code as requested by 
> Ioi, i.e. none of the tests use "java -version" at all and the 
> ItablesTest forces the use of vtables, even when class sharing is 
> enabled (as on windows). Thanks to Coleen for guiding me through the 
> latter.
>
> The fixes pass JPRT once and for all! Webrev at 
> http://cr.openjdk.java.net/~rprotacio/8146435.02/
>
> I changed the copyrights in ClassB.java and VtablesTest.java because I 
> had erroneously written them as 2015 when they weren't actually 
> checked in until 2016.
>
> Thank you,
> Rachel
>
>
> On 1/13/2016 2:26 PM, Ioi Lam wrote:
>>
>>
>> On 1/13/16 7:32 AM, Rachel Protacio wrote:
>>> Hi,
>>>
>>> Yes, we have agreed not to use "-version" for triggering potentially 
>>> changeable processes. Ioi, I think you may have misread my code as I 
>>> did not use "java -version" for ItablesTest.java. 
>> Hi Rachel,
>>
>> Thanks for the explanation. I looked at ItablesTest again. It uses 
>> ClassB, which is pretty simple and by itself won't generate the 
>> "vtable index" output. If it's not too much effort, instead of 
>> removing the that output from the test, I would suggest changing 
>> ClassB so that it would produce the desired output.
>>
>> Thanks
>>
>> - Ioi
>>
>>
>>> I had left it in the DefaultMethodsTest.java and 
>>> ClassInitializationTest.java tests because they are testing for 
>>> output that seemed to be independent of -version implementation, in 
>>> particular, java/lang/String default methods and any initialization 
>>> that had no class initialization side effects (such as 
>>> java.io.OutputStream), respectively. But I can make them more robust 
>>> by running on a dummy class.
>>>
>>> As for the CDS, I realize I misinterpreted your conclusion. I can 
>>> get rid of "-Xshare:off" and remove the parts of the tests looking 
>>> for lines that disappear when CDS is enabled.
>>>
>>> Thanks,
>>> Rachel
>>>
>>> On 1/12/2016 9:45 PM, David Holmes wrote:
>>>> I agree with Ioi. This is similar to the monitor 
>>>> inflation/deflation logging test - we need to ensure we use 
>>>> something guaranteed to always generate the expected log output.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 13/01/2016 11:50 AM, Ioi Lam wrote:
>>>>> Rachel,
>>>>>
>>>>> I don't think adding "-Xshare:off" is the correct fix. For example,
>>>>> ItablesTest.java just runs "java -version", and depends on the 
>>>>> fact that
>>>>> at least one of the JDK classes that are implicitly loaded has an 
>>>>> itable
>>>>> (or whatever contents that produces the "vtable index " output).
>>>>>
>>>>> This behavior depends on internal JDK implementation and there's no
>>>>> guarantee that it will always be true. There's also no guarantee that
>>>>> the behavior will be the same across all the OS ports.
>>>>>
>>>>> In the future, some changes of "java -version" may cause this test to
>>>>> fail only on a particular platform. I don't want someone to spend 
>>>>> a day
>>>>> to track down some imaginative windows issues again just to find out
>>>>> that it's a test bug.
>>>>>
>>>>> So, as I mentioned in the bug report, the tests should be written to
>>>>> load a class that *you have written* to ensure that the output 
>>>>> will be
>>>>> there regardless of JDK implementation.
>>>>>
>>>>> Here are other cases where the tests are dependent on unspecified
>>>>> behaviors:
>>>>>
>>>>> out.shouldContain("[Initialized").shouldContain("without side
>>>>> effects]");
>>>>>
>>>>>       All of the output in DefaultMethodsTest.java
>>>>>
>>>>> In fact, I think you should remove "-Xshare:off" from the tests. 
>>>>> Adding
>>>>> -Xshare:off will also force the tests to be executed with CDS 
>>>>> disabled,
>>>>> and would cover up problems that only show up when CDS is enabled.
>>>>>
>>>>> Thanks
>>>>> - Ioi
>>>>>
>>>>> On 1/12/16 10:27 AM, Rachel Protacio wrote:
>>>>>> Hello!
>>>>>>
>>>>>> Please review this fix which adds "-Xshare:off" to all UL tests'
>>>>>> ProcessBuilders to prevent windows failures. Extreme thanks to 
>>>>>> Ioi for
>>>>>> debugging and finding the source of the problem.
>>>>>>
>>>>>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8146435/
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8146435
>>>>>>
>>>>>> Tests sent to JPRT, pass on all platforms.
>>>>>>
>>>>>> Thank you!
>>>>>> Rachel
>>>>>
>>>
>>
>