From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7896 invoked by alias); 29 Mar 2012 11:37:19 -0000 Received: (qmail 7667 invoked by uid 22791); 29 Mar 2012 11:37:17 -0000 X-SWARE-Spam-Status: No, hits=-6.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_XP,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 29 Mar 2012 11:37:00 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q2TBauQ8029023 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 29 Mar 2012 07:36:56 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q2TBas1h022124; Thu, 29 Mar 2012 07:36:55 -0400 Message-ID: <4F744956.4060500@redhat.com> Date: Thu, 29 Mar 2012 11:37:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120316 Thunderbird/11.0 MIME-Version: 1.0 To: Hui Zhu CC: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [PATCH]testsuite: test for disconnected-tracing References: <4F71C7F0.90209@mentor.com> <87iphoy2xx.fsf@fleche.redhat.com> <4F73B80C.7050703@mentor.com> In-Reply-To: <4F73B80C.7050703@mentor.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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 X-SW-Source: 2012-03/txt/msg00987.txt.bz2 On 03/29/2012 02:17 AM, Hui Zhu wrote: > Hi Tom, > > Thanks for your review. > I post a new patch according to your comments. > > Best, > Hui > > 2012-03-29 Hui Zhu > > * gdb.trace/Makefile.in (BUILD_DATA_DIRECTORY): Add "BUILD_DATA_DIRECTORY" ? It looks like the change was to "PROGS" ? > disconnected-tracing. > * gdb.trace/disconnected-tracing.c: New file. > * gdb.trace/disconnected-tracing.exp: New file. > > On 03/29/12 02:51, Tom Tromey wrote: >>>>>>> ">" == Hui Zhu writes: >> >>>> +gdb_test "set confirm off" ".*" >> >> I think we use gdb_test_no_output for things like this now. >> I'm not sure whether it applies to other cases in your patch. >> >>>> +gdb_test "delete" ".*" >> >> Perhaps just calling delete_breakpoints would work? >> >> Tom > > disconnected-tracing.txt > > > --- > testsuite/gdb.trace/Makefile.in | 2 > testsuite/gdb.trace/disconnected-tracing.c | 5 ++ > testsuite/gdb.trace/disconnected-tracing.exp | 59 +++++++++++++++++++++++++++ > 3 files changed, 65 insertions(+), 1 deletion(-) > > --- a/testsuite/gdb.trace/Makefile.in > +++ b/testsuite/gdb.trace/Makefile.in > @@ -5,7 +5,7 @@ srcdir = @srcdir@ > > PROGS = ax backtrace deltrace infotrace packetlen passc-dyn passcount \ > report save-trace tfile tfind tracecmd tsv unavailable while-dyn \ > - while-stepping > + while-stepping disconnected-tracing > > all info install-info dvi install uninstall installcheck check: > @echo "Nothing to be done for $@..." > --- /dev/null > +++ b/testsuite/gdb.trace/disconnected-tracing.c > @@ -0,0 +1,5 @@ > +int > +main() > +{ > + return 0; > +} > \ No newline at end of file Please add one. Add a copyright header while at it. Best be consistent and make that a rule, even if the file is actually void of copyrightable content for now. > --- /dev/null > +++ b/testsuite/gdb.trace/disconnected-tracing.exp > @@ -0,0 +1,59 @@ > +# Copyright 2012 Free Software Foundation, Inc. > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +# Test of disconnected-tracing. > + > +load_lib "trace-support.exp"; > + > +set testfile "disconnected-tracing" > +set executable $testfile > +set srcfile ${testfile}.c > +set binfile $objdir/$subdir/$testfile > +set expfile $testfile.exp > +set gdbserver_reconnect_p 1 Please see gdb.base/solib-disc.exp, the only current user of gdbserver_reconnect_p. Add: if { [info proc gdb_reconnect] == "" } { return 0 } > + > +if [prepare_for_testing $expfile $executable $srcfile \ > + {debug nowarnings}] { > + untested "failed to prepare for trace tests" > + return -1 > +} > + > +if ![runto_main] { > + fail "Can't run to main to check for trace support" > + return -1 > +} > + > +if ![gdb_target_supports_trace] { > + unsupported "target does not support trace" > + return -1; > +} > + > +gdb_test_no_output "set confirm off" ".*" Please always look at the resulting messages in gdb.sum: PASS: gdb.trace/disconnected-tracing.exp: .* ^^ PASS: gdb.trace/disconnected-tracing.exp: trace main PASS: gdb.trace/disconnected-tracing.exp: tstart PASS: gdb.trace/disconnected-tracing.exp: First info tracepoints PASS: gdb.trace/disconnected-tracing.exp: disconnect PASS: gdb.trace/disconnected-tracing.exp: Second info tracepoints PASS: gdb.trace/disconnected-tracing.exp: disconnect PASS: gdb.trace/disconnected-tracing.exp: Third info tracepoints PASS: gdb.trace/disconnected-tracing.exp: Fourth info tracepoints There are duplicate messages here. See . I'd lowercase the "First", etc., in the test messages. It's much more common. > + > +gdb_test "trace main" ".*" > +gdb_test "tstart" ".*" Use gdb_test_no_output for tstart: gdb_test_no_output "tstart" "start trace experiment" > + > +gdb_test "info tracepoints" ".*in main at.*" "First info tracepoints" > +gdb_test "disconnect" ".*" > + > +gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport Please see gdb.base/solib-disc.exp for how to reconnect to the target. > +gdb_test "info tracepoints" ".*in main at.*" "Second info tracepoints" > +gdb_test "disconnect" ".*" > + > +delete_breakpoints > +gdb_test "info tracepoints" ".*No tracepoints..*" "Third info tracepoints" > + > +gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport > +gdb_test "info tracepoints" ".*in main at.*" "Fourth info tracepoints" But most importantly, I've tried this patch against a tree from just before the fixes, but the new tests all run cleanly there too. Also, should there be a "set disconnected-tracing on" somewhere? If not, then the test's filename appears misleading. Maybe something with "reconnect" in the name would be more representative? -- Pedro Alves