RFR(S): 8164039: Convert test_memset_with_concurrent_readers to GTest

[Kim Barrett]

> On Aug 19, 2016, at 1:00 PM, Kirill Zhaldybin <kirill.zhaldybin at oracle.com> wrote:
> 
> Kim,
> 
> Thank you for reviewing the fix!
> 
> Here are a new WebRev: http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8164039/webrev.02/
> Changes:
> 1. TEST_VM and print_cr were changed to TEST and printf

I wonder whether printf is appropriate, but I can’t find any indication of where gtest directs its
output, nor any way to change it.  Are you sure it writes to stdout rather than stderr?  If the
latter, or if gtest provides options for directing output somewhere other than stdout, then printf
isn’t the right function.

> 2. I kept EXPECTs so the test could check all 3 boolean flags but also added
> ASSERT_TRUE(head_clear && middle_set && tail_clear);
> after them which stops the test execution if any of flags are false and prevents huge amount of not so needed output.
> 
> Could you please let me know you opinion?

Looks good, subject to the above question about using printf.

> 
> Thank you.
> 
> Regards, Kirill
> 
> 
> On 19.08.2016 00:48, Kim Barrett wrote:
>>> 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.