[RFA] gdb_expect -> gdb_test_multiple for filesym.exp

[RFA] gdb_expect -> gdb_test_multiple for filesym.exp

[Keith Seitz]

Hi,

Given the discussion yesterday on this, here is a patch to convert the 
test I committed yesterday to use gdb_test_multiple instead of gdb_expect.

Ok?
Keith

PS. I apparently neglected to commit my original ChangeLog entry for 
this patch. I will commit that (in its proper place) with this patch.

testsuite/ChangeLog
2013-05-16  Keith Seitz  <keiths@redhat.com>

	* gdb.base/filesym.exp: Use gdb_test_multiple insteadc of
	gdb_expect.

Index: testsuite/gdb.base/filesym.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/filesym.exp,v
retrieving revision 1.1
diff -u -p -r1.1 filesym.exp
--- testsuite/gdb.base/filesym.exp	15 May 2013 21:21:05 -0000	1.1
+++ testsuite/gdb.base/filesym.exp	16 May 2013 17:56:31 -0000
@@ -26,25 +26,21 @@ if {[prepare_for_testing $testfile.exp $
  # complete to "filesym"; completing again, we expect the symbol name 
and the
  # filename (in that order).

+set tst "complete on \"filesy\""
  send_gdb "break filesy\t"
-gdb_expect {
-     -re "m\$" {
-	pass "complete on \"filesy\""
+gdb_test_multiple "" $tst {
+    -re "m\$" {
+	pass $tst

  	# Now ask for the completion list
+	set tst "completion list for \"filesym\""
  	send_gdb "\t\t"
-	gdb_expect {
+	gdb_test_multiple "" $tst {
  	    -re ".*filesym\[ \t\]+filesym.c.*$gdb_prompt break filesym\$" {
-		pass "completion list for \"filesym\""
-	    }
-
-	    default {
-		fail "completion list for \"filesym\""
+		pass $tst
  	    }
  	}
      }
-
-    default {
-	fail "complete on \"filesy\""
-    }
  }
+
+unset -nocomplain tst

Re: [RFA] gdb_expect -> gdb_test_multiple for filesym.exp

[Pedro Alves]

On 05/16/2013 07:00 PM, Keith Seitz wrote:

> Ok?

Fine with me.

> testsuite/ChangeLog
> 2013-05-16  Keith Seitz  <keiths@redhat.com>
> 
>     * gdb.base/filesym.exp: Use gdb_test_multiple insteadc of
>     gdb_expect.

Typo "instead".

> +set tst "complete on \"filesy\""
>  send_gdb "break filesy\t"
> -gdb_expect {
> -     -re "m\$" {
> -    pass "complete on \"filesy\""
> +gdb_test_multiple "" $tst {
> +    -re "m\$" {
> +    pass $tst

Is indentation ok here?  Hard to tell from the diff.

> +unset -nocomplain tst

Curious.  Did you trip on another test getting confused with 'tst'
left set?

Thanks,
-- 
Pedro Alves

Re: [RFA] gdb_expect -> gdb_test_multiple for filesym.exp

[Keith Seitz]

On 05/16/2013 11:59 AM, Pedro Alves wrote:
>>      * gdb.base/filesym.exp: Use gdb_test_multiple insteadc of
>>      gdb_expect.
>
> Typo "instead".

Fixed.

>
>> +set tst "complete on \"filesy\""
>>   send_gdb "break filesy\t"
>> -gdb_expect {
>> -     -re "m\$" {
>> -    pass "complete on \"filesy\""
>> +gdb_test_multiple "" $tst {
>> +    -re "m\$" {
>> +    pass $tst
>
> Is indentation ok here?  Hard to tell from the diff.

I'm using default indentation, so I can only assume it is. FWIW, it 
looks correct to me in emacs.

>> +unset -nocomplain tst
>
> Curious.  Did you trip on another test getting confused with 'tst'
> left set?

Yes, I see this more often than not. I've sort of gotten into the habit 
of adding that, since our test suite does not attempt to keep itself 
clean, and it really doesn't harm anything.

Usually, one only runs into problems if, for example, a test uses a 
variable as an array and a subsequent test attempts to use the same 
variable as a scalar.

It can be easily removed, if you wished. Just let me know.

Keith

Re: [RFA] gdb_expect -> gdb_test_multiple for filesym.exp

[Pedro Alves]

On 05/16/2013 08:07 PM, Keith Seitz wrote:

> It can be easily removed, if you wished. Just let me know.

It's fine with me as is.

-- 
Pedro Alves

Re: [RFA] gdb_expect -> gdb_test_multiple for filesym.exp

[Keith Seitz]

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

On 05/16/2013 12:20 PM, Pedro Alves wrote:
> On 05/16/2013 08:07 PM, Keith Seitz wrote:
>
>> It can be easily removed, if you wished. Just let me know.
>
> It's fine with me as is.

Actually, I'm revising this test a tiny bit. Jan mentioned privately 
that this test was failing the read1 test scenario (see 
testsuite/12649). With this update, the patch now passes that, too.

Ok?
Keith

2013-05-16  Keith Seitz  <keiths@redhat.com>

	* gdb.base/filesym.exp: Use gdb_test_multiple instead of
	gdb_expect.
	Add test to flush the remaining input buffer so that this
	file passes testsuite/12649.




[-- Attachment #2: completer-test-test_multiple.patch --]
[-- Type: text/x-patch, Size: 1387 bytes --]

Index: testsuite/gdb.base/filesym.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/filesym.exp,v
retrieving revision 1.1
diff -u -p -r1.1 filesym.exp
--- testsuite/gdb.base/filesym.exp	15 May 2013 21:21:05 -0000	1.1
+++ testsuite/gdb.base/filesym.exp	16 May 2013 20:39:05 -0000
@@ -26,25 +26,26 @@ if {[prepare_for_testing $testfile.exp $
 # complete to "filesym"; completing again, we expect the symbol name and the
 # filename (in that order).
 
+set tst "complete on \"filesy\""
 send_gdb "break filesy\t"
-gdb_expect {
-     -re "m\$" {
-	pass "complete on \"filesy\""
+gdb_test_multiple "" $tst {
+    -re "m\$" {
+	pass $tst
 
 	# Now ask for the completion list
+	set tst "completion list for \"filesym\""
 	send_gdb "\t\t"
-	gdb_expect {
-	    -re ".*filesym\[ \t\]+filesym.c.*$gdb_prompt break filesym\$" {
-		pass "completion list for \"filesym\""
-	    }
+	gdb_test_multiple "" $tst {
+	    -re ".*filesym\[ \t\]+filesym.c.*$gdb_prompt " {
+		pass $tst
 
-	    default {
-		fail "completion list for \"filesym\""
+		# Flush the rest of the output by creating the breakpoint.
+		# This ensures this file passes testsuite/12649.
+		send_gdb "\n"
+		gdb_test "" "Breakpoint 1.*" "set breakpoint at filesym"
 	    }
 	}
     }
-
-    default {
-	fail "complete on \"filesy\""
-    }
 }
+
+unset -nocomplain tst

Re: [RFA] gdb_expect -> gdb_test_multiple for filesym.exp

[Pedro Alves]

On 05/16/2013 09:39 PM, Keith Seitz wrote:
> On 05/16/2013 12:20 PM, Pedro Alves wrote:
>> On 05/16/2013 08:07 PM, Keith Seitz wrote:
>>
>>> It can be easily removed, if you wished. Just let me know.
>>
>> It's fine with me as is.
> 
> Actually, I'm revising this test a tiny bit. Jan mentioned privately that this test was failing the read1 test scenario (see testsuite/12649). With this update, the patch now passes that, too.

Ah, missed that indeed.

As long as we're revising... :-)

> +set tst "complete on \"filesy\""
>  send_gdb "break filesy\t"
> -gdb_expect {
> -     -re "m\$" {
> -	pass "complete on \"filesy\""
> +gdb_test_multiple "" $tst {
> +    -re "m\$" {
> +	pass $tst

IMO, \t completion tests should check the whole expected line.
That's because '-re "m' matches anything before that "m".
So, say, this plausible bug where we'd complete the whole word instead
of just the missing letter

 "break filesy\t" -> "break filesyfilesym"

would go unnoticed.

That's how the tests in completion.exp work too.

>  
>  	# Now ask for the completion list
> +	set tst "completion list for \"filesym\""
>  	send_gdb "\t\t"
> -	gdb_expect {
> -	    -re ".*filesym\[ \t\]+filesym.c.*$gdb_prompt break filesym\$" {
> -		pass "completion list for \"filesym\""
> -	    }
> +	gdb_test_multiple "" $tst {
> +	    -re ".*filesym\[ \t\]+filesym.c.*$gdb_prompt " {
> +		pass $tst

Likewise, this would accept 3 or more entries, as long as
the expect 2 are there.

IOW, here's what IMO the test should end up like.

set tst "complete on \"filesy\""
send_gdb "break filesy\t"
gdb_test_multiple "" $tst {
    -re "^break filesy\\\x07m" {
	pass $tst

	# Now ask for the completion list
	set tst "completion list for \"filesym\""
	send_gdb "\t\t"
	gdb_test_multiple "" $tst {
	    -re "\\\x07\r\nfilesym\[ \t\]+filesym.c\[ \t\]+\r\n$gdb_prompt " {
		pass $tst

		# Flush the rest of the output by creating the breakpoint.
		# This ensures this file passes testsuite/12649.
		send_gdb "\n"
		gdb_test "" "Breakpoint 1.*" "set breakpoint at filesym"
	    }
	}
    }
}

-- 
Pedro Alves

Re: [RFA] gdb_expect -> gdb_test_multiple for filesym.exp

[Keith Seitz]

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

On 05/17/2013 03:51 AM, Pedro Alves wrote:
> As long as we're revising... :-)

Sure!

> set tst "complete on \"filesy\""
> send_gdb "break filesy\t"
> gdb_test_multiple "" $tst {
>      -re "^break filesy\\\x07m" {
> 	pass $tst
>
> 	# Now ask for the completion list
> 	set tst "completion list for \"filesym\""
> 	send_gdb "\t\t"
> 	gdb_test_multiple "" $tst {
> 	    -re "\\\x07\r\nfilesym\[ \t\]+filesym.c\[ \t\]+\r\n$gdb_prompt " {
> 		pass $tst
>
> 		# Flush the rest of the output by creating the breakpoint.
> 		# This ensures this file passes testsuite/12649.
> 		send_gdb "\n"
> 		gdb_test "" "Breakpoint 1.*" "set breakpoint at filesym"
> 	    }
> 	}
>      }
> }

I've used this verbatim and tested with the read1 scenario, too. All 
looks good.

Keith

ChangeLog

2013-05-20  Keith Seitz  <keiths@redhat.com>
	    Pedro Alves  <palves@redhat.com>

	* gdb.base/filesym.exp: Use gdb_test_multiple instead of
	gdb_expect.
	Add test to flush the remaining input buffer so that this
	file passes testsuite/12649.




[-- Attachment #2: completer-test-test_multiple-2.patch --]
[-- Type: text/x-patch, Size: 1423 bytes --]

Index: testsuite/gdb.base/filesym.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/filesym.exp,v
retrieving revision 1.1
diff -u -p -r1.1 filesym.exp
--- testsuite/gdb.base/filesym.exp	15 May 2013 21:21:05 -0000	1.1
+++ testsuite/gdb.base/filesym.exp	20 May 2013 19:11:51 -0000
@@ -26,25 +26,26 @@ if {[prepare_for_testing $testfile.exp $
 # complete to "filesym"; completing again, we expect the symbol name and the
 # filename (in that order).
 
+set tst "complete on \"filesy\""
 send_gdb "break filesy\t"
-gdb_expect {
-     -re "m\$" {
-	pass "complete on \"filesy\""
+gdb_test_multiple "" $tst {
+    -re "break filesy\\\x07m\$" {
+	pass $tst
 
 	# Now ask for the completion list
+	set tst "completion list for \"filesym\""
 	send_gdb "\t\t"
-	gdb_expect {
-	    -re ".*filesym\[ \t\]+filesym.c.*$gdb_prompt break filesym\$" {
-		pass "completion list for \"filesym\""
-	    }
+	gdb_test_multiple "" $tst {
+	    -re "\\\x07\r\nfilesym\[ \t\]+filesym.c\[ \t\]+\r\n$gdb_prompt " {
+		pass $tst
 
-	    default {
-		fail "completion list for \"filesym\""
+		# Flush the rest of the output by creating the breakpoint.
+		# This ensures this file passes testsuite/12649.
+		send_gdb "\n"
+		gdb_test "" "Breakpoint 1.*" "set breakpoint at filesym"
 	    }
 	}
     }
-
-    default {
-	fail "complete on \"filesy\""
-    }
 }
+
+unset -nocomplain tst

Re: [RFA] gdb_expect -> gdb_test_multiple for filesym.exp

[Pedro Alves]

On 05/20/2013 08:14 PM, Keith Seitz wrote:

> I've used this verbatim and tested with the read1 scenario, too. All looks good.

Thanks.  This is OK.

-- 
Pedro Alves

Re: [RFA] gdb_expect -> gdb_test_multiple for filesym.exp

[Keith Seitz]

On 05/20/2013 05:18 PM, Pedro Alves wrote:
> On 05/20/2013 08:14 PM, Keith Seitz wrote:
>
>> I've used this verbatim and tested with the read1 scenario, too. All looks good.
>
> Thanks.  This is OK.

I've committed this now. Thank you for your help.

Keith