[3/3] add missing gdb_test arguments in paginate-bg-execution.exp

Message ID 5674636B.6090507@codesourcery.com
State New, archived
Headers

Commit Message

Sandra Loosemore Dec. 18, 2015, 7:50 p.m. UTC
  This patch fixes a think-o in gdb.base/paginate-bg-execution.exp -- 
there are two calls to gdb_test with only a single argument.  Looking at 
the definition of this proc in lib/gdb.exp, the second argument (the 
output pattern) is not supposed to be optional.  For whatever reason, I 
was only seeing failures on remote Windows host testing, but it must 
have been an accident that it appeared to be working elsewhere.

I copied the breakpoint output pattern used elsewhere in the testsuite, 
and confirmed this passes now.  OK to commit?

-Sandra
  

Comments

Pedro Alves Dec. 18, 2015, 8:40 p.m. UTC | #1
On 12/18/2015 07:50 PM, Sandra Loosemore wrote:
> This patch fixes a think-o in gdb.base/paginate-bg-execution.exp -- 
> there are two calls to gdb_test with only a single argument.  Looking at 
> the definition of this proc in lib/gdb.exp, the second argument (the 
> output pattern) is not supposed to be optional.  For whatever reason, I 
> was only seeing failures on remote Windows host testing, but it must 
> have been an accident that it appeared to be working elsewhere.
> 
> I copied the breakpoint output pattern used elsewhere in the testsuite, 
> and confirmed this passes now.  OK to commit?

Can you show the gdb.log of the failed run?

Thanks,
Pedro Alves
  
Sandra Loosemore Dec. 18, 2015, 11:49 p.m. UTC | #2
On 12/18/2015 01:40 PM, Pedro Alves wrote:
> On 12/18/2015 07:50 PM, Sandra Loosemore wrote:
>> This patch fixes a think-o in gdb.base/paginate-bg-execution.exp --
>> there are two calls to gdb_test with only a single argument.  Looking at
>> the definition of this proc in lib/gdb.exp, the second argument (the
>> output pattern) is not supposed to be optional.  For whatever reason, I
>> was only seeing failures on remote Windows host testing, but it must
>> have been an accident that it appeared to be working elsewhere.
>>
>> I copied the breakpoint output pattern used elsewhere in the testsuite,
>> and confirmed this passes now.  OK to commit?
>
> Can you show the gdb.log of the failed run?

Hmmmm.  I lost the original log, and now I cannot reproduce the failure. 
  I must be losing my marbles.  :-(

Is it supposed to be correct to call gdb_test with only one argument? 
If yes, I'll withdraw this patch and submit another one to tweak the 
comments in lib/gdb.exp to explicitly say the pattern arg is optional.

-Sandra
  
Pedro Alves Dec. 21, 2015, 11:53 a.m. UTC | #3
On 12/18/2015 11:49 PM, Sandra Loosemore wrote:
> On 12/18/2015 01:40 PM, Pedro Alves wrote:
>> On 12/18/2015 07:50 PM, Sandra Loosemore wrote:
>>> This patch fixes a think-o in gdb.base/paginate-bg-execution.exp --
>>> there are two calls to gdb_test with only a single argument.  Looking at
>>> the definition of this proc in lib/gdb.exp, the second argument (the
>>> output pattern) is not supposed to be optional.  For whatever reason, I
>>> was only seeing failures on remote Windows host testing, but it must
>>> have been an accident that it appeared to be working elsewhere.
>>>
>>> I copied the breakpoint output pattern used elsewhere in the testsuite,
>>> and confirmed this passes now.  OK to commit?
>>
>> Can you show the gdb.log of the failed run?
> 
> Hmmmm.  I lost the original log, and now I cannot reproduce the failure. 
>   I must be losing my marbles.  :-(
> 
> Is it supposed to be correct to call gdb_test with only one argument? 

If it wasn't supposed to be correct, then it'd be good to add
an "error" call in gdb_test, to make it a hard error.

But I think it is supposed to work.  At least

 $ grep -rn "gdb_test "  | grep -v "\".*\".*\"" | grep -v "\\\\"

shows many (hundreds) of instances.  With no explicit pattern,
we end up just matching the prompt, ignoring whatever output
precedes it.

> If yes, I'll withdraw this patch and submit another one to tweak the 
> comments in lib/gdb.exp to explicitly say the pattern arg is optional.

I should we should do that indeed.

Thanks,
Pedro Alves
  
Joel Brobecker Dec. 21, 2015, 12:27 p.m. UTC | #4
> If it wasn't supposed to be correct, then it'd be good to add
> an "error" call in gdb_test, to make it a hard error.
> 
> But I think it is supposed to work.  At least
> 
>  $ grep -rn "gdb_test "  | grep -v "\".*\".*\"" | grep -v "\\\\"
> 
> shows many (hundreds) of instances.  With no explicit pattern,
> we end up just matching the prompt, ignoring whatever output
> precedes it.

Yes, I confirm it is supposed to work, or at least it's been
acknowledged to be that way for quite a while. The last time
we discussed it is when we created gdb_test_no_output.
  

Patch

diff --git a/gdb/testsuite/gdb.base/paginate-bg-execution.exp b/gdb/testsuite/gdb.base/paginate-bg-execution.exp
index f2a4d73..12d5250 100644
--- a/gdb/testsuite/gdb.base/paginate-bg-execution.exp
+++ b/gdb/testsuite/gdb.base/paginate-bg-execution.exp
@@ -37,7 +37,7 @@  proc test_bg_execution_pagination_return {} {
 	    return 0
 	}
 
-	gdb_test "b after_sleep"
+	gdb_test "b after_sleep" "Breakpoint .* at .*"
 
 	gdb_test_no_output "set height 2"
 
@@ -80,7 +80,7 @@  proc test_bg_execution_pagination_cancel { how } {
 	    return 0
 	}
 
-	gdb_test "b after_sleep"
+	gdb_test "b after_sleep" "Breakpoint .* at .*"
 
 	gdb_test_no_output "set height 2"