[RFA_v3,6/8] Add a test for 'frame apply'

Message ID 20180624183708.888-7-philippe.waroquiers@skynet.be
State New, archived
Headers

Commit Message

Philippe Waroquiers June 24, 2018, 6:37 p.m. UTC
  Add a test for 'frame apply'

gdb/testsuite/ChangeLog
2018-06-24  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.base/frameapply.c: New file.
	* gdb.base/frameapply.exp: New file.
---
 gdb/testsuite/gdb.base/frameapply.c   |  71 ++++++++++
 gdb/testsuite/gdb.base/frameapply.exp | 179 ++++++++++++++++++++++++++
 2 files changed, 250 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/frameapply.c
 create mode 100644 gdb/testsuite/gdb.base/frameapply.exp
  

Comments

Pedro Alves July 9, 2018, 7:19 p.m. UTC | #1
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
  

Patch

diff --git a/gdb/testsuite/gdb.base/frameapply.c b/gdb/testsuite/gdb.base/frameapply.c
new file mode 100644
index 0000000000..dccf4031ed
--- /dev/null
+++ b/gdb/testsuite/gdb.base/frameapply.c
@@ -0,0 +1,71 @@ 
+/* 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/>.  */
+
+static void
+setup_done (void)
+{
+}
+
+static int
+f1 (int f1arg)
+{
+  int f1loc;
+
+  f1loc = f1arg - 1;
+
+  setup_done ();
+  return f1loc;
+}
+
+static int
+f2 (int f2arg)
+{
+  int f2loc;
+
+  f2loc = f1 (f2arg - 1);
+
+  return f2loc;
+}
+
+static int
+f3 (int f3arg)
+{
+  int f3loc;
+
+  f3loc = f2 (f3arg - 1);
+
+  return f3loc;
+}
+
+static int
+f4 (int f4arg)
+{
+  int f4loc;
+
+  f4loc = f3 (f4arg - 1);
+
+  return f4loc;
+}
+
+int
+main (void)
+{
+  int result;
+
+  result = f4 (4);
+  return 0;
+}
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
+}
+
+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"
+}
+
+foreach cmd {"frame apply 3 p /x 20" "frame apply id 0-2 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}" \
+	    ] \
+	"Run a simple command on the 3 innermost frames"
+}
+
+gdb_test "frame apply -3 p /x 20" \
+    [multi_line \
+	 "#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 the 3 outermost frames"
+
+# check -c (continue) and -s (silently continue) flags
+foreach cmd {
+    "frame apply all p f3arg"
+    "frame apply id 0-5 p f3arg"} {
+    gdb_test $cmd \
+	[multi_line \
+	     "#0${ws}setup_done ${any}" \
+	     "No symbol \\\"f3arg\\\" in current context." \
+	    ] \
+	"Run a failing command that aborts frame apply"
+}
+
+foreach cmd {
+    "frame apply all -c p f3arg"
+    "frame apply id 0-5 -c p f3arg"} {
+    gdb_test $cmd \
+	[multi_line \
+	     "#0${ws}setup_done ${any}" \
+	     "No symbol \\\"f3arg\\\" in current context." \
+	     "#1${ws}${any} f1 ${any}" \
+	     "No symbol \\\"f3arg\\\" in current context." \
+	     "#2${ws}${any} f2 ${any}" \
+	     "No symbol \\\"f3arg\\\" in current context." \
+	     "#3${ws}${any} f3 ${any}" \
+	     "\\\$\[0-9]+ = 3${any}" \
+	     "#4${ws}${any} f4 ${any}" \
+	     "No symbol \\\"f3arg\\\" in current context." \
+	     "#5${ws}${any} main ${any}" \
+	     "No symbol \\\"f3arg\\\" in current context." \
+	    ] \
+	"Run a command failing in all frames except #3, -c to continue"
+}
+
+foreach cmd {
+    "frame apply all -s p f3arg"
+    "faas p f3arg"
+    "frame apply id 0-5 -s p f3arg"} {
+    gdb_test $cmd \
+	[multi_line \
+	     "#3${ws}${any} f3 ${any}" \
+	     "\\\$\[0-9]+ = 3${any}" \
+	    ] \
+	"Run a command failing in all frames except #3, -s to silently continue"
+}
+
+# check quietness
+foreach cmd {
+    "frame apply 2 p /x 20"
+    "frame apply id 0-1 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"
+}
+
+foreach cmd {
+    "frame apply 2 -q p /x 20"
+    "frame apply id 0-1 -q p /x 20"} {
+    gdb_test $cmd \
+	[multi_line \
+	     "\\\$\[0-9]+ = 0x14${any}" \
+	     "\\\$\[0-9]+ = 0x14${any}" \
+	    ] \
+	"Run a command with -q quiet flag, printing only command results"
+}
+
+# check multiple flags together
+foreach cmd {
+    "frame apply all -q -s -q p f3arg"
+    "frame apply id 0-5 -q -s -q p f3arg"} {
+    gdb_test $cmd \
+	"\\\$\[0-9]+ = 3${any}" \
+	"Run a command failing in all frames except #3, -s to silently continue, quiet"
+}
+
+# check invalid flag combinations
+gdb_test "frame apply all -c -s p f3arg" \
+    "frame apply all: -c and -s are mutually exclusive" \
+    "Check -c and -s cannot be used simultaneously"
+
+# check some cases of missing or wrong args
+gdb_test "frame apply" "Missing COUNT argument." "missing COUNT"
+gdb_test "frame apply -c" "Invalid COUNT argument." "invalid COUNT arg"
+gdb_test "frame apply id 4-2 p 1" "inverted range" "inverted range"
+gdb_test "frame apply id 0-3" \
+    "Please specify a command to apply on the selected frames" \
+    "missing command"