From 8bb9a2895ee344ddea737f4df173c944b9a6cbf0 Mon Sep 17 00:00:00 2001 From: Madelyn Olson <34459052+madolson@users.noreply.github.com> Date: Mon, 8 Jan 2024 17:56:06 -0800 Subject: [PATCH] Address some failures with new tests for improving debug report (#12915) Fix a daily test failure because alpine doesn't support stack traces and add in an extra assertion related to making sure the stack trace was printed twice. --- tests/integration/logging.tcl | 16 ++-------------- tests/support/util.tcl | 19 +++++++++++++++++++ tests/unit/moduleapi/crash.tcl | 29 ++++++++++++++++++++--------- 3 files changed, 41 insertions(+), 23 deletions(-) diff --git a/tests/integration/logging.tcl b/tests/integration/logging.tcl index 1ed3cc4d8..b547cd8fa 100644 --- a/tests/integration/logging.tcl +++ b/tests/integration/logging.tcl @@ -1,22 +1,10 @@ tags {"external:skip"} { set system_name [string tolower [exec uname -s]] -set backtrace_supported 0 +set backtrace_supported [system_backtrace_supported] set threads_mngr_supported 0 ;# Do we support printing stack trace from all threads, not just the one that got the signal? - -# We only support darwin or Linux with glibc -if {$system_name eq {darwin}} { - set backtrace_supported 1 -} elseif {$system_name eq {linux}} { +if {$system_name eq {linux}} { set threads_mngr_supported 1 - # Avoid the test on libmusl, which does not support backtrace - # and on static binaries (ldd exit code 1) where we can't detect libmusl - catch { - set ldd [exec ldd src/redis-server] - if {![string match {*libc.*musl*} $ldd]} { - set backtrace_supported 1 - } - } } # look for the DEBUG command in the backtrace, used when we triggered diff --git a/tests/support/util.tcl b/tests/support/util.tcl index 4136fb17e..9b9ea0ac1 100644 --- a/tests/support/util.tcl +++ b/tests/support/util.tcl @@ -1123,3 +1123,22 @@ proc format_command {args} { set _ $cmd } +# Returns whether or not the system supports stack traces +proc system_backtrace_supported {} { + set system_name [string tolower [exec uname -s]] + if {$system_name eq {darwin}} { + return 1 + } elseif {$system_name ne {linux}} { + return 0 + } + + # libmusl does not support backtrace. Also return 0 on + # static binaries (ldd exit code 1) where we can't detect libmusl + catch { + set ldd [exec ldd src/redis-server] + if {![string match {*libc.*musl*} $ldd]} { + return 1 + } + } + return 0 +} diff --git a/tests/unit/moduleapi/crash.tcl b/tests/unit/moduleapi/crash.tcl index 82b32f954..dedbb1a1e 100644 --- a/tests/unit/moduleapi/crash.tcl +++ b/tests/unit/moduleapi/crash.tcl @@ -1,6 +1,7 @@ # This file is used to test certain crash edge cases to make sure they produce # correct stack traces for debugging. set testmodule [file normalize tests/modules/crash.so] +set backtrace_supported [system_backtrace_supported] # Valgrind will complain that the process terminated by a signal, skip it. if {!$::valgrind} { @@ -20,8 +21,11 @@ if {!$::valgrind} { wait_for_log_messages 0 {"*=== REDIS BUG REPORT END. Make sure to include from START to END. ===*"} $loglines 10 1000 assert_equal 1 [count_log_message 0 "=== REDIS BUG REPORT END. Make sure to include from START to END. ==="] assert_equal 2 [count_log_message 0 "ASSERTION FAILED"] - # There will be 3 crash assertions, 1 in the first stack trace and 2 in the second - assert_equal 3 [count_log_message 0 "assertCrash"] + if {$backtrace_supported} { + # Make sure the crash trace is printed twice. There will be 3 instances of, + # assertCrash 1 in the first stack trace and 2 in the second. + assert_equal 3 [count_log_message 0 "assertCrash"] + } assert_equal 1 [count_log_message 0 "RECURSIVE ASSERTION FAILED"] assert_equal 1 [count_log_message 0 "=== REDIS BUG REPORT START: Cut & paste starting from here ==="] } @@ -34,18 +38,25 @@ if {!$::valgrind} { set res [wait_for_log_messages 0 {"*=== REDIS BUG REPORT START: Cut & paste starting from here ===*"} 0 10 1000] set loglines [lindex $res 1] - set res [wait_for_log_messages 0 {"*Crashed running the instruction at*"} $loglines 10 1000] - set loglines [lindex $res 1] + if {$backtrace_supported} { + set res [wait_for_log_messages 0 {"*Crashed running the instruction at*"} $loglines 10 1000] + set loglines [lindex $res 1] - set res [wait_for_log_messages 0 {"*Crashed running signal handler. Providing reduced version of recursive crash report*"} $loglines 10 1000] - set loglines [lindex $res 1] - set res [wait_for_log_messages 0 {"*Crashed running the instruction at*"} $loglines 10 1000] - set loglines [lindex $res 1] + set res [wait_for_log_messages 0 {"*Crashed running signal handler. Providing reduced version of recursive crash report*"} $loglines 10 1000] + set loglines [lindex $res 1] + set res [wait_for_log_messages 0 {"*Crashed running the instruction at*"} $loglines 10 1000] + set loglines [lindex $res 1] + } wait_for_log_messages 0 {"*=== REDIS BUG REPORT END. Make sure to include from START to END. ===*"} $loglines 10 1000 assert_equal 1 [count_log_message 0 "=== REDIS BUG REPORT END. Make sure to include from START to END. ==="] assert_equal 1 [count_log_message 0 "Crashed running signal handler. Providing reduced version of recursive crash report"] - assert_equal 2 [count_log_message 0 "Crashed running the instruction at"] + if {$backtrace_supported} { + assert_equal 2 [count_log_message 0 "Crashed running the instruction at"] + # Make sure the crash trace is printed twice. There will be 3 instances of + # modulesCollectInfo, 1 in the first stack trace and 2 in the second. + assert_equal 3 [count_log_message 0 "modulesCollectInfo"] + } assert_equal 1 [count_log_message 0 "=== REDIS BUG REPORT START: Cut & paste starting from here ==="] } }