This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA_v3 6/8] Add a test for 'frame apply'
- From: Pedro Alves <palves at redhat dot com>
- To: Philippe Waroquiers <philippe dot waroquiers at skynet dot be>, gdb-patches at sourceware dot org
- Date: Mon, 9 Jul 2018 20:19:15 +0100
- Subject: Re: [RFA_v3 6/8] Add a test for 'frame apply'
- References: <20180624183708.888-1-philippe.waroquiers@skynet.be> <20180624183708.888-7-philippe.waroquiers@skynet.be>
On 06/24/2018 07:37 PM, Philippe Waroquiers wrote:
> Add a test for 'frame apply'
Very nice. This looks mostly OK, though there's a couple details
that need sorting out. See below.
> diff --git a/gdb/testsuite/gdb.base/frameapply.exp b/gdb/testsuite/gdb.base/frameapply.exp
> new file mode 100644
> index 0000000000..1911e15b72
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/frameapply.exp
> @@ -0,0 +1,179 @@
> +# This testcase is part of GDB, the GNU debugger.
> +
> +# Copyright 2018 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 'frame apply [all | COUNT | -COUNT | id ID...] [FLAG]... COMMAND'
> +
> +standard_testfile
> +
> +if { [prepare_for_testing "failed to prepare" ${testfile}] } {
> + return -1
> +}
> +
> +clean_restart ${binfile}
> +
> +
> +if ![runto setup_done] then {
> + gdb_suppress_tests
> +}
gdb_suppress_tests is a really old mechanism that we shouldn't
be adding new uses of. Make that call fail and return immediately,
instead of continuing.
> +
> +set any "\[^\r\n\]*"
> +set ws "\[ \t\]\+"
> +set number "\[0-9]\+"
> +
> +
> +# check all | COUNT | -COUNT | id ID... with a simple command
> +foreach cmd {
> + "frame apply all p /x 20"
> + "frame apply id 0-5 p /x 20"
> + "frame apply id 0-3 4-5 p /x 20"
> + "frame apply id 0 1-2 3-5 p /x 20"
> + "frame apply id 0 1 2 3 4 5 p /x 20"
> + "frame apply id 0-0 1-1 2-2 3-3 4-4 5-5 p /x 20" } {
> + gdb_test $cmd \
> + [multi_line \
> + "#0${ws}setup_done ${any}" \
> + "\\\$\[0-9]+ = 0x14${any}" \
> + "#1${ws}${any} f1 ${any}" \
> + "\\\$\[0-9]+ = 0x14${any}" \
> + "#2${ws}${any} f2 ${any}" \
> + "\\\$\[0-9]+ = 0x14${any}" \
> + "#3${ws}${any} f3 ${any}" \
> + "\\\$\[0-9]+ = 0x14${any}" \
> + "#4${ws}${any} f4 ${any}" \
> + "\\\$\[0-9]+ = 0x14${any}" \
> + "#5${ws}${any} main ${any}" \
> + "\\\$\[0-9]+ = 0x14${any}" \
> + ] \
> + "Run a simple command on all frames"
lowercase "Run" -> "run". Likewise throughout.
> +# check quietness
> +foreach cmd {
> + "frame apply 2 p /x 20"
> + "frame apply id 0-1 p /x 20"} {
I noticed that the testcase produces duplicated test names in gdb.sum.
Take a look here:
https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique
You can fix this with e.g., foreach_with_prefix. I'd suggest
wrapping the different stages of the testcase with
with_test_prefix as well, for visual guidance when browsing
gdb.sum results though that's less important.
Something like this:
# Check quietness.
with_test_prefix "quietness" {
foreach_with_prefix frames {"2" "id 0-1"} {
set cmd "frame apply $frames p /x 20"
gdb_test $cmd \
[multi_line \
"#0${ws}setup_done ${any}" \
"\\\$\[0-9]+ = 0x14${any}" \
"#1${ws}${any} f1 ${any}" \
"\\\$\[0-9]+ = 0x14${any}" \
] \
"run a command, printing location"
...
}
}
(Note, "Check quietness." with uppercase and period. Might as
well write complete sentences while at it.)
The same comments apply to patch #7, so I won't repeat them there.
Thanks,
Pedro Alves