From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 126850 invoked by alias); 9 Dec 2016 15:32:06 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 126836 invoked by uid 89); 9 Dec 2016 15:32:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS,URIBL_RED autolearn=ham version=3.3.2 spammy=H*i:sk:1481290, H*MI:sk:1481290, H*f:sk:1481290, H*M:f8b7 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 09 Dec 2016 15:31:55 +0000 Received: from svr-orw-mbx-03.mgc.mentorg.com ([147.34.90.203]) by relay1.mentorg.com with esmtp id 1cFN9N-00013O-PN from Luis_Gustavo@mentor.com ; Fri, 09 Dec 2016 07:31:53 -0800 Received: from [172.30.6.208] (147.34.91.1) by svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Fri, 9 Dec 2016 07:31:51 -0800 Reply-To: Luis Machado Subject: Re: [PATCH] btrace: add unsupported/untested messages when skipping tests References: <1481290056-9983-1-git-send-email-markus.t.metzger@intel.com> To: Markus Metzger , From: Luis Machado Message-ID: <44ab7b08-e6e3-d949-f8b7-02c830e090f5@codesourcery.com> Date: Fri, 09 Dec 2016 15:32:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <1481290056-9983-1-git-send-email-markus.t.metzger@intel.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: svr-orw-mbx-02.mgc.mentorg.com (147.34.90.202) To svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) X-IsSubscribed: yes X-SW-Source: 2016-12/txt/msg00244.txt.bz2 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 > > 2016-12-09 Markus Metzger > > 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 . > > -# 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 . > > -# 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.