RFR(XXS) : 8177709 : Convert TestVirtualSpace_test to GTest

[Thomas Stüfe]

Looks both good Igor. +1 for the const pointers :)

..Thomas
On Wed, Oct 17, 2018 at 11:45 PM Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
>
> Hi Thomas, JC
>
> I tried to use a test fixture and the code became very clumsy, so I went w/ a simple RAII.
> http://cr.openjdk.java.net/~iignatyev//8177709/webrev.0-1/index.html
>
> I've also noticed that we have same problem in TestReservedSpace_test tests -- http://cr.openjdk.java.net/~iignatyev//8171097/webrev.0-1/index.html
>
> Thanks,
> -- Igor
>
> On Oct 17, 2018, at 12:04 PM, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
>
>
>
> On Wed 17. Oct 2018 at 21:00, Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
>>
>> EXPECT would just mark a test as failed and allow the method to continue, so rs.release() will be called.
>
>
> Oh, okay. Thanks for clarifying.
>
>>
>>
>> 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.
>
>
> Okay fine by me.
>
> ..thomas
>
>>
>>
>> -- 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
>> >>
>>
>