RFR - JDK-8134432: [TESTBUG] Rewrite test/runtime/6888954/vmerrors.sh in Java

George Triantafillou george.triantafillou at oracle.com
Wed Oct 7 15:39:43 UTC 2015


Thanks Coleen.

-George

On 10/6/2015 2:49 PM, Coleen Phillimore wrote:
>
>
> On 10/6/15 2:35 PM, George Triantafillou wrote:
>> Hi Coleen,
>>
>> On 10/5/2015 3:47 PM, Coleen Phillimore wrote:
>>>
>>> George, This looks like a nice improvement!
>>>
>>> It's sort of unfortunate that this test has to be kept in sync with 
>>> debug.cpp but is not run with JPRT.   So someone could check in a 
>>> change that breaks it.   Since it is platform independent, is there 
>>> a way to exclude it _only_ for Solaris?
>> Thanks for your review.  While it's possible to restrict the test to 
>> non-Solaris platforms, there's currently no means to specify this 
>> restriction only for JPRT runs.  In addition, the original test has 
>> always been restricted to never run under JPRT.
>
> Ok.
>
>>
>>>
>>> Also, I don't know the convention for tests or Java, but it would 
>>> have saved me time to find the runTest method above main where it's 
>>> called.
>> I can move the runTest method if you'd like.  Let me know. Thanks.
>
> Yes, you can move it or not.  I don't need to see another code review 
> for that.
>
> Coleen
>>
>> -George
>>>
>>> Coleen
>>>
>>> On 10/5/15 3:36 PM, George Triantafillou wrote:
>>>> Further testing indicates that test execution times on Solaris are 
>>>> prohibitively long, so the test has been excluded from JPRT test runs.
>>>>
>>>> New webrev:
>>>>
>>>> http://cr.openjdk.java.net/~gtriantafill/8134432/webrev.02/ 
>>>> <http://cr.openjdk.java.net/%7Egtriantafill/8134432/webrev.02/>
>>>>
>>>> Thanks.
>>>>
>>>> -George
>>>>
>>>> On 10/5/2015 9:56 AM, Dmitry Dmitriev wrote:
>>>>> Hi George,
>>>>>
>>>>> Looks good to me!
>>>>>
>>>>> Thanks,
>>>>> Dmitry
>>>>>
>>>>> On 02.10.2015 19:50, George Triantafillou wrote:
>>>>>> Hi Dmitry,
>>>>>>
>>>>>> Thanks very much for your comments.  Since checking the page size 
>>>>>> is not the intention of this test, I've removed the hard coded 
>>>>>> values. In addition, I've removed the two previously missing test 
>>>>>> cases since they both failed on Windows.
>>>>>>
>>>>>> New webrev:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~gtriantafill/8134432/webrev.01/ 
>>>>>> <http://cr.openjdk.java.net/%7Egtriantafill/8134432/webrev.01/>
>>>>>>
>>>>>> -George
>>>>>>
>>>>>> On 9/28/2015 9:43 AM, Dmitry Dmitriev wrote:
>>>>>>> Hi George,
>>>>>>>
>>>>>>> For test cases 2, 4 and 6 you expect to see "num=4096" in the 
>>>>>>> output(lines 48, 50, 52). As I see from 
>>>>>>> src/share/vm/utilities/debug.cpp, 'num' in this case is equal to 
>>>>>>> the 'vm_page_size'.
>>>>>>> Probably will be better not use hard coded value(4096) in the 
>>>>>>> test and use pattern for that string? Or use "getVMPageSize" 
>>>>>>> white box method to get page size value?
>>>>>>>
>>>>>>> Otherwise looks good!
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Dmitry
>>>>>>>
>>>>>>> On 25.09.2015 18:05, George Triantafillou wrote:
>>>>>>>> Please review this fix for JDK-8134432:
>>>>>>>>
>>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8134432
>>>>>>>> webrev: 
>>>>>>>> http://cr.openjdk.java.net/~gtriantafill/8134432/webrev.00/ 
>>>>>>>> <http://cr.openjdk.java.net/%7Egtriantafill/8134432/webrev.00/>
>>>>>>>>
>>>>>>>> The test was rewritten in Java, and two previously missing test 
>>>>>>>> cases were added to the test.
>>>>>>>>
>>>>>>>> The fix was tested with RBT on all platforms with the JPRT 
>>>>>>>> hotspot_all testset.  Thanks.
>>>>>>>>
>>>>>>>> -George
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>



More information about the hotspot-runtime-dev mailing list