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

[Luis Machado <lgustavo at codesourcery dot 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?