<div><br></div><div><br><div class="gmail_quote"><div dir="ltr">On Wed 17. Oct 2018 at 21:00, Igor Ignatyev <<a href="mailto:igor.ignatyev@oracle.com">igor.ignatyev@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">EXPECT would just mark a test as failed and allow the method to continue, so rs.release() will be called. </blockquote><div dir="auto"><br></div><div dir="auto">Oh, okay. Thanks for clarifying.</div><div dir="auto"><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
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.</blockquote><div dir="auto"><br></div><div dir="auto">Okay fine by me.</div><div dir="auto"><br></div><div dir="auto">..thomas</div><div dir="auto"><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
-- Igor<br>
<br>
> On Oct 17, 2018, at 11:53 AM, Thomas Stüfe <<a href="mailto:thomas.stuefe@gmail.com" target="_blank">thomas.stuefe@gmail.com</a>> wrote:<br>
> <br>
> On Wed, Oct 17, 2018 at 8:29 PM Igor Ignatyev <<a href="mailto:igor.ignatyev@oracle.com" target="_blank">igor.ignatyev@oracle.com</a>> wrote:<br>
>> <br>
>> Hi Thomas,<br>
>> <br>
>> 1st of all, thanks for looking at it, appreciate that.<br>
>> <br>
>> 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?<br>
>> <br>
>> -- Igor<br>
> <br>
> Slight preference for SetUp/TearDown (EXCEPT would quit the testing<br>
> altogether, wont it?).<br>
> <br>
> Lazy me would have just used an RAII object like this:<br>
> <br>
> struct Cleaner {<br>
>  ReservedSpace* rs;<br>
>  ~Cleaner() { rs.release(); }<br>
> };<br>
> <br>
> But I leave it up to you.<br>
> <br>
> ... Thomas<br>
> <br>
>> <br>
>>> On Oct 17, 2018, at 11:16 AM, Thomas Stüfe <<a href="mailto:thomas.stuefe@gmail.com" target="_blank">thomas.stuefe@gmail.com</a>> wrote:<br>
>>> <br>
>>> Hi Igor,<br>
>>> <br>
>>> test_virtual_space_actual_committed_space() and<br>
>>> actual_committed_space_one_large_page():<br>
>>> <br>
>>> Won't we leak the reserved space now when we run into an ASSERT and<br>
>>> then continue with the next gtests?<br>
>>> <br>
>>> Cheers, Thomas<br>
>>> <br>
>>> On Wed, Oct 17, 2018 at 7:55 PM Igor Ignatyev <<a href="mailto:igor.ignatyev@oracle.com" target="_blank">igor.ignatyev@oracle.com</a>> wrote:<br>
>>>> <br>
>>>> <a href="http://cr.openjdk.java.net/~iignatyev//8177709/webrev.00/index.html" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~iignatyev//8177709/webrev.00/index.html</a><br>
>>>>> 141 lines changed: 140 ins; 1 del; 0 mod;<br>
>>>> <br>
>>>> Hi all,<br>
>>>> <br>
>>>> could you please review this small (and hopefully trivial) patch which converts internal TestVirtualSpace_test to gtest?<br>
>>>> 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.<br>
>>>> <br>
>>>> for the sake of reviewer, <a href="http://hg.openjdk.java.net/jdk/jdk/file/2e928420389d/src/hotspot/share/memory/virtualspace.cpp#l1261" rel="noreferrer" target="_blank">http://hg.openjdk.java.net/jdk/jdk/file/2e928420389d/src/hotspot/share/memory/virtualspace.cpp#l1261</a> is the old version.<br>
>>>> <br>
>>>> webrev: <a href="http://cr.openjdk.java.net/~iignatyev//8177709/webrev.00/index.html" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~iignatyev//8177709/webrev.00/index.html</a><br>
>>>> JBS: <a href="https://bugs.openjdk.java.net/browse/JDK-8177709" rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8177709</a><br>
>>>> testing:<br>
>>>> - converted tests on linux-x64, windows-x64, macosx-x64, solaris-sparcv9 in product and fastdebug variants<br>
>>>> - build w/ precompiled-headers enabled and disabled<br>
>>>> <br>
>>>> Thanks,<br>
>>>> -- Igor<br>
>> <br>
<br>
</blockquote></div></div>