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

Harold Seigel hseigel at openjdk.org
Wed Aug 17 13:44:38 UTC 2022


On Tue, 16 Aug 2022 15:12:43 GMT, Coleen Phillimore <coleenp 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 52:
> 
>> 50:     sigemptyset(&act.sa_mask);
>> 51:     act.sa_flags = 0;
>> 52:     if (sigaction(SIGFPE, &act, &old_SIGFPE_act) == -1) {
> 
> Instead of perror() which I haven't seen in the gtests (maybe it's ok), we usually use ASSERT statements like:
> ASSERT_NE(sigaction(SIGFPE, &act, &old_SIGFPE_act), -1) << sigaction(SIGFPE) failed;

I changed this to use ASSERT_*.

> test/hotspot/gtest/runtime/test_signals.cpp line 66:
> 
>> 64:     // Set tty to local stringStream to capture output from run_periodic_checks()
>> 65:     // calls to print_signal_handlers().
>> 66:     tty = &st;
> 
> Would it be possible to add an outputStream* st parameter to run_periodic_checks() instead of doing this? I believe that function only uses tty twice.

Done.

> test/hotspot/gtest/runtime/test_signals.cpp line 76:
> 
>> 74:       return "restoring SIGFPE handler failed";
>> 75:     }
>> 76:     if (sigaction(SIGILL, &old_SIGILL_act, 0)) {
> 
> Not == -1 ?

Fixed

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

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


More information about the hotspot-runtime-dev mailing list