RFR(S): 8166910, 8166911: Convert TestNewSize_test and TestOldSize_test to GTest
Jesper Wilhelmsson
jesper.wilhelmsson at oracle.com
Wed Oct 19 20:45:42 UTC 2016
Looks good!
/Jesper
Den 19/10/16 kl. 19:49, skrev Kirill Zhaldybin:
> Jesper,
>
> Thank you for reviewing the fix!
>
> Here are a new WebRev:
> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8166910/webrev.01/
>
> Changes:
> 1. I created a small class MinHeapSizeGuard which stores min_heap_size value and
> restores it after we leave TestWrapper::test scope.
> 2. I also changed TEST to TEST_VM since otherwise I got assert on not defined
> page size in debug build.
>
> Could you please let me know your opinion?
>
> Thank you.
>
> Regards, Kirill
>
> On 19.10.2016 16:40, Jesper Wilhelmsson wrote:
>> Hi Kirill,
>>
>> In TestWrapper you modify _min_heap_size, but this variable is not
>> protected by the FLAG_GUARD macros. (It is not a flag so it can't be.) I
>> think you need to restore it's value since you run some of your tests in
>> the same VM as other tests.
>>
>> Besides that the test looks OK. I'm not 100% comfortable with the
>> executor classes, but I don't have to be either :)
>> /Jesper
>>
>>
>> Den 13/10/16 kl. 21:26, skrev Kirill Zhaldybin:
>>> Dear all,
>>>
>>> Could you please review this fix for 8166910 and 8166911?
>>>
>>> Changes:
>>> 1. All asserts changed to GTest's
>>> 2. The test separated to 4 scenarios.
>>> 3. Since the initial internal test could (and actually did) impact
>>> other tests I
>>> moved test scenarios which use FLAG_SET_CMDLINE to other vm. Otherwise
>>> even
>>> after successful execution running the same test again caused failure.
>>> 4. Flag storage changed to new nice FLAG_GUARD macros.
>>> 5. The test deeply refactored from straight scenarios to the series of
>>> executors
>>> and test wrapper which runs them.
>>>
>>> WebRev:
>>> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8166910/webrev.00/
>>>
>>> CR: https://bugs.openjdk.java.net/browse/JDK-8166910
>>> https://bugs.openjdk.java.net/browse/JDK-8166911
>>>
>>> Thank you.
>>>
>>> Regards, Kirill
>>>
>>>
>
More information about the hotspot-gc-dev
mailing list