RFR: 8292054: Test runtime/posixSig/TestPosixSig.java fails with 'Test failed, bad output.' [v2]

[Harold Seigel]

On Wed, 17 Aug 2022 13:56:21 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Harold Seigel has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Improve gtest based on review comments
>
> test/hotspot/gtest/runtime/test_signals.cpp line 29:
> 
>> 27: #include "memory/resourceArea.hpp"
>> 28: #include "runtime/os.hpp"
>> 29: #include "utilities/defaultStream.hpp"
> 
> Probably don't need this, but need `ostream.hpp`

Fixed

> test/hotspot/gtest/runtime/test_signals.cpp line 53:
> 
>> 51:     act.sa_flags = 0;
>> 52:     ASSERT_NE(sigaction(SIGFPE, &act, &old_SIGFPE_act), -1) << "Setting SIGFPE handler failed";
>> 53:     ASSERT_NE(sigaction(SIGILL, &act, &old_SIGILL_act), -1) << "Setting SIGILL handler failed";
> 
> Could you add `<< "(" << errno << ")"` to the error text? May need errno.h.

Done

> test/hotspot/gtest/runtime/test_signals.cpp line 59:
> 
>> 57:     stringStream st;
>> 58:     os::run_periodic_checks(&st);
>> 59:     char* res = st.as_string();
> 
> By using "const char* res = st.base()" instead of as_string(), you can access the string buffer without copying and don't need to use a ResourceMark.

Fixed

> test/hotspot/gtest/runtime/test_signals.cpp line 76:
> 
>> 74: 
>> 75: // This tests the fix for JDK-8285792.
>> 76: TEST_VM(PosixSignalTest, check_handlers) {
> 
> Please use TEST_OTHER_VM. Your test changes global state and thus pollutes the VM for followup tests.

Fixed.

-------------

PR: https://git.openjdk.org/jdk/pull/9882