RFR(XXS) : 8177709 : Convert TestVirtualSpace_test to GTest

[Thomas Stüfe]

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
> >>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20181017/528e0604/attachment.htm>