RFR(XXS) : 8177709 : Convert TestVirtualSpace_test to GTest
Igor Ignatyev
igor.ignatyev at oracle.com
Wed Oct 17 19:00:43 UTC 2018
EXPECT would just mark a test as failed and allow the method to continue, so rs.release() will be called.
test fixture will basically do the same as an RAII object, but it will be a bit more obvious as test preparation and cleaning steps will be separated from test actions, so I'll go w/ setUp/tearDown.
-- Igor
> On Oct 17, 2018, at 11:53 AM, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
>
> On Wed, Oct 17, 2018 at 8:29 PM Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
>>
>> Hi Thomas,
>>
>> 1st of all, thanks for looking at it, appreciate that.
>>
>> it seems we can. there are two ways I can fix that either replace ASSERT by EXCEPT or introduce a text fixture w/ SetUp and TearDown. which do you prefer?
>>
>> -- Igor
>
> Slight preference for SetUp/TearDown (EXCEPT would quit the testing
> altogether, wont it?).
>
> Lazy me would have just used an RAII object like this:
>
> struct Cleaner {
> ReservedSpace* rs;
> ~Cleaner() { rs.release(); }
> };
>
> But I leave it up to you.
>
> ... Thomas
>
>>
>>> On Oct 17, 2018, at 11:16 AM, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
>>>
>>> Hi Igor,
>>>
>>> test_virtual_space_actual_committed_space() and
>>> actual_committed_space_one_large_page():
>>>
>>> Won't we leak the reserved space now when we run into an ASSERT and
>>> then continue with the next gtests?
>>>
>>> Cheers, Thomas
>>>
>>> On Wed, Oct 17, 2018 at 7:55 PM Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
>>>>
>>>> http://cr.openjdk.java.net/~iignatyev//8177709/webrev.00/index.html
>>>>> 141 lines changed: 140 ins; 1 del; 0 mod;
>>>>
>>>> Hi all,
>>>>
>>>> could you please review this small (and hopefully trivial) patch which converts internal TestVirtualSpace_test to gtest?
>>>> again, the old test is still used by WhiteBox::runMemoryUnitTests, the old code hasn't been removed. it will be removed later when all 4 tests used by runMemoryUnitTests are converted.
>>>>
>>>> for the sake of reviewer, http://hg.openjdk.java.net/jdk/jdk/file/2e928420389d/src/hotspot/share/memory/virtualspace.cpp#l1261 is the old version.
>>>>
>>>> webrev: http://cr.openjdk.java.net/~iignatyev//8177709/webrev.00/index.html
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8177709
>>>> testing:
>>>> - converted tests on linux-x64, windows-x64, macosx-x64, solaris-sparcv9 in product and fastdebug variants
>>>> - build w/ precompiled-headers enabled and disabled
>>>>
>>>> Thanks,
>>>> -- Igor
>>
More information about the hotspot-dev
mailing list