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

Ioi Lam ioi.lam at oracle.com
Tue Jan 19 23:12:34 UTC 2016

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

More information about the hotspot-runtime-dev mailing list