Re: [PATCH]testsuite: test for disconnected-tracing

[Pedro Alves <palves@redhat.com>]

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  <hui_zhu@mentor.com>
> 
>     * 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<hui_zhu@mentor.com>  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 <http://www.gnu.org/licenses/>.
> +
> +# 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
<http://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique>.

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