Re: [PATCH v2] Add autoload-breakpoints [2/6] ReportAsync-test

[Yao Qi <yao@codesourcery.com>]

Hi,
Looks a mini-server-for-test is created.  Why not using GDBserver?
Unless we have a strong justification to do this, I prefer testing GDB
remote feature with GDBserver.

When we modify the GDB internal logic on RSP, the test cases of this
kind may be affected.  This increases the burden of maintenance.

On the other hand, I skim over the patch, and have some comments below,

On 04/28/2012 03:14 PM, Hui Zhu wrote:
> 2012-04-28  Hui Zhu  <hui_zhu@mentor.com>
> 
>     * Makefile.in (ALL_SUBDIRS): Add gdb.remote.
>     * configure (ac_config_files): Add gdb.remote/Makefile.
>     * configure.ac (AC_OUTPUT): Ditto.

configure is "Regenerated" from changed configure.ac.  This entry is
incorrect.

> --- /dev/null
> +++ b/testsuite/gdb.remote/Makefile.in

Copyright header is missing.

> @@ -0,0 +1,17 @@
> +VPATH = @srcdir@
> +srcdir = @srcdir@
> +
> +EXECUTABLES   = reportasync-test
> +
> +MISCELLANEOUS =
> +
> +all info install-info dvi install uninstall installcheck check:
> +	@echo "Nothing to be done for $@..."
> +
> +clean mostlyclean:
> +	rm -f *~ *.o *.x *.ci *.sl a.out core
> +	rm -f $(EXECUTABLES) $(MISCELLANEOUS)
> +
> +distclean maintainer-clean realclean: clean
> +	rm -f Makefile config.status config.log site.* gdb.log gdb.sum
> +
> --- /dev/null
> +++ b/testsuite/gdb.remote/reportasync-test.c
> @@ -0,0 +1,371 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   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/>.  */
> +
> +#include <sys/types.h>
> +#include <sys/socket.h>
> +#include <netinet/in.h>
> +#include <netinet/tcp.h>

This program is not portable.  I got errors when build it on mingw32.

> --- /dev/null
> +++ b/testsuite/gdb.remote/reportasync-test.exp
> @@ -0,0 +1,80 @@
> +# This testcase is part of GDB, the GNU debugger.
> +
> +# 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/>.
> +
> +load_lib gdbserver-support.exp
> +
> +set testfile "reportasync-test"
> +set srcfile ${testfile}.c
> +set binfile ${objdir}/${subdir}/${testfile}
> +
> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
> +    untested reportasync-test.exp
> +    return -1
> +}
> +
> +gdb_exit
> +gdb_start
> +gdb_reinitialize_dir $srcdir/$subdir
> +
> +# Make sure we're disconnected, in case we're testing with an
> +# extended-remote board, therefore already connected.
> +gdb_test "disconnect" ".*"
> +
> +send_gdb "show remote report-async\n"

Remote feature should be tested in remote target or board file.
You are testing a remote feature GDB unconditionally, even in native
gdb.  It looks incorrect to me.

> +gdb_expect 10 {
> +    -re "Undefined show remote command" {
> +	fail "ReportAsync support"
> +	return
> +    }
> +    -re "Support for the `ReportAsync' packet is auto-detected" {
> +	pass "ReportAsync support"
> +    }
> +}

Why not using gdb_test_multiple?

> +
> +#Let GDB connect to test server
> +set portnum 2345
> +while 1 {
> +    set server_spawn_id [remote_spawn target "$binfile $portnum"]
> +    expect {
> +	    -i $server_spawn_id
> +	    -notransfer
> +	    -re "Listening on" {}
> +	    -re "Can't bind port" {
> +		incr portnum
> +		continue
> +	    }
> +    }
> +    break
> +}
> +gdb_target_cmd "remote" ":$portnum"
> +
> +gdb_test "continue" ".*0x00000000 in ?? ().*" "Continue with AsyncReport first time"
> +
> +exec sleep 2
> +
> +gdb_test "set debug remote 1" ".*"

gdb_test_no_output

> +
> +gdb_test "x 0" ".*Ignore a ReportAsync shake hands package because waiting a ack.*" "Fake shake hands package first time"
> +
> +gdb_test "set debug remote 0" ".*"

gdb_test_no_output

> +
> +gdb_test "continue" ".*0x00000000 in ?? ().*" "Continue with AsyncReport second time"
> +
> +gdb_test "set debug remote 1" ".*"

gdb_test_no_output.

> +
> +gdb_test "tstatus" ".*Ignore a ReportAsync shake hands package because waiting a response.*" "Fake shake hands package second time"
> +

This line is too long.
Not sure it is a good idea to rely on the debugging message for testing
purpose.

Messages in test result are not unique,
]$ cat testsuite/gdb.sum  | grep "PASS" | sort | uniq -c | sort -n
      1 PASS: gdb.remote/reportasync-test.exp: Continue with AsyncReport
first time
      1 PASS: gdb.remote/reportasync-test.exp: Continue with AsyncReport
second time
      1 PASS: gdb.remote/reportasync-test.exp: disconnect
      1 PASS: gdb.remote/reportasync-test.exp: Fake shake hands package
first time
      1 PASS: gdb.remote/reportasync-test.exp: Fake shake hands package
second time
      1 PASS: gdb.remote/reportasync-test.exp: ReportAsync support
      1 PASS: gdb.remote/reportasync-test.exp: set debug remote 0
      2 PASS: gdb.remote/reportasync-test.exp: set debug remote 1

-- 
Yao (齐尧)