Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)

[Pedro Alves <pedro@codesourcery.com>]

On Sunday 01 May 2011 10:16:30, Jan Kratochvil wrote:
> On Thu, 28 Apr 2011 17:14:31 +0200, Pedro Alves wrote:
> >  set oldtimeout1 $timeout
> > -set timeout 30
> > +set timeout 10
> 
> 10 is too low for parallel runs where machine can be in 20+ load.  I do not
> see this test needs to excercise $timeout, I would even remove this whole
> override.

I simply changed it while writing the patch, so to get the timeouts
faster, and forgot to remove that change before posting...

> 
> 
> > @@ -114,7 +113,6 @@ gdb_expect  {
> >  #exp_internal 0
> >  
> >  send_gdb "show output\t"
> > -sleep 1
> >  gdb_expect  {
> >          -re "^show output-radix $"\
> >              { send_gdb "\n"
> ###              gdb_expect {
> ###                      -re "Default output radix for printing of values is 10\\..*$gdb_prompt $"\
> ###                                        { pass "complete 'show output'"}
> ###                      -re ".*$gdb_prompt $" { fail "complete 'show output'"}
> > @@ -125,16 +123,6 @@ gdb_expect  {
> >                        timeout           {fail "(timeout) complete 'show output'"}
> >                       }
> >              }
> > -        -re "^show output$"\
> > -            { send_gdb "\n"
> > -               gdb_expect {
> > -                      -re "Default output radix for printing of values is 10\\..*$gdb_prompt $"\
> > -                                        { fail "complete 'show output'"}
> > -                      -re ".*$gdb_prompt $" { fail "complete 'show output'"}
> > -                      timeout           { fail "(timeout) complete 'show output'"}
> > -                     }
> > -
> > -             }
> >  
> >          -re ".*$gdb_prompt $"       { fail "complete 'show output'" }
> >          timeout         { fail "(timeout) complete 'show output'" }
> 
> 
> The problem with this proposed intermediate step is that it in fact brings a
> testsuite regression.  Original "sleep 1" was there to ensure all the output
> has been caught.  This was racy but in most cases it worked.
> 
> Now it will false PASS with regressing GDB where the current FSF GDB HEAD
> testcase would correctly FAIL.  If GDB outputs "show output-radix " first and
> after 0.5sec it yet outputs "foobar" the original testcase correctly FAILed
> while the current testcase will falsely PASS.

I don't think we should worry about that.  If there's ever such a
regression, we can add a specific test for it.

> 
> The "complete" command appraoch does introduce this new kind of race.
> 
> But the patch can be commited in two parts if it is preferred although
> reviewing these racy send_gdb-gdb_expect cases for the intermediate step is
> tricky and it gets dropped immediately afterwards.

What do you mean is dropped immediately afterwards?

> 
> 
> > @@ -410,7 +365,7 @@ gdb_expect  {
> >  		    timeout           {fail "(timeout) complete 'p \"break1.'"}
> >  		}
> >  	    }
> > -	-re "^p \"break1\\..*$"
> > +	-re "^p \"break1\\...*$"
> >  	    {	send_gdb "\n"
> >  		gdb_expect {
> >  		    -re ".*$gdb_prompt $" { fail "complete 'p \"break1.'"}
> 
> I do not see this change as valid/relevant.

The pattern above reads:

	-re "^p \"break1\\.c\"$"\
...
	-re "^p \"break1\\..*$"
...

It looked like "^p \"break1\\.c" could wrongly match the latter pattern,
if the "c" wasn't in the buffer yet?

-- 
Pedro Alves