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

Luis Machado
2016-12-09T19:50:00Z

Could you please hold off on applying this patch? I'm working on a patch
to further cleanup the test names situation and that will conflict with
what you have, mainly related to the 'prepare_for_testing' calls.

On 12/09/2016 09:31 AM, Luis Machado wrote:
> 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.