RFR(S): 8164039: Convert test_memset_with_concurrent_readers to GTest

Kim Barrett kim.barrett at oracle.com
Thu Aug 18 21:48:27 UTC 2016


> On Aug 16, 2016, at 1:44 PM, Kirill Zhaldybin <kirill.zhaldybin at oracle.com> wrote:
> 
> Igor,
> 
> Thank you for reviewing the fix!
> 
> I changed ASSERTs to EXPECTs.
> 
> Here are a new webrev: http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8164039/webrev.01/
> 
> Regards, Kirill
> 
> On 16.08.2016 20:18, Igor Ignatyev wrote:
>> Hi Kirill,
>> 
>> I think it’s better to use EXPECT_* instead of ASSERT_* in this test, so we will perform all checks in one run.
>> otherwise the change looks good to me.


Using EXPECT rather than ASSERT is very nearly equivalent to just
removing the asserts in the original code.  The existing code already
provides most of the information and a lot more.  The asserts were
added during development of the original test (earlier they were just
part of the output, with a failure indication at the very end).  The
test dumps the full block on failure, which makes it easy to see what
bits are wrong on failure.  But if there's one failure, there are
probably a lot more, and the amount of output generated becomes
unpleasant to wade through.

Also, I think this new gtest should be using TEST rather than TEST_VM.
I assume TEST_VM was used because of the output being generated via
tty->print_cr.  But rather than using that reporting mechanism, the
new test should be using EXPECT/ASSERT streaming and the like.




More information about the hotspot-gc-dev mailing list