RFR(S): 8164039: Convert test_memset_with_concurrent_readers to GTest

[Kirill Zhaldybin]

Kim,

Here are a new WebRev: 
http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8164039/webrev.03/
I changed output from printf to stream which then printed by ASSERT.

Could you please let me know your opinion?

Thank you.

Regards, Kirill

On 19.08.2016 23:41, Kim Barrett wrote:
>> 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.
>