[rfa] gdb.base/volatile.exp / varargs.exp: replace send_gdb with gdb_test

[rfa] gdb.base/volatile.exp / varargs.exp: replace send_gdb with gdb_test

[Michael Snyder]

[-- Attachment #1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2: v.exp.txt --]
[-- Type: text/plain, Size: 6287 bytes --]

2010-05-19  Michael Snyder  <msnyder@vmware.com>

	* gdb.base/varargs.exp: Replace send_gdb with gdb_test.
	* gdb.base/volatile.exp: Replace send_gdb with gdb_test.

Index: varargs.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/varargs.exp,v
retrieving revision 1.12
diff -u -p -r1.12 varargs.exp
--- varargs.exp	5 May 2010 18:06:58 -0000	1.12
+++ varargs.exp	19 May 2010 20:23:15 -0000
@@ -62,9 +62,13 @@ gdb_exit
 gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 gdb_load ${binfile}
-send_gdb "set print sevenbit-strings\n" ; gdb_expect -re "$gdb_prompt $"
-send_gdb "set print address off\n" ; gdb_expect -re "$gdb_prompt $"
-send_gdb "set width 0\n" ; gdb_expect -re "$gdb_prompt $"
+gdb_test "set print sevenbit-strings" "" \
+    "set print sevenbit-strings; ${testfile}"
+gdb_test "set print address off" "" \
+    "set print address off; ${testfile}"
+gdb_test "set width 0" "" \
+    "set width 0; ${testfile}"
+
 
 if [gdb_skip_stdio_test "varargs.exp"] {
     # Nothing in this module is testable without printf.
@@ -89,57 +93,25 @@ if { $hp_aCC_compiler } {
     gdb_test "set overload-resolution 0" ""
 }
 
-send_gdb "print find_max1(5,1,2,3,4,5)\n"
-gdb_expect {
-    -re ".*find_max\\(5, 1, 2, 3, 4, 5\\) returns 5\[ \r\n\]+.\[0-9\]+ = 5.*$gdb_prompt $" {
-        pass "print find_max1(5,1,2,3,4,5)"
-      }
-    -re ".*$gdb_prompt $" { fail "print find_max1(5,1,2,3,4,5)" }
-    timeout           { fail "(timeout) print find_max1(5,1,2,3,4,5)" }
-  }
-
-
-
-
-send_gdb "print find_max1(1,3)\n"
-gdb_expect {
-    -re ".*find_max\\(1, 3\\) returns 3\[ \r\n\]+.\[0-9\]+ = 3.*$gdb_prompt $" {
-        pass "print find_max1(1,3)"
-      }
-    -re ".*$gdb_prompt $" { fail "print find_max1(1,3)" }
-    timeout           { fail "(timeout) print find_max1(1,3)" }
-  }
-
-
-send_gdb "print find_max1(10,1,2,3,4,5,6,7,8,29,0)\n"
-gdb_expect {
-    -re ".*find_max\\(10, 1, 2, 3, 4, 5, 6, 7, 8, 29, 0\\) returns 29\[ \r\n\]+.\[0-9\]+ = 29.*$gdb_prompt $" {
-        pass "print find_max1(10,1,2,3,4,5,6,7,8,29,0)"
-      }
-    -re ".*$gdb_prompt $" { fail "print find_max1(10,1,2,3,4,5,6,7,8,29,0)" }
-    timeout           { fail "(timeout) print find_max1(10,1,2,3,4,5,6,7,8,29,0)" }
-  }
-
-
-
-send_gdb "print find_max2(3,1,2,3)\n"
-gdb_expect {
-    -re ".*find_max\\(3, 1, 2, 3\\) returns 3\[ \r\n\]+.\[0-9\]+ = 3.*$gdb_prompt $" {
-        pass "print find_max2(3,1,2,3)"
-      }
-    -re ".*$gdb_prompt $" { fail "print find_max2(3,1,2,3)" }
-    timeout           { fail "(timeout) print find_max2(3,1,2,3)" }
-  }
-
+gdb_test "print find_max1(5,1,2,3,4,5)" \
+    "find_max\\(5, 1, 2, 3, 4, 5\\) returns 5\[ \r\n\]+.\[0-9\]+ = 5" \
+    "print find_max1(5,1,2,3,4,5)"
+
+gdb_test "print find_max1(1,3)" \
+    ".*find_max\\(1, 3\\) returns 3\[ \r\n\]+.\[0-9\]+ = 3" \
+    "print find_max1(1,3)"
+
+gdb_test "print find_max1(10,1,2,3,4,5,6,7,8,29,0)" \
+    ".*find_max\\(10, 1, 2, 3, 4, 5, 6, 7, 8, 29, 0\\) returns 29\[ \r\n\]+.\[0-9\]+ = 29" \
+    "print find_max1(10,1,2,3,4,5,6,7,8,29,0)"
+
+gdb_test "print find_max2(3,1,2,3)" \
+    ".*find_max\\(3, 1, 2, 3\\) returns 3\[ \r\n\]+.\[0-9\]+ = 3" \
+    "print find_max2(3,1,2,3)"
 
 if {![target_info exists gdb,skip_float_tests]} {
-  send_gdb "print find_max_double(5,1.0,17.0,2.0,3.0,4.0)\n"
-  gdb_expect {
-    -re ".*find_max\\(.*\\) returns 17\\.000000\[ \r\n\]+.\[0-9\]+ = 17.*$gdb_prompt $" {
-      pass "print find_max_double(5,1.0,17.0,2.0,3.0,4.0)"
-    }
-    -re ".*$gdb_prompt $" { fail "print find_max_double(5,1.0,17.0,2.0,3.0,4.0)" }
-    timeout           { fail "(timeout) print find_max_double(5,1.0,17.0,2.0,3.0,4.0)" }
-  }
+    gdb_test "print find_max_double(5,1.0,17.0,2.0,3.0,4.0)" \
+	".*find_max\\(.*\\) returns 17\\.000000\[ \r\n\]+.\[0-9\]+ = 17" \
+	"print find_max_double(5,1.0,17.0,2.0,3.0,4.0)"
 }
 
Index: volatile.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/volatile.exp,v
retrieving revision 1.18
diff -u -p -r1.18 volatile.exp
--- volatile.exp	5 May 2010 18:06:58 -0000	1.18
+++ volatile.exp	19 May 2010 20:23:15 -0000
@@ -87,28 +87,17 @@ proc local_compiler_xfail_check_2 { } {
     }
 }
 
-send_gdb "break marker1\n" ; gdb_expect -re ".*$gdb_prompt $"
+gdb_test "break marker1" "" ""
 
-    send_gdb "cont\n"
-    gdb_expect {
-        -re "Break.* marker1 \\(\\) at .*:$decimal.*$gdb_prompt $" {
-            send_gdb "up\n"
-            gdb_expect {
-                -re ".*$gdb_prompt $" {}
-                timeout { fail "up from marker1" }
-            }
-        }
-        -re "Break.* marker1__.* \\(\\) at .*:$decimal.*$gdb_prompt $" {
-	    fail "continue to marker1 (demangling)"
-            send_gdb "up\n"
-            gdb_expect {
-                -re ".*$gdb_prompt $" {}
-                timeout { fail "up from marker1" }
-            }
-        }
-        -re "$gdb_prompt $" { fail "continue to marker1"  }
-        timeout { fail "(timeout) continue to marker1"  }
+gdb_test_multiple "cont" "continue to marker1" {
+    -re "Break.* marker1 \\(\\) at .*:$decimal.*$gdb_prompt $" {
+	pass "continue to marker1"
     }
+    -re "Break.* marker1__.* \\(\\) at .*:$decimal.*$gdb_prompt $" {
+	fail "continue to marker1 (demangling)"
+    }
+}
+gdb_test "up" " in main .*" "up from marker1"
 
 # As of Feb 1999, GCC does not issue info about volatility of vars, so
 # these tests are all expected to fail if GCC is the compiler. -sts
@@ -248,11 +237,7 @@ gdb_test "ptype vendor" "type = const vo
 # test function parameters
 local_compiler_xfail_check
 local_compiler_xfail_check_2
-send_gdb "ptype qux2\n"
-gdb_expect {
-    -re "type = int \\(volatile unsigned char, const volatile int, volatile short( int)?, volatile long( int)? \\*, float \\* volatile, const volatile signed char \\* const volatile\\).*$gdb_prompt $" {
-        pass "ptype qux2"
-    }
-    -re ".*$gdb_prompt $" { fail "ptype qux2" }
-    timeout           { fail "(timeout) ptype qux2" }
-  }   
+
+gdb_test "ptype qux2" \
+    "type = int \\(volatile unsigned char, const volatile int, volatile short( int)?, volatile long( int)? \\*, float \\* volatile, const volatile signed char \\* const volatile\\)" \
+    "ptype qux2"

Re: [rfa] gdb.base/volatile.exp / varargs.exp: replace send_gdb with gdb_test

[Joel Brobecker]

For the record, Michael and I were discussing privately by email,
and one of the things I wrote was that he should feel free to commit
if he feels confident about his patches.  Since this can be relatively
mechanical, this should be relatively safe, and we can deal with
adjustments using followup patches...

> 2010-05-19  Michael Snyder  <msnyder@vmware.com>
> 
> 	* gdb.base/varargs.exp: Replace send_gdb with gdb_test.
> 	* gdb.base/volatile.exp: Replace send_gdb with gdb_test.

Some questions/suggestions...

> -send_gdb "set print sevenbit-strings\n" ; gdb_expect -re "$gdb_prompt $"
> -send_gdb "set print address off\n" ; gdb_expect -re "$gdb_prompt $"
> -send_gdb "set width 0\n" ; gdb_expect -re "$gdb_prompt $"
> +gdb_test "set print sevenbit-strings" "" \
> +    "set print sevenbit-strings; ${testfile}"
> +gdb_test "set print address off" "" \
> +    "set print address off; ${testfile}"
> +gdb_test "set width 0" "" \
> +    "set width 0; ${testfile}"

Just wondering why you added the "testfile" at the end of the test
description? It does not seem to serve any purpose, and so yo should
be able to simplify the above using:

> +gdb_test "set print sevenbit-strings" ""
> +gdb_test "set print address off" ""
> +gdb_test "set width 0" ""

You actually don't need to specify the second argument if it's empty,
but I think it's good practice to do so, just to show that we do indeed
expect the command to produce no output.  The fact that we cannot
currently verify that is a bit unfortunate, but at least the above
expresses it at the testsuite level...

> +gdb_test "print find_max1(5,1,2,3,4,5)" \
> +    "find_max\\(5, 1, 2, 3, 4, 5\\) returns 5\[ \r\n\]+.\[0-9\]+ = 5" \
> +    "print find_max1(5,1,2,3,4,5)"

If the test description simply repeats the command sent to GDB,
then it's not necessary to provide the third argument. gdb_test
does it automatically for you. Thus...

> +gdb_test "print find_max1(5,1,2,3,4,5)" \
> +    "find_max\\(5, 1, 2, 3, 4, 5\\) returns 5\[ \r\n\]+.\[0-9\]+ = 5"

... should give you the same result.

Generally speaking, my approach is becoming to use the command as
the description as long as it's not ambiguous for the testcase -
meaning that I don't use the same command twice in the same testcase.
There might be times where I'd want a specific description, but
that should be relatively rare...

-- 
Joel

Re: [rfa] gdb.base/volatile.exp / varargs.exp: replace send_gdb with gdb_test

[Pedro Alves]

On Thursday 20 May 2010 16:50:10, Joel Brobecker wrote:
> > +gdb_test "set print sevenbit-strings" ""
> > +gdb_test "set print address off" ""
> > +gdb_test "set width 0" ""
> 
> You actually don't need to specify the second argument if it's empty,
> but I think it's good practice to do so, just to show that we do indeed
> expect the command to produce no output.  The fact that we cannot
> currently verify that is a bit unfortunate, but at least the above
> expresses it at the testsuite level...
> 

You can anchor the output with gdb_test_multiple.  This, for example,
checks that a command produces no output other than the prompt:

    set cmd "..."
    set msg "..."
    set cmd_regex [string_to_regexp $cmd]
    gdb_test_multiple $cmd $msg {
        -re "^$cmd_regex\r\n$gdb_prompt $" {
            pass $msg
        }
    }

something like this is used in several places in the
testsuite.  we could perhaps add a wrapper in gdb.exp.

-- 
Pedro Alves

Re: [rfa] gdb.base/volatile.exp / varargs.exp: replace send_gdb with gdb_test

[Joel Brobecker]

> You can anchor the output with gdb_test_multiple.  This, for example,
> checks that a command produces no output other than the prompt:
> 
>     set cmd "..."
>     set msg "..."
>     set cmd_regex [string_to_regexp $cmd]
>     gdb_test_multiple $cmd $msg {
>         -re "^$cmd_regex\r\n$gdb_prompt $" {
>             pass $msg
>         }
>     }

Good to know! I thought I tried the above, but I must have screwed
up somewhere - maybe I was missing the \r\n...

> something like this is used in several places in the
> testsuite.  we could perhaps add a wrapper in gdb.exp.

I think we should - it's something that is so common, and the
gdb.reverse section alone is littered with FIXMEs because of
this issue. gdb_test_no_output?

-- 
Joel

Re: [rfa] gdb.base/volatile.exp / varargs.exp: replace send_gdb with gdb_test

[Pedro Alves]

On Thursday 20 May 2010 17:31:30, Joel Brobecker wrote:
> > something like this is used in several places in the
> > testsuite.  we could perhaps add a wrapper in gdb.exp.
> 
> I think we should - it's something that is so common, and the
> gdb.reverse section alone is littered with FIXMEs because of
> this issue. 

Yeah, I pointed this out before in that context, and also pointed
out in that context that for "set foo", we could try "show foo", and
confirm that what we tried to set actually was set.  Similarly
for any test that should produce no output and that has a
corresponding "state getter".

> gdb_test_no_output?

Sounds good.

-- 
Pedro Alves

Re: [rfa] gdb.base/volatile.exp / varargs.exp: replace send_gdb with gdb_test

[Joel Brobecker]

> > I think we should - it's something that is so common, and the
> > gdb.reverse section alone is littered with FIXMEs because of
> > this issue. 
> 
> Yeah, I pointed this out before in that context, and also pointed
> out in that context that for "set foo", we could try "show foo", and
> confirm that what we tried to set actually was set.  Similarly
> for any test that should produce no output and that has a
> corresponding "state getter".

Makes total sense to me. It might be unfair to put that on Michael's
shoulders for that patch, but if he's up for it, maybe as a followup
patch...

> > gdb_test_no_output?
> 
> Sounds good.

Michael: Are you up for implementing that function? It should be in
testsuite/lib/gdb.exp, and implemented using the code that Pedro
provided (minus the regexp part between the anchor and the \n\r).
Otherwise, I'll try to work on it later this afternoon for you.

-- 
Joel

Re: [rfa] gdb.base/volatile.exp / varargs.exp: replace send_gdb with gdb_test

[Pedro Alves]

On Thursday 20 May 2010 18:06:39, Joel Brobecker wrote:
> > Yeah, I pointed this out before in that context, and also pointed
> > out in that context that for "set foo", we could try "show foo", and
> > confirm that what we tried to set actually was set.  Similarly
> > for any test that should produce no output and that has a
> > corresponding "state getter".
> 
> Makes total sense to me. It might be unfair to put that on Michael's
> shoulders for that patch, but if he's up for it, maybe as a followup
> patch...

To be clear, I wasn't suggesting that Michael implemented it
for this patch, or even that Michael himself did it.  I even
think that patches that replace send_gdb with gdb_test should
avoid doing more than just that.

-- 
Pedro Alves

Re: [rfa] gdb.base/volatile.exp / varargs.exp: replace send_gdb with gdb_test

[Michael Snyder]

Joel Brobecker wrote:
> For the record, Michael and I were discussing privately by email,
> and one of the things I wrote was that he should feel free to commit
> if he feels confident about his patches.  Since this can be relatively
> mechanical, this should be relatively safe, and we can deal with
> adjustments using followup patches...
> 
>> 2010-05-19  Michael Snyder  <msnyder@vmware.com>
>>
>> 	* gdb.base/varargs.exp: Replace send_gdb with gdb_test.
>> 	* gdb.base/volatile.exp: Replace send_gdb with gdb_test.
> 
> Some questions/suggestions...
> 
>> -send_gdb "set print sevenbit-strings\n" ; gdb_expect -re "$gdb_prompt $"
>> -send_gdb "set print address off\n" ; gdb_expect -re "$gdb_prompt $"
>> -send_gdb "set width 0\n" ; gdb_expect -re "$gdb_prompt $"
>> +gdb_test "set print sevenbit-strings" "" \
>> +    "set print sevenbit-strings; ${testfile}"
>> +gdb_test "set print address off" "" \
>> +    "set print address off; ${testfile}"
>> +gdb_test "set width 0" "" \
>> +    "set width 0; ${testfile}"
> 
> Just wondering why you added the "testfile" at the end of the test
> description?

I copied it from an existing test.  My guess is that it
uniquifies the output, which otherwise would be identical
for a bunch of tests.

 > It does not seem to serve any purpose, and so yo should
> be able to simplify the above using:
> 
>> +gdb_test "set print sevenbit-strings" ""
>> +gdb_test "set print address off" ""
>> +gdb_test "set width 0" ""

Checking....  it seems that we have instances of at least four
different styles for these three specific statements.  I will
try to homogenize them.

Re: [rfa] gdb.base/volatile.exp / varargs.exp: replace send_gdb with gdb_test

[Michael Snyder]

Joel Brobecker wrote:

> Michael: Are you up for implementing that function? It should be in
> testsuite/lib/gdb.exp, and implemented using the code that Pedro
> provided (minus the regexp part between the anchor and the \n\r).
> Otherwise, I'll try to work on it later this afternoon for you.

Joel, I am happy to let you do it.  ;-)
I'll continue to concentrate on the send_gdb changes, and maybe
I'll hold off submitting them until I can include what you've done.

Michael

Re: [rfa] gdb.base/volatile.exp / varargs.exp: replace send_gdb with gdb_test

[Joel Brobecker]

> >Just wondering why you added the "testfile" at the end of the test
> >description?
> 
> I copied it from an existing test.  My guess is that it
> uniquifies the output, which otherwise would be identical
> for a bunch of tests.

You mean, from another testcase? The test description is the combination
of the testcase name (.exp file) and test name (in our case, the command).
I think I double-checked in this specific testcase that the command was
used only once, so the uniqueness should be guaranteed.

> >>+gdb_test "set print sevenbit-strings" ""
> >>+gdb_test "set print address off" ""
> >>+gdb_test "set width 0" ""
> 
> Checking....  it seems that we have instances of at least four
> different styles for these three specific statements.  I will
> try to homogenize them.

Just a thought, because I haven't really looked at what the "four
different styles" are, but I agree with Pedro when he says that
we should have separate patches for each step. If the homogenization
is not a mechanical step, perhaps it would be a good candidate for
a followup test?

-- 
Joel

[commit/testsuite] lib/gdb.exp: New gdb_test_no_output function.

[Joel Brobecker]

This implements the suggestion provided by Pedro on how to test a command
which is expected to generate no output.  I can now see why my own attempts
so far have failed: the output being matched by gdb_test_multiple actually
contains the command itself.  This is something that I managed to never
actually notice :-/.

2010-05-20  Pedro Alves  <pedro@codesourcery.com>
            Joel Brobecker  <brobecker@adacore.com>

        * lib/gdb.exp (gdb_test_no_output): New function.
        * lib/gdb.ada/arrayidx.exp: Use gdb_test_no_output instead of gdb_test
        when testing commands that should produce no output.

Tested on x86_64-linux.
---
 gdb/testsuite/ChangeLog            |    7 +++++++
 gdb/testsuite/gdb.ada/arrayidx.exp |    8 ++------
 gdb/testsuite/lib/gdb.exp          |   25 +++++++++++++++++++++++++
 3 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index b6f9469..93df121 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2010-05-20  Pedro Alves  <pedro@codesourcery.com>
+            Joel Brobecker  <brobecker@adacore.com>
+
+	* lib/gdb.exp (gdb_test_no_output): New function.
+	* lib/gdb.ada/arrayidx.exp: Use gdb_test_no_output instead of gdb_test
+	when testing commands that should produce no output.
+
 2010-05-19  Michael Snyder  <msnyder@vmware.com>
 
 	* gdb.base/whatis.exp: Replace send_gdb with gdb_test.
diff --git a/gdb/testsuite/gdb.ada/arrayidx.exp b/gdb/testsuite/gdb.ada/arrayidx.exp
index 13be845..3d45461 100644
--- a/gdb/testsuite/gdb.ada/arrayidx.exp
+++ b/gdb/testsuite/gdb.ada/arrayidx.exp
@@ -41,9 +41,7 @@ runto "p.adb:$bp_location"
 
 # First, print all the arrays without indexes
 
-gdb_test "set print array-indexes off" \
-         ""  \
-         "set print array-indexes to off"
+gdb_test_no_output "set print array-indexes off"
 
 gdb_test "print one_two_three" \
          "= \\(1, 2, 3\\)" \
@@ -79,9 +77,7 @@ gdb_test "print empty" \
 
 # Next, print all the arrays with the indexes
 
-gdb_test "set print array-indexes on" \
-         ""  \
-         "set print array-indexes to on"
+gdb_test_no_output "set print array-indexes on"
 
 gdb_test "print one_two_three" \
          "= \\(1 => 1, 2 => 2, 3 => 3\\)" \
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 70df5ea..563ffa7 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -877,6 +877,31 @@ proc gdb_test { args } {
 	}
      }]
 }
+
+# gdb_test_no_output COMMAND MESSAGE
+# Send a command to GDB and verify that this command generated no output.
+#
+# See gdb_test_multiple for a description of the COMMAND and MESSAGE
+# parameters.  If MESSAGE is ommitted, then COMMAND will be used as
+# the message.
+
+proc gdb_test_no_output { args } {
+    global gdb_prompt
+    set command [lindex $args 0]
+    if [llength $args]>1 then {
+	set message [lindex $args 1]
+    } else {
+	set message $command
+    }
+
+    set command_regex [string_to_regexp $command]
+    gdb_test_multiple $command $message {
+        -re "^$command_regex\r\n$gdb_prompt $" {
+            pass $message
+        }
+    }
+}
+
 \f
 # Test that a command gives an error.  For pass or fail, return
 # a 1 to indicate that more tests can proceed.  However a timeout
-- 
1.7.1