[PATCH] gdb.base/pr10179.exp: fix racy tests (PR testsuite/12649)

[PATCH] gdb.base/pr10179.exp: fix racy tests (PR testsuite/12649)

[Marek Polacek]

This one is quite trivial.  It was needed to write the whole
question instead of using the `.*' at the end.  I have also
added the message input.  Ok?

2011-05-02  Marek Polacek  <mpolacek@redhat.com>

        * gdb.base/pr10179.exp: Fix racy tests by completing
        the question properly.  Also provide the message input.


Index: gdb/testsuite/gdb.base/pr10179.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/pr10179.exp,v
retrieving revision 1.2
diff -u -r1.2 pr10179.exp
--- gdb/testsuite/gdb.base/pr10179.exp	1 Jan 2011 15:33:42 -0000	1.2
+++ gdb/testsuite/gdb.base/pr10179.exp	2 May 2011 17:09:34 -0000
@@ -29,10 +29,10 @@
 
 gdb_test "rbreak foo.*" "Breakpoint \[0-9\]+\[^\\n\]*\\nint foo\[12\]\[^\\n\]*\\nBreakpoint \[0-9\]+\[^\\n\]*\\nint foo\[12\].*"
 
-gdb_test "delete breakpoints" ".*" "" "Delete all breakpoints.*" "y"
+gdb_test "delete breakpoints" "" "delete breakpoints" "Delete all breakpoints\\? \\(y or n\\) " "y"
 
 gdb_test "rbreak pr10179-a.c:foo.*" "Breakpoint \[0-9\]+\[^\\n\]*\\nint foo.*"
 
-gdb_test "delete breakpoints" ".*" "" "Delete all breakpoints.*" "y"
+gdb_test "delete breakpoints" "" "delete breakpoints" "Delete all breakpoints\\? \\(y or n\\) " "y"
 
 gdb_test "rbreak pr10179-a.c : .*" "Breakpoint \[0-9\]+\[^\\n\]*\\nint bar1\[^\\n\]*\\nBreakpoint \[0-9\]+\[^\\n\]*\\nint foo1\[^\\n\]*\\nBreakpoint \[0-9\]+\[^\\n\]*\\nint main\[^\\n\]*.*"

Re: [PATCH] gdb.base/pr10179.exp: fix racy tests (PR testsuite/12649)

[Pedro Alves]

On Monday 02 May 2011 18:17:47, Marek Polacek wrote:
> This one is quite trivial.  

Probably.  But, it most probably only became trivial/obvious to you
after looking at the gdb.log around the FAIL, and noticing, "ahh,
the race is because FOO, and BAR".  If you don't explain those FOOs
and BARs on your patch submission, whoever reviews the patch needs
to do about the same analysis work you've done.
Please, can you try again, with some explanation of _why_ is
it needed to ...

> It was needed to write the whole
> question instead of using the `.*' at the end.  I have also
> added the message input.  

... do these?  For instance, break.exp doesn't appear to
need to match the whole question.

Not sure the original PR 10179 had something to do with
deleting breakpoints, otherwise, there's a delete_breakpoints
procedure tests can use to do this.

I'm not objecting or approving in any way, and it's really
probably trivial.  Just trying to point out that if you make it
easier to okay your patches, you'll get okay's much quicker.  :-)

> Ok?
> 
> 2011-05-02  Marek Polacek  <mpolacek@redhat.com>
> 
>         * gdb.base/pr10179.exp: Fix racy tests by completing
>         the question properly.  Also provide the message input.
> 
> 
> Index: gdb/testsuite/gdb.base/pr10179.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/pr10179.exp,v
> retrieving revision 1.2
> diff -u -r1.2 pr10179.exp
> --- gdb/testsuite/gdb.base/pr10179.exp	1 Jan 2011 15:33:42 -0000	1.2
> +++ gdb/testsuite/gdb.base/pr10179.exp	2 May 2011 17:09:34 -0000
> @@ -29,10 +29,10 @@
>  
>  gdb_test "rbreak foo.*" "Breakpoint \[0-9\]+\[^\\n\]*\\nint foo\[12\]\[^\\n\]*\\nBreakpoint \[0-9\]+\[^\\n\]*\\nint foo\[12\].*"
>  
> -gdb_test "delete breakpoints" ".*" "" "Delete all breakpoints.*" "y"
> +gdb_test "delete breakpoints" "" "delete breakpoints" "Delete all breakpoints\\? \\(y or n\\) " "y"
>  
>  gdb_test "rbreak pr10179-a.c:foo.*" "Breakpoint \[0-9\]+\[^\\n\]*\\nint foo.*"
>  
> -gdb_test "delete breakpoints" ".*" "" "Delete all breakpoints.*" "y"
> +gdb_test "delete breakpoints" "" "delete breakpoints" "Delete all breakpoints\\? \\(y or n\\) " "y"
>  
>  gdb_test "rbreak pr10179-a.c : .*" "Breakpoint \[0-9\]+\[^\\n\]*\\nint bar1\[^\\n\]*\\nBreakpoint \[0-9\]+\[^\\n\]*\\nint foo1\[^\\n\]*\\nBreakpoint \[0-9\]+\[^\\n\]*\\nint main\[^\\n\]*.*"
> 

-- 
Pedro Alves

Re: [PATCH] gdb.base/pr10179.exp: fix racy tests (PR testsuite/12649)

[Marek Polacek]

On 05/02/2011 09:24 PM, Pedro Alves wrote:
> Please, can you try again, with some explanation of _why_ is
> it needed to ...

You are right, I'm sorry.  Thus, here is another try.

> For instance, break.exp doesn't appear to
> need to match the whole question.
 
In break.exp, there are no `gdb_test "delete breakpoints"'.

> Not sure the original PR 10179 had something to do with
> deleting breakpoints, otherwise, there's a delete_breakpoints
> procedure tests can use to do this.

I think the best thing here is to make use of the `delete_breakpoints', since
pr10179.exp addresses to test the `rbreak', not deleting the breakpoints.
Replacing `gdb_test "delete breakpoints"' with delete_breakpoints also
cures the races.  IIUC, the race was caused by this part:

	"Delete all breakpoints.*" "y"

Thus, when using read1(), we match the question "Delete all breakpoints?
(y or n)" right after the "breakpoints" word.  This will leave in the buffer
"? (y or n)".  The "(y or n)" matches and causes the "interactive prompt" fail.

Here is a better patch.  Tested with both read{,1}.  OK now?  Thanks,

2011-05-03  Marek Polacek  <mpolacek@redhat.com>

        * gdb.base/pr10179.exp: Get rid of races using `delete_breakpoints'
        in place of `gdb_test "delete breakpoints"'.  This eliminates two
        testcases.


Index: pr10179.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/pr10179.exp,v
retrieving revision 1.2
diff -u -r1.2 pr10179.exp
--- pr10179.exp	1 Jan 2011 15:33:42 -0000	1.2
+++ pr10179.exp	3 May 2011 16:46:27 -0000
@@ -29,10 +29,10 @@
 
 gdb_test "rbreak foo.*" "Breakpoint \[0-9\]+\[^\\n\]*\\nint foo\[12\]\[^\\n\]*\\nBreakpoint \[0-9\]+\[^\\n\]*\\nint foo\[12\].*"
 
-gdb_test "delete breakpoints" ".*" "" "Delete all breakpoints.*" "y"
+delete_breakpoints
 
 gdb_test "rbreak pr10179-a.c:foo.*" "Breakpoint \[0-9\]+\[^\\n\]*\\nint foo.*"
 
-gdb_test "delete breakpoints" ".*" "" "Delete all breakpoints.*" "y"
+delete_breakpoints
 
 gdb_test "rbreak pr10179-a.c : .*" "Breakpoint \[0-9\]+\[^\\n\]*\\nint bar1\[^\\n\]*\\nBreakpoint \[0-9\]+\[^\\n\]*\\nint foo1\[^\\n\]*\\nBreakpoint \[0-9\]+\[^\\n\]*\\nint main\[^\\n\]*.*"

Re: [PATCH] gdb.base/pr10179.exp: fix racy tests (PR testsuite/12649)

[Pedro Alves]

On Tuesday 03 May 2011 17:53:05, Marek Polacek wrote:
> > For instance, break.exp doesn't appear to
> > need to match the whole question.
>  
> In break.exp, there are no `gdb_test "delete breakpoints"'.

Right, I was refering its first test: "delete breakpoints".
It doesn't use gdb_test, but I didn't know what was doing
on within gdb_test itself that was causing the race you were
seeing.

> > Not sure the original PR 10179 had something to do with
> > deleting breakpoints, otherwise, there's a delete_breakpoints
> > procedure tests can use to do this.
> 
> I think the best thing here is to make use of the `delete_breakpoints', since
> pr10179.exp addresses to test the `rbreak', not deleting the breakpoints.
> Replacing `gdb_test "delete breakpoints"' with delete_breakpoints also
> cures the races.  IIUC, the race was caused by this part:
> 
> 	"Delete all breakpoints.*" "y"
> 
> Thus, when using read1(), we match the question "Delete all breakpoints?
> (y or n)" right after the "breakpoints" word.  This will leave in the buffer
> "? (y or n)".  The "(y or n)" matches and causes the "interactive prompt" fail.

You're assuming I already knew the fail was of "interactive prompt"
kind.  :-)  I didn't, but I now ran the test and see the failure.  Seeing
this also helps review:

 FAIL: gdb.base/pr10179.exp: delete breakpoints (got interactive prompt)
 FAIL: gdb.base/pr10179.exp: rbreak pr10179-a.c:foo.*
 FAIL: gdb.base/pr10179.exp: rbreak pr10179-a.c : .* (got interactive prompt)

Knowing all this, your original patch was indeed trivial.

> 
> Here is a better patch.  Tested with both read{,1}.  OK now?  Thanks,
> 
> 2011-05-03  Marek Polacek  <mpolacek@redhat.com>
> 
>         * gdb.base/pr10179.exp: Get rid of races using `delete_breakpoints'
>         in place of `gdb_test "delete breakpoints"'.  This eliminates two
>         testcases.

Okay, thanks, this is even better.

-- 
Pedro Alves

Re: [PATCH] gdb.base/pr10179.exp: fix racy tests (PR testsuite/12649)

[Marek Polacek]

On 05/03/2011 07:50 PM, Pedro Alves wrote:
> You're assuming I already knew the fail was of "interactive prompt"
> kind.  :-)  I didn't, but I now ran the test and see the failure.  Seeing
> this also helps review:
> 
>  FAIL: gdb.base/pr10179.exp: delete breakpoints (got interactive prompt)
>  FAIL: gdb.base/pr10179.exp: rbreak pr10179-a.c:foo.*
>  FAIL: gdb.base/pr10179.exp: rbreak pr10179-a.c : .* (got interactive prompt)
> 
> Knowing all this, your original patch was indeed trivial.

Yeah, I should have shown this before.  Next time, I will.

> Okay, thanks, this is even better.

Applied: http://sourceware.org/ml/gdb-cvs/2011-05/msg00019.html

Thank you.

	Marek