Re: [PATCH 5/6] Test tracepoints are installed or not

[Yao Qi <yao@codesourcery.com>]

On 12/12/2012 01:53 AM, Pedro Alves wrote:
> Actually, sorry for not thinking of this before, but, it seems to me

Never mind, your comments are always welcome :)

> this use of pass+exp_continue is fragile.  Say a GDB bug is
> introduced, and the second =breakpoint-modified fails to be output.  In
> that case, this will result in:
> 
>       PASS: tracepoint on pendfunc2 resolved
>       FAIL: tracepoint on pendfunc2 resolved
> 
> instead of
>       PASS: tracepoint on pendfunc2 resolved
>       FAIL: tracepoint on pendfunc2 installed
> 

My intention of using exp_coutine here is to handle the situation that
notifications may arrive in different orders.  The fail message is not
clear, but not a big deal, to me.  It is important to catch a fail and
then we can check gdb.log and test case to fix it.

> You could fix that by doing:
> 
>   +	    # Pending tracepoint is resolved.
>    	    pass "$test"
>   +	    set test "tracepoint on pendfunc2 installed"
>   +	    exp_continue
> 
> But then, if the order of the notifications changes (IOW, due to a bug,
> we end up with the tracepoint not installed), this won't catch it.

Looks the fix is worse than the original one, at least no fail is
missing in the original one.

> It seems best to me to only use exp_continue in cases we won't to
> consume/skip output, and in the case of this patch, split the two
> tests into two consecutive gdb_expects.

If this way, we can't handle that two notifications arrive in a
reversed order (which is also correct).  If we don't have to worry
about the order (after all, the order of MI notifications are still
deterministic nowadays), I am OK with your suggestion.

-- 
Yao (齐尧)

gdb/testsuite:

2012-12-12  Yao Qi  <yao@codesourcery.com>

	* gdb.trace/mi-tracepoint-changed.exp (test_pending_resolved): Check
	'installed' field in '=breakpoint-modified'.
	(test_reconnect): Check 'installed' field in
	'=breakpoint-modified' and '=breakpoint-created'.
---
 gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp |   27 ++++++++++++++-------
 1 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp b/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
index 2913b12..0073d4b 100644
--- a/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
+++ b/gdb/testsuite/gdb.trace/mi-tracepoint-changed.exp
@@ -133,13 +133,18 @@ proc test_reconnect { } { with_test_prefix "reconnect" {
 	    fail "$test: 2"
 	    exp_continue
 	}
-	-re "=breakpoint-created,bkpt=\{number=\"3\",type=\"tracepoint\",disp=\"keep\",enabled=\"y\",.*,func=\"main\".*${mi_gdb_prompt}" {
+	-re "=breakpoint-created,bkpt=\{number=\"3\",type=\"tracepoint\",disp=\"keep\",enabled=\"y\",.*,func=\"main\".*installed=\"y\"" {
 	    # A tracepoint on main was defined in the stub, not in GDB,
 	    # so we should see a =breakpoint-created notification.
 	    pass $test
 	}
-	timeout {
-	    fail $test
+    }
+    # Tracepoint on marker is defined.  After the sync, we know that
+    # the tracepoint is in remote stub.  Mark it 'installed'.
+    set test "tracepoint on marker is installed"
+    gdb_expect {
+	-re "=breakpoint-modified,bkpt=\{number=\"2\".*,func=\"marker\".*installed=\"y\".*${mi_gdb_prompt}$" {
+	    pass "$test"
 	}
     }
     # Check that tracepoint 1 is still pending.
@@ -188,18 +193,22 @@ proc test_pending_resolved { } { with_test_prefix "pending resolved" {
 
     mi_send_resuming_command "exec-continue" "continuing execution to marker 1"
 
+    # It is expected to get two "=breakpoint-modified" notifications.
+    # Pending tracepoint is resolved.
     set test "tracepoint on pendfunc2 resolved"
     gdb_expect {
-	-re ".*=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\"" {
+	-re "=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\".*installed=\"n\"" {
 	    pass "$test"
 	}
-	-re ".*${mi_gdb_prompt}$" {
-	    fail $test
-	}
-	timeout {
-	    fail "$test (timeout)"
+    }
+    # Resolved tracepoint is installed.
+    set test "tracepoint on pendfunc2 installed"
+    gdb_expect {
+	-re "=breakpoint-modified,bkpt=\{number=\"1\",type=\"tracepoint\".*.*times=\"0\".*installed=\"y\"" {
+	    pass "$test"
 	}
     }
+
     mi_expect_stop "breakpoint-hit" "marker" ".*" ".*" ".*" \
 	{"" "disp=\"keep\""} "continue to marker"
 
-- 
1.7.7.6