Re: [PATCH] Don't handle timeout inside gdb_test_multiple

[Luis Machado <lgustavo@codesourcery.com>]

On 12/02/2016 05:04 AM, Pedro Alves wrote:
> On 12/02/2016 01:07 AM, Luis Machado wrote:
>> This fixes a few cases where the testcase is explicitly handling timeouts
>> inside gdb_test_multiple when it is not necessary.
>>
>> It also converts two gdb_test_multiple calls to gdb_test_no_output calls
>> (also removing the timeout handling).
>>
>> gdb/testsuite/ChangeLog:
>>
>> 2016-12-01  Luis Machado  <lgustavo@codesourcery.com>
>>
>> 	* gdb.base/maint.exp: Remove timeout handling for gdb_test_multiple.
>> 	* gdb.cp/gdb2495.exp: Likewise and convert gdb_test_multiple into
>> 	gdb_test_no_output for a couple of cases.
>> 	* gdb.cp/ovldbreak.exp: Remove timeout handling for gdb_test_multiple.
>> ---
>>  gdb/testsuite/gdb.base/maint.exp   |  9 ---------
>>  gdb/testsuite/gdb.cp/gdb2495.exp   | 10 ++--------
>>  gdb/testsuite/gdb.cp/ovldbreak.exp |  4 ----
>>  3 files changed, 2 insertions(+), 21 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
>> index b07b370..17c606b 100644
>> --- a/gdb/testsuite/gdb.base/maint.exp
>> +++ b/gdb/testsuite/gdb.base/maint.exp
>> @@ -297,9 +297,6 @@ gdb_test_multiple "maint print msymbols msymbols_output2 ${testfile}" "maint pri
>>  		    -re ".*$gdb_prompt $" {
>>  		        fail "maint print msymbols"
>>  		    }
>> -		    timeout {
>> -		        fail "(timeout) maint print msymbols"
>> -		    }
>
> This was:
>
> 	    	gdb_test_multiple "shell grep factorial msymbols_output2" "maint print msymbols" {
> 		    -re "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*$gdb_prompt $" {
> 		    	pass "maint print msymbols"
> 		    }
> 		    -re ".*$gdb_prompt $" {
> 		        fail "maint print msymbols"
> 		    }
> 		    timeout {
> 		        fail "(timeout) maint print msymbols"
> 		    }
>
> gdb_test_multiple has an internal FAIL for "$gdb_prompt $" too.  So this
> could be reduced to:
>
> 	    	gdb_test_multiple "shell grep factorial msymbols_output2" "maint print msymbols" {
> 		    -re "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*$gdb_prompt $" {
> 		    	pass "maint print msymbols"
> 		    }
>                 }
>
> which can then be reduced to:
>
> 	    	gdb_test "shell grep factorial msymbols_output2" \
> 		    "\\\[ *$decimal\\\] \[tT\]\[ \t\]+$hex \\.?factorial.*" \
>                     "maint print msymbols"
>
>
> Looks like a good chunk of the file could be similarly simplified.
>

Heh... i did notice those things, but i tried to keep the patch focused.

Actually, i noticed more tests using outdated constructs (gdb_expect) 
without the need to do so. That seemed like a slightly bigger change 
though (but would be nice!)

I can patch the above up with no problems though.

>> --- a/gdb/testsuite/gdb.cp/ovldbreak.exp
>> +++ b/gdb/testsuite/gdb.cp/ovldbreak.exp
>> @@ -58,10 +58,6 @@ proc take_gdb_out_of_choice_menu {} {
>>      gdb_test_multiple " " " " {
>>          -re ".*$gdb_prompt $" {
>>          }
>> -        timeout {
>> -            perror "could not resynchronize to command prompt (timeout)"
>> -            continue
>> -        }
>
> Looking at both the description and callers of this procedure, I'd think
> that the perror here was intended to abort the testcase instead of getting
> into a potentially long cascading timeout sequence.

This was a bit cryptic. It wasn't clear whether this was still 
functional or not. And it looks hack-ish.

Should i put this back? Maybe investigate if we need that code at all?