Re: [PATCH] btrace: add unsupported/untested messages when skipping tests

Luis Machado
2016-12-09T15:32:06Z

Thanks for this patch Markus. It keeps things more organized.

In previous reviews it was deemed more appropriate to just say "failed
to compile", since the source file information could be extracted from
the testcase itself via the log file.

So could you drop printing srcfile and go with the simpler message?

Otherwise this looks good to me except for the one nit at the end.

On 12/09/2016 07:27 AM, Markus Metzger wrote:
> We may silently skip gdb.btrace tests if
>
> - the target does not support record-btrace
> - the target does not support TSX
> - the target does not support gdbserver
> - we fail to compile the test
> - we fail to run to main
>
> Add unsupported/untested messages for each of those.
>
> CC: Luis Machado <lgustavo@codesourcery.com>
>
> 2016-12-09 Markus Metzger <markus.t.metzger@intel.com>
>
> testsuite/
> * gdb.btrace/buffer-size.exp: Add unsupported/untested message if
> the test is skipped.
> * gdb.btrace/data.exp: Likewise.
> * gdb.btrace/delta.exp: Likewise.
> * gdb.btrace/dlopen.exp: Likewise.
> * gdb.btrace/enable-running.exp: Likewise.
> * gdb.btrace/enable.exp: Likewise.
> * gdb.btrace/exception.exp: Likewise.
> * gdb.btrace/function_call_history.exp: Likewise.
> * gdb.btrace/gcore.exp: Likewise.
> * gdb.btrace/instruction_history.exp: Likewise.
> * gdb.btrace/multi-thread-step.exp: Likewise.
> * gdb.btrace/nohist.exp: Likewise.
> * gdb.btrace/non-stop.exp: Likewise.
> * gdb.btrace/reconnect.exp: Likewise.
> * gdb.btrace/record_goto-step.exp: Likewise.
> * gdb.btrace/record_goto.exp: Likewise.
> * gdb.btrace/rn-dl-bind.exp: Likewise.
> * gdb.btrace/segv.exp: Likewise.
> * gdb.btrace/step.exp: Likewise.
> * gdb.btrace/stepi.exp: Likewise.
> * gdb.btrace/tailcall-only.exp: Likewise.
> * gdb.btrace/tailcall.exp: Likewise.
> * gdb.btrace/tsx.exp: Likewise.
> * gdb.btrace/unknown_functions.exp: Likewise.
> * gdb.btrace/vdso.exp: Likewise.
> ---
> gdb/testsuite/gdb.btrace/buffer-size.exp | 9 ++++++---
> gdb/testsuite/gdb.btrace/data.exp | 10 +++++++---
> gdb/testsuite/gdb.btrace/delta.exp | 10 +++++++---
> gdb/testsuite/gdb.btrace/dlopen.exp | 17 +++++++++++++----
> gdb/testsuite/gdb.btrace/enable-running.exp | 7 ++++++-
> gdb/testsuite/gdb.btrace/enable.exp | 16 +++++++++++++---
> gdb/testsuite/gdb.btrace/exception.exp | 10 +++++++---
> gdb/testsuite/gdb.btrace/function_call_history.exp | 11 ++++++++---
> gdb/testsuite/gdb.btrace/gcore.exp | 9 ++++++---
> gdb/testsuite/gdb.btrace/instruction_history.exp | 10 +++++++---
> gdb/testsuite/gdb.btrace/multi-thread-step.exp | 9 ++++++---
> gdb/testsuite/gdb.btrace/nohist.exp | 9 ++++++---
> gdb/testsuite/gdb.btrace/non-stop.exp | 10 ++++++----
> gdb/testsuite/gdb.btrace/reconnect.exp | 11 +++++++++--
> gdb/testsuite/gdb.btrace/record_goto-step.exp | 10 ++++++----
> gdb/testsuite/gdb.btrace/record_goto.exp | 11 ++++++++---
> gdb/testsuite/gdb.btrace/rn-dl-bind.exp | 10 +++++++---
> gdb/testsuite/gdb.btrace/segv.exp | 9 ++++++---
> gdb/testsuite/gdb.btrace/step.exp | 10 ++++++----
> gdb/testsuite/gdb.btrace/stepi.exp | 15 +++++++++------
> gdb/testsuite/gdb.btrace/tailcall-only.exp | 11 ++++++++---
> gdb/testsuite/gdb.btrace/tailcall.exp | 10 +++++++---
> gdb/testsuite/gdb.btrace/tsx.exp | 15 ++++++++++++---
> gdb/testsuite/gdb.btrace/unknown_functions.exp | 10 +++++++---
> gdb/testsuite/gdb.btrace/vdso.exp | 10 +++++++---
> 25 files changed, 190 insertions(+), 79 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.btrace/buffer-size.exp b/gdb/testsuite/gdb.btrace/buffer-size.exp
> index 2f19bf1..8708b12 100644
> --- a/gdb/testsuite/gdb.btrace/buffer-size.exp
> +++ b/gdb/testsuite/gdb.btrace/buffer-size.exp
> @@ -17,16 +17,19 @@
> # You should have received a copy of the GNU General Public License
> # along with this program. If not, see <http://www.gnu.org/licenses/>.
>
> -# check for btrace support
> -if { [skip_btrace_tests] } { return -1 }
> +if { [skip_btrace_tests] } {
> + unsupported "target does not support record-btrace"
> + return -1
> +}
>
> -# start inferior
> standard_testfile record_goto.c
> if [prepare_for_testing $testfile.exp $testfile $srcfile] {
> + untested "failed to compile $srcfile"
> return -1
> }
>
> if ![runto_main] {
> + untested "failed to run to main"
> return -1
> }
>
> diff --git a/gdb/testsuite/gdb.btrace/data.exp b/gdb/testsuite/gdb.btrace/data.exp
> index f39b24f..19b7cd5 100644
> --- a/gdb/testsuite/gdb.btrace/data.exp
> +++ b/gdb/testsuite/gdb.btrace/data.exp
> @@ -17,15 +17,19 @@
> # You should have received a copy of the GNU General Public License
> # along with this program. If not, see <http://www.gnu.org/licenses/>.
>
> -# check for btrace support
> -if { [skip_btrace_tests] } { return -1 }
> +if { [skip_btrace_tests] } {
> + unsupported "target does not support record-btrace"
> + return -1
> +}
>
> -# start inferior
> standard_testfile
> if [prepare_for_testing $testfile.exp $testfile $srcfile] {
> + untested "failed to compile $srcfile"
> return -1
> }

For example, this and other occurrences.

> @@ -26,17 +33,19 @@ set binfile_lib [standard_output_file $basename_lib.so]
>
> if { [gdb_compile_shlib $srcfile_lib $binfile_lib \
> [list additional_flags=-fPIC]] != "" } {
> - untested "failed to compile shared library"
> + untested "failed to compile $srcfile_lib"
> return -1
> }

This looks correct and shouldn't be changed.