* [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
@ 2011-04-27 14:59 Marek Polacek
2011-04-27 15:05 ` Joel Brobecker
2011-04-28 11:56 ` Marek Polacek
0 siblings, 2 replies; 28+ messages in thread
From: Marek Polacek @ 2011-04-27 14:59 UTC (permalink / raw)
To: gdb-patches
Hi!
I am now trying to get rid of racy cases in gdb.base/completion.exp. The
first thing to do is to make use of `complete' rather than of '\t'. The '\t's
do not work well with char-wise read1() and thus they're occasionally causing
problems. The next thing to do is to remove ubiquitous `sleep's, which are racy
themselves and also make this test quite slow (with read1() this test takes
~8 minutes (!), with normal read() about 35 seconds).
This test basically comprises of chunks like this:
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'"}
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 "1complete 'show output'"}
-re ".*$gdb_prompt $" { fail "2complete 'show output'"}
timeout { fail "(timeout) complete 'show output'"}
}
}
-re ".*$gdb_prompt $" { fail "3complete 'show output'" }
timeout { fail "(timeout) complete 'show output'" }
}
I think this whole shenanigans is not necessary iff we are interested
only in testing that the _completion_ works. If you look at it, this chunk
has only one `pass'. What if we could replace this all with only one line like
the following?
gdb_test "complete show output" "show output-radix" "complete 'show output'"
Thus, my point is that we could replace those "send_gdb + sleep + gdb_expect"
sequences with just one gdb_test{,multiple,no_output}. I don't know yet if this
transformation is possible for every test in the completion.exp file.
Maybe the changes would be quite dramatical. However, this test would be _much_
simpler and much faster. Also, the current formatting is ugly ;).
So, do you think this is a good idea? Is there something I'm missing?
Thanks.
Marek
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-04-27 14:59 [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649) Marek Polacek
@ 2011-04-27 15:05 ` Joel Brobecker
2011-04-27 15:13 ` Tom Tromey
2011-04-27 15:23 ` Pedro Alves
2011-04-28 11:56 ` Marek Polacek
1 sibling, 2 replies; 28+ messages in thread
From: Joel Brobecker @ 2011-04-27 15:05 UTC (permalink / raw)
To: Marek Polacek; +Cc: gdb-patches
> Thus, my point is that we could replace those "send_gdb + sleep +
> gdb_expect" sequences with just one gdb_test{,multiple,no_output}. I
> don't know yet if this transformation is possible for every test in
> the completion.exp file. Maybe the changes would be quite dramatical.
> However, this test would be _much_ simpler and much faster. Also, the
> current formatting is ugly ;).
>
> So, do you think this is a good idea? Is there something I'm missing?
I don't know the history of the testcase, and this is only my own
opinion, but I tend to agree with you. I think we should keep one
test with \t, to make sure that a tab does trigger the completion,
but the rest of the testcase should be using the "complete" command.
That's what we do at AdaCore anyways...
--
Joel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-04-27 15:05 ` Joel Brobecker
@ 2011-04-27 15:13 ` Tom Tromey
2011-04-27 15:23 ` Pedro Alves
1 sibling, 0 replies; 28+ messages in thread
From: Tom Tromey @ 2011-04-27 15:13 UTC (permalink / raw)
To: Joel Brobecker; +Cc: Marek Polacek, gdb-patches
>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
>> Thus, my point is that we could replace those "send_gdb + sleep +
>> gdb_expect" sequences with just one gdb_test{,multiple,no_output}. I
>> don't know yet if this transformation is possible for every test in
>> the completion.exp file. Maybe the changes would be quite dramatical.
>> However, this test would be _much_ simpler and much faster. Also, the
>> current formatting is ugly ;).
>>
>> So, do you think this is a good idea? Is there something I'm missing?
Joel> I don't know the history of the testcase, and this is only my own
Joel> opinion, but I tend to agree with you. I think we should keep one
Joel> test with \t, to make sure that a tab does trigger the completion,
Joel> but the rest of the testcase should be using the "complete" command.
Joel> That's what we do at AdaCore anyways...
I tend to agree. One concern I do have is that "complete" and TAB
completion aren't always equivalent. There is a PR open about this
IIRC. IIRC the issue is that one approach respects some kind of
completion limit and the other does not. But, if this is tested, it
will presumably show up in the conversion.
Tom
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-04-27 15:05 ` Joel Brobecker
2011-04-27 15:13 ` Tom Tromey
@ 2011-04-27 15:23 ` Pedro Alves
2011-04-27 17:41 ` Marek Polacek
1 sibling, 1 reply; 28+ messages in thread
From: Pedro Alves @ 2011-04-27 15:23 UTC (permalink / raw)
To: gdb-patches; +Cc: Joel Brobecker, Marek Polacek
On Wednesday 27 April 2011 16:05:29, Joel Brobecker wrote:
> > Thus, my point is that we could replace those "send_gdb + sleep +
> > gdb_expect" sequences with just one gdb_test{,multiple,no_output}. I
> > don't know yet if this transformation is possible for every test in
> > the completion.exp file. Maybe the changes would be quite dramatical.
> > However, this test would be _much_ simpler and much faster. Also, the
> > current formatting is ugly ;).
> >
> > So, do you think this is a good idea? Is there something I'm missing?
>
> I don't know the history of the testcase, and this is only my own
> opinion, but I tend to agree with you. I think we should keep one
> test with \t, to make sure that a tab does trigger the completion,
> but the rest of the testcase should be using the "complete" command.
> That's what we do at AdaCore anyways...
How to fix the race that Marek is seeing in that leftover \t instance?
Marek wrote:
> The '\t's do not work well with char-wise read1() and thus
> they're occasionally causing problems.
What are these problems exactly?
I also wonder what's the rationale for the sleeps in the
current implementation?
> # tests for command completion
> #
> # Here are some useful test cases for completion.
> # They should be tested with both M-? and TAB.
An idea would be for the test to exercise all supported completion
methods (using a convenience procedure, not duplicating
the tests!).
--
Pedro Alves
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-04-27 15:23 ` Pedro Alves
@ 2011-04-27 17:41 ` Marek Polacek
2011-04-28 14:19 ` Pedro Alves
0 siblings, 1 reply; 28+ messages in thread
From: Marek Polacek @ 2011-04-27 17:41 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Joel Brobecker
On 04/27/2011 05:23 PM, Pedro Alves wrote:
> What are these problems exactly?
Duh. Now I'm not sure what I've meant when writing this. But
you cannot, for instance, use the '\t' in gdb_test "blahblah\t",
since this will end up with "ERROR: Undefined command".
> I also wonder what's the rationale for the sleeps in the
> current implementation?
Probably some imperfect way to avoid races--so the buffer would
be read at once after that sleep.
> An idea would be for the test to exercise all supported completion
> methods (using a convenience procedure, not duplicating
> the tests!).
That would be nice indeed. But only if we'd keep it at the same time
as simple as possible. Thanks,
Marek
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-04-27 14:59 [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649) Marek Polacek
2011-04-27 15:05 ` Joel Brobecker
@ 2011-04-28 11:56 ` Marek Polacek
1 sibling, 0 replies; 28+ messages in thread
From: Marek Polacek @ 2011-04-28 11:56 UTC (permalink / raw)
To: gdb-patches
Ok, here's a stab at the first part of this test. What I tried to do
is to convert "send_gdb + sleep + gdb_expect" to gdb_test{,_multiple,no_output}.
Also some quick notes indicated by `XXX'. Do you agree with it so far?
I've tried to preserve all the already existing tests. Thanks.
[ ... ]
set oldtimeout1 $timeout
set timeout 30
gdb_test_no_output "complete hfgfh" "complete 'hfgfh'"
#exp_internal 0
gdb_test "complete show output" "show output-radix" "complete 'show output'"
gdb_test "complete show output-" "show output-radix" "complete 'show output-'"
gdb_test "complete p" "passcount\r\npath\r\nprint\r\nprint-object\r\nprintf\r\nptype\r\npwd\r\npython" \
"complete 'p'"
# It is not possible to use `complete' here since this would list all
# the symbols.
gdb_test "p \t" "The history is empty." "complete 'p '"
gdb_test_no_output "complete info t foo" "complete 'info t foo'"
# XXX Should we list everything, i.e. "info target\r\ninfo tasks\r\ninfo terminal\r\n..."?
gdb_test "complete info t" "\(info \[a-z\]+\r\n\)*" "complete 'info t'"
gdb_test_no_output "complete info t " "complete 'info t '"
gdb_test_no_output "complete info asdfgh" "complete 'info asdfgh'"
gdb_test_no_output "complete info asdfgh\\x20" "complete 'info asdfgh '"
gdb_test "complete info" "info" "complete 'info'"
gdb_test "complete info " "\(info \[a-z\]+\r\n\)*" "complete 'info '"
gdb_test "info" "\"info\".*unambiguous\\." "complete (2) 'info '"
gdb_test "complete help info wat" "help info watchpoints" "complete 'help info wat'"
# XXX
# Does the `complete' behave correct?
# (gdb) p "break1<TAB> -> (gdb) p "break1.c"
# (gdb) complete p "break1 -> p "break1.c
# No `"' here? ------------------------> ^
# We proceed as it is.
gdb_test "complete p \"break1" "p \"break1.c" "complete 'p \"break1'"
# XXX Here was:
# setup_xfail "*-*-*"
gdb_test "complete p \"break1." "p \"break1\\.c" "complete 'p \"break1.'"
gdb_test "p 'arg\t" "Unmatched single quote\\." "complete 'p \'arg'"
gdb_test "p 'arg\t\t" ".*argv.*" "complete (2) 'p \'arg'"
# These tests used to try completing the shorter "p b-a".
# Unfortunately, on some systems, there are .o files in system
# libraries which declare static variables named `b'. Of course,
[ ... ]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-04-27 17:41 ` Marek Polacek
@ 2011-04-28 14:19 ` Pedro Alves
2011-04-28 15:14 ` Pedro Alves
0 siblings, 1 reply; 28+ messages in thread
From: Pedro Alves @ 2011-04-28 14:19 UTC (permalink / raw)
To: gdb-patches; +Cc: Marek Polacek, Joel Brobecker
On Wednesday 27 April 2011 18:40:32, Marek Polacek wrote:
> On 04/27/2011 05:23 PM, Pedro Alves wrote:
> > What are these problems exactly?
>
> Duh. Now I'm not sure what I've meant when writing this. But
> you cannot, for instance, use the '\t' in gdb_test "blahblah\t",
> since this will end up with "ERROR: Undefined command".
Are you sure? I did't see that when I try it.
> > I also wonder what's the rationale for the sleeps in the
> > current implementation?
>
> Probably some imperfect way to avoid races--so the buffer would
> be read at once after that sleep.
Yeah.
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'"}
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 "^show output$" regex will match if expect happens
to not see the expanded output-radix in the buffer yet. I don't
see a reason for that regex, so I think we should just remove it in this case.
And I'd much rather we do this (remote the races) as first step instead
of completely rewriting the whole test file into something completely
different...
The other races are similar in spirit. It's caused by having a
single gdb_expect with regexes like:
-re "address.*types.*$gdb_prompt info $" {}
-re ".*$gdb_prompt $" {}
This is racy because the recond regex will also match
"address.*types.*$gdb_prompt" if "$gdb_prompt " happens to
be in the buffer but "info " isn't yet.
Please try the patch below, and let me know what you think.
A follow up step would convert/simplify the send_gdb+gdb_expects
into gdb_test&friends.
--
Pedro Alves
2011-04-28 Pedro Alves <pedro@codesourcery.com>
Fix races.
testsuite/
* gdb.base/completion.exp: Remove all sleep calls. Remove
unnecessary regexs. Don't explicitly expect anything after the
prompt. Eat the prompt if necessary.
---
gdb/testsuite/gdb.base/completion.exp | 139 +++++++++-------------------------
1 file changed, 40 insertions(+), 99 deletions(-)
Index: src/gdb/testsuite/gdb.base/completion.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/completion.exp 2011-04-28 14:02:22.000000000 +0100
+++ src/gdb/testsuite/gdb.base/completion.exp 2011-04-28 14:52:26.595644001 +0100
@@ -96,7 +96,6 @@ set timeout 30
send_gdb "hfgfh\t"
-sleep 1
gdb_expect {
-re "^hfgfh\\\x07$"\
{ send_gdb "\n"
@@ -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"
@@ -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'" }
@@ -142,7 +130,6 @@ gdb_expect {
send_gdb "show output-\t"
-sleep 1
gdb_expect {
-re "^show output-radix $"\
{ send_gdb "\n"
@@ -153,27 +140,15 @@ 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-'" }
}
send_gdb "p\t"
-sleep 1
gdb_expect {
-re "^p\\\x07$"\
{ send_gdb "\n"
- sleep 1
gdb_expect {
-re "The history is empty\\..*$gdb_prompt $"\
{ pass "complete 'p'"}
@@ -186,11 +161,9 @@ gdb_expect {
}
send_gdb "p \t"
-sleep 3
gdb_expect {
-re "^p \\\x07$"\
{ send_gdb "\n"
- sleep 1
gdb_expect {
-re "The history is empty\\..*$gdb_prompt $"\
{ pass "complete 'p '"}
@@ -204,7 +177,6 @@ gdb_expect {
send_gdb "info t foo\t"
-sleep 1
gdb_expect {
-re "^info t foo\\\x07$"\
{ send_gdb "\n"
@@ -220,7 +192,6 @@ gdb_expect {
}
send_gdb "info t\t"
-sleep 1
gdb_expect {
-re "^info t\\\x07$"\
{ send_gdb "\n"
@@ -238,7 +209,6 @@ gdb_expect {
send_gdb "info t \t"
-sleep 1
gdb_expect {
-re "^info t \\\x07$"\
{ send_gdb "\n"
@@ -256,7 +226,6 @@ gdb_expect {
send_gdb "info asdfgh\t"
-sleep 1
gdb_expect {
-re "^info asdfgh\\\x07$"\
{ send_gdb "\n"
@@ -274,7 +243,6 @@ gdb_expect {
send_gdb "info asdfgh \t"
-sleep 1
gdb_expect {
-re "^info asdfgh \\\x07$"\
{ send_gdb "\n"
@@ -291,7 +259,6 @@ gdb_expect {
}
send_gdb "info\t"
-sleep 1
gdb_expect {
-re "^info $"\
{ send_gdb "\n"
@@ -307,7 +274,6 @@ gdb_expect {
}
send_gdb "info \t"
-sleep 1
gdb_expect {
-re "^info \\\x07$"\
{ send_gdb "\n"
@@ -324,12 +290,11 @@ gdb_expect {
send_gdb "info \t"
-sleep 1
gdb_expect {
-re "^info \\\x07$"\
{ send_gdb "\t"
gdb_expect {
- -re "address.*types.*$gdb_prompt info $"\
+ -re "address.*types.*$gdb_prompt $"\
{ send_gdb "\n"
gdb_expect {
-re "\"info\".*unambiguous\\..*$gdb_prompt $"\
@@ -365,7 +330,6 @@ gdb_expect {
send_gdb "p \"break1\t"
-sleep 1
gdb_expect {
-re "^p \"break1\\\x07$"\
{ send_gdb "\n"
@@ -381,20 +345,12 @@ gdb_expect {
timeout {fail "(timeout) complete 'p \"break1'"}
}
}
- -re "^p \"break1.*$"
- { send_gdb "\n"
- gdb_expect {
- -re ".*$gdb_prompt $" { fail "complete 'p \"break1'"}
- timeout {fail "(timeout) complete 'p \"break1'"}
- }
- }
-re ".*$gdb_prompt $" { fail "complete 'p \"break1'" }
timeout { fail "(timeout) complete 'p \"break1'" }
}
setup_xfail "*-*-*"
send_gdb "p \"break1.\t"
-sleep 1
gdb_expect {
-re "^p \"break1\\.\\\x07$"\
{ send_gdb "\n"
@@ -410,7 +366,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.'"}
@@ -422,7 +378,6 @@ gdb_expect {
}
send_gdb "p 'arg\t"
-sleep 1
gdb_expect {
-re "^p 'arg\\\x07$"\
{ send_gdb "\n"
@@ -438,12 +393,11 @@ gdb_expect {
}
send_gdb "p 'arg\t"
-sleep 1
gdb_expect {
-re "^p 'arg\\\x07$" {
send_gdb "\t"
gdb_expect {
- -re ".*argv.*$gdb_prompt p 'arg$" {
+ -re ".*argv.*$gdb_prompt $" {
send_gdb "\n"
gdb_expect {
-re "(Invalid character constant\\.|Unmatched single quote\\.).*$gdb_prompt $" {
@@ -503,7 +457,6 @@ gdb_expect {
# So, I'm hoping that there is no system with a static library variable named
# `no_var_by_this_name'.
send_gdb "p no_var_named_this-arg\t"
-sleep 1
gdb_expect {
-re "^p no_var_named_this-arg\\\x07$" {
send_gdb "\n"
@@ -528,12 +481,11 @@ gdb_expect {
}
send_gdb "p no_var_named_this-arg\t"
-sleep 1
gdb_expect {
-re "^p no_var_named_this-arg\\\x07$" {
send_gdb "\t"
gdb_expect {
- -re ".*argv.*$gdb_prompt p no_var_named_this-arg$" {
+ -re ".*argv.*$gdb_prompt $" {
send_gdb "\n"
gdb_expect {
-re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
@@ -548,28 +500,26 @@ gdb_expect {
}
}
-re "(There are $decimal possibilities\\. Do you really\r\nwish to see them all.|Display all $decimal possibilities.) \\(y or n\\)$" {
- send_gdb "n"
+ send_gdb "n\n"
+
+ # Eat the prompt
gdb_expect {
- -re "\\(gdb\\) p no_var_named_this-arg$" {
- send_gdb "\n"
- gdb_expect {
- -re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
- pass "complete (2) 'p no_var_named_this-arg'"
- }
- -re ".*$gdb_prompt $" {
- fail "complete (2) 'p no_var_named_this-arg'"
- }
- timeout {
- fail "(timeout) complete (2) 'p no_var_named_this-arg'"
- }
- }
+ -re ".*$gdb_prompt $" {
+ pass "complete (2) 'p no_var_named_this-arg' (eat prompt)"
+ }
+ timeout { fail "(timeout) complete (2) 'p no_var_named_this-'" }
+ }
+
+ gdb_expect {
+ -re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
+ pass "complete (2) 'p no_var_named_this-arg'"
}
-re ".*$gdb_prompt $" {
- fail "complete (2) 'p no_var_named_this-arg'"
- }
+ fail "complete (2) 'p no_var_named_this-arg'"
+ }
timeout {
- fail "(timeout) complete (2) 'p no_var_named_this-arg'"
- }
+ fail "(timeout) complete (2) 'p no_var_named_this-arg'"
+ }
}
}
-re ".*$gdb_prompt $" {
@@ -583,37 +533,35 @@ gdb_expect {
}
send_gdb "p no_var_named_this-\t"
-sleep 1
gdb_expect {
-re "^p no_var_named_this-\\\x07$" {
send_gdb "\t"
+
gdb_expect {
-re "(There are $decimal possibilities\\. Do you really\r\nwish to see them all.|Display all $decimal possibilities.) \\(y or n\\)$" {
- send_gdb "n"
+ send_gdb "n\n"
+
+ # Eat the prompt
gdb_expect {
- -re "\\(gdb\\) p no_var_named_this-$" {
- send_gdb "\n"
- gdb_expect {
- -re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
- pass "complete (2) 'p no_var_named_this-'"
- }
- -re ".*$gdb_prompt $" {
- fail "complete (2) 'p no_var_named_this-'"
- }
- timeout {
- fail "(timeout) complete (2) 'p no_var_named_this-'"
- }
- }
+ -re ".*$gdb_prompt $" {
+ pass "complete (2) 'p no_var_named_this-' (eat prompt)"
+ }
+ timeout { fail "(timeout) complete (2) 'p no_var_named_this-'" }
+ }
+
+ gdb_expect {
+ -re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
+ pass "complete (2) 'p no_var_named_this-'"
}
-re ".*$gdb_prompt $" {
- fail "complete (2) 'p no_var_named_this-'"
- }
+ fail "complete (2) 'p no_var_named_this-'"
+ }
timeout {
- fail "(timeout) complete (2) 'p no_var_named_this-'"
- }
+ fail "(timeout) complete (2) 'p no_var_named_this-'"
+ }
}
}
- -re ".*argv.*$gdb_prompt p no_var_named_this-$" {
+ -re ".*argv.*$gdb_prompt $" {
send_gdb "\n"
gdb_expect {
-re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
@@ -638,11 +586,9 @@ gdb_expect {
}
send_gdb "p values\[0\].a\t"
-sleep 3
gdb_expect {
-re "^p values.0..a_field $"\
{ send_gdb "\n"
- sleep 1
gdb_expect {
-re "^.* = 0.*$gdb_prompt $"\
{ pass "complete 'p values\[0\].a'"}
@@ -761,7 +707,6 @@ gdb_test " " "Source directories searche
send_gdb "complete file ./gdb.base/compl\n"
-sleep 1
gdb_expect {
-re "file ./gdb.base/completion\\.exp.*$gdb_prompt $"
{ pass "complete-command 'file ./gdb.base/compl'"}
@@ -770,7 +715,6 @@ gdb_expect {
}
send_gdb "file ./gdb.base/complet\t"
-sleep 1
gdb_expect {
-re "^file ./gdb.base/completion\\.exp $"\
{ send_gdb "\n"
@@ -788,14 +732,12 @@ gdb_expect {
}
send_gdb "info func marke\t"
-sleep 1
gdb_expect {
-re "^info func marke.*r$"\
{
send_gdb "\t\t"
- sleep 3
gdb_expect {
- -re "marker1.*$gdb_prompt info func marker$"\
+ -re "marker1.*$gdb_prompt $"\
{ send_gdb "\n"
gdb_expect {
-re "All functions matching regular expression \"marker\":.*File.*break1.c:\r\nint marker1\\((void|)\\);\r\nint marker2\\(int\\).*marker3\\(char.*char.*\\).*marker4\\(long( int)?\\);.*$gdb_prompt $"\
@@ -814,9 +756,8 @@ gdb_expect {
send_gdb "set follow-fork-mode \t\t"
-sleep 1
gdb_expect {
- -re "child.*parent.*$gdb_prompt set follow-fork-mode $"\
+ -re "child.*parent.*$gdb_prompt $"\
{ send_gdb "\n"
gdb_expect {
-re "Requires an argument.*child.*parent.*$gdb_prompt $"\
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-04-28 14:19 ` Pedro Alves
@ 2011-04-28 15:14 ` Pedro Alves
2011-04-29 14:10 ` Marek Polacek
2011-05-01 9:17 ` Jan Kratochvil
0 siblings, 2 replies; 28+ messages in thread
From: Pedro Alves @ 2011-04-28 15:14 UTC (permalink / raw)
To: gdb-patches; +Cc: Marek Polacek, Joel Brobecker
On Thursday 28 April 2011 15:19:21, Pedro Alves wrote:
> On Wednesday 27 April 2011 18:40:32, Marek Polacek wrote:
> > On 04/27/2011 05:23 PM, Pedro Alves wrote:
> > > What are these problems exactly?
> >
> > Duh. Now I'm not sure what I've meant when writing this. But
> > you cannot, for instance, use the '\t' in gdb_test "blahblah\t",
> > since this will end up with "ERROR: Undefined command".
>
> Are you sure? I did't see that when I try it.
>
> > > I also wonder what's the rationale for the sleeps in the
> > > current implementation?
> >
> > Probably some imperfect way to avoid races--so the buffer would
> > be read at once after that sleep.
>
> Yeah.
>
> 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'"}
> 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 "^show output$" regex will match if expect happens
> to not see the expanded output-radix in the buffer yet. I don't
> see a reason for that regex, so I think we should just remove it in this case.
>
> And I'd much rather we do this (remote the races) as first step instead
> of completely rewriting the whole test file into something completely
> different...
>
> The other races are similar in spirit. It's caused by having a
> single gdb_expect with regexes like:
>
> -re "address.*types.*$gdb_prompt info $" {}
> -re ".*$gdb_prompt $" {}
>
> This is racy because the recond regex will also match
> "address.*types.*$gdb_prompt" if "$gdb_prompt " happens to
> be in the buffer but "info " isn't yet.
>
> Please try the patch below, and let me know what you think.
> A follow up step would convert/simplify the send_gdb+gdb_expects
> into gdb_test&friends.
Here's a better one. I forgot to run the test _without_ Jan's read1
hack before, which revealed we should remove the end of line anchors
after the prompt as well, when matching the result of showing the
tab completion options, given that there'll be text afterwards (the
current command is left on readline's current line buffer), e.g.:
(gdb) set follow-fork-mode
child parent
(gdb) set follow-fork-mode
(no test required eating the previous leftover in the buffer,
e.g., due to some ^ anchor, so I didn't bother with it)
--
Pedro Alves
2011-04-28 Pedro Alves <pedro@codesourcery.com>
Fix races.
* gdb.base/completion.exp: Remove all sleep calls. Remove
unnecessary regexs. Don't explicitly expect anything after the
prompt. Eat the prompt if necessary.
---
gdb/testsuite/gdb.base/completion.exp | 142 +++++++++-------------------------
1 file changed, 41 insertions(+), 101 deletions(-)
Index: src/gdb/testsuite/gdb.base/completion.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/completion.exp 2011-04-28 14:02:22.000000000 +0100
+++ src/gdb/testsuite/gdb.base/completion.exp 2011-04-28 16:05:15.125644002 +0100
@@ -92,11 +92,10 @@ if ![runto_main] then {
}
set oldtimeout1 $timeout
-set timeout 30
+set timeout 10
send_gdb "hfgfh\t"
-sleep 1
gdb_expect {
-re "^hfgfh\\\x07$"\
{ send_gdb "\n"
@@ -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"
@@ -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'" }
@@ -142,7 +130,6 @@ gdb_expect {
send_gdb "show output-\t"
-sleep 1
gdb_expect {
-re "^show output-radix $"\
{ send_gdb "\n"
@@ -153,27 +140,15 @@ 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-'" }
}
send_gdb "p\t"
-sleep 1
gdb_expect {
-re "^p\\\x07$"\
{ send_gdb "\n"
- sleep 1
gdb_expect {
-re "The history is empty\\..*$gdb_prompt $"\
{ pass "complete 'p'"}
@@ -186,11 +161,9 @@ gdb_expect {
}
send_gdb "p \t"
-sleep 3
gdb_expect {
-re "^p \\\x07$"\
{ send_gdb "\n"
- sleep 1
gdb_expect {
-re "The history is empty\\..*$gdb_prompt $"\
{ pass "complete 'p '"}
@@ -204,7 +177,6 @@ gdb_expect {
send_gdb "info t foo\t"
-sleep 1
gdb_expect {
-re "^info t foo\\\x07$"\
{ send_gdb "\n"
@@ -220,7 +192,6 @@ gdb_expect {
}
send_gdb "info t\t"
-sleep 1
gdb_expect {
-re "^info t\\\x07$"\
{ send_gdb "\n"
@@ -238,7 +209,6 @@ gdb_expect {
send_gdb "info t \t"
-sleep 1
gdb_expect {
-re "^info t \\\x07$"\
{ send_gdb "\n"
@@ -256,7 +226,6 @@ gdb_expect {
send_gdb "info asdfgh\t"
-sleep 1
gdb_expect {
-re "^info asdfgh\\\x07$"\
{ send_gdb "\n"
@@ -274,7 +243,6 @@ gdb_expect {
send_gdb "info asdfgh \t"
-sleep 1
gdb_expect {
-re "^info asdfgh \\\x07$"\
{ send_gdb "\n"
@@ -291,7 +259,6 @@ gdb_expect {
}
send_gdb "info\t"
-sleep 1
gdb_expect {
-re "^info $"\
{ send_gdb "\n"
@@ -307,7 +274,6 @@ gdb_expect {
}
send_gdb "info \t"
-sleep 1
gdb_expect {
-re "^info \\\x07$"\
{ send_gdb "\n"
@@ -322,14 +288,12 @@ gdb_expect {
timeout { fail "(timeout) complete 'info '" }
}
-
send_gdb "info \t"
-sleep 1
gdb_expect {
-re "^info \\\x07$"\
{ send_gdb "\t"
gdb_expect {
- -re "address.*types.*$gdb_prompt info $"\
+ -re "address.*types.*$gdb_prompt "\
{ send_gdb "\n"
gdb_expect {
-re "\"info\".*unambiguous\\..*$gdb_prompt $"\
@@ -365,7 +329,6 @@ gdb_expect {
send_gdb "p \"break1\t"
-sleep 1
gdb_expect {
-re "^p \"break1\\\x07$"\
{ send_gdb "\n"
@@ -381,20 +344,12 @@ gdb_expect {
timeout {fail "(timeout) complete 'p \"break1'"}
}
}
- -re "^p \"break1.*$"
- { send_gdb "\n"
- gdb_expect {
- -re ".*$gdb_prompt $" { fail "complete 'p \"break1'"}
- timeout {fail "(timeout) complete 'p \"break1'"}
- }
- }
-re ".*$gdb_prompt $" { fail "complete 'p \"break1'" }
timeout { fail "(timeout) complete 'p \"break1'" }
}
setup_xfail "*-*-*"
send_gdb "p \"break1.\t"
-sleep 1
gdb_expect {
-re "^p \"break1\\.\\\x07$"\
{ send_gdb "\n"
@@ -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.'"}
@@ -422,7 +377,6 @@ gdb_expect {
}
send_gdb "p 'arg\t"
-sleep 1
gdb_expect {
-re "^p 'arg\\\x07$"\
{ send_gdb "\n"
@@ -438,12 +392,11 @@ gdb_expect {
}
send_gdb "p 'arg\t"
-sleep 1
gdb_expect {
-re "^p 'arg\\\x07$" {
send_gdb "\t"
gdb_expect {
- -re ".*argv.*$gdb_prompt p 'arg$" {
+ -re ".*argv.*$gdb_prompt " {
send_gdb "\n"
gdb_expect {
-re "(Invalid character constant\\.|Unmatched single quote\\.).*$gdb_prompt $" {
@@ -503,7 +456,6 @@ gdb_expect {
# So, I'm hoping that there is no system with a static library variable named
# `no_var_by_this_name'.
send_gdb "p no_var_named_this-arg\t"
-sleep 1
gdb_expect {
-re "^p no_var_named_this-arg\\\x07$" {
send_gdb "\n"
@@ -528,12 +480,11 @@ gdb_expect {
}
send_gdb "p no_var_named_this-arg\t"
-sleep 1
gdb_expect {
-re "^p no_var_named_this-arg\\\x07$" {
send_gdb "\t"
gdb_expect {
- -re ".*argv.*$gdb_prompt p no_var_named_this-arg$" {
+ -re ".*argv.*$gdb_prompt " {
send_gdb "\n"
gdb_expect {
-re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
@@ -548,28 +499,26 @@ gdb_expect {
}
}
-re "(There are $decimal possibilities\\. Do you really\r\nwish to see them all.|Display all $decimal possibilities.) \\(y or n\\)$" {
- send_gdb "n"
+ send_gdb "n\n"
+
+ # Eat the prompt
gdb_expect {
- -re "\\(gdb\\) p no_var_named_this-arg$" {
- send_gdb "\n"
- gdb_expect {
- -re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
- pass "complete (2) 'p no_var_named_this-arg'"
- }
- -re ".*$gdb_prompt $" {
- fail "complete (2) 'p no_var_named_this-arg'"
- }
- timeout {
- fail "(timeout) complete (2) 'p no_var_named_this-arg'"
- }
- }
+ -re "$gdb_prompt " {
+ pass "complete (2) 'p no_var_named_this-arg' (eat prompt)"
+ }
+ timeout { fail "(timeout) complete (2) 'p no_var_named_this-' (eat prompt)" }
+ }
+
+ gdb_expect {
+ -re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
+ pass "complete (2) 'p no_var_named_this-arg'"
}
-re ".*$gdb_prompt $" {
- fail "complete (2) 'p no_var_named_this-arg'"
- }
+ fail "complete (2) 'p no_var_named_this-arg'"
+ }
timeout {
- fail "(timeout) complete (2) 'p no_var_named_this-arg'"
- }
+ fail "(timeout) complete (2) 'p no_var_named_this-arg'"
+ }
}
}
-re ".*$gdb_prompt $" {
@@ -583,37 +532,35 @@ gdb_expect {
}
send_gdb "p no_var_named_this-\t"
-sleep 1
gdb_expect {
-re "^p no_var_named_this-\\\x07$" {
send_gdb "\t"
+
gdb_expect {
-re "(There are $decimal possibilities\\. Do you really\r\nwish to see them all.|Display all $decimal possibilities.) \\(y or n\\)$" {
- send_gdb "n"
+ send_gdb "n\n"
+
+ # Eat the prompt
gdb_expect {
- -re "\\(gdb\\) p no_var_named_this-$" {
- send_gdb "\n"
- gdb_expect {
- -re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
- pass "complete (2) 'p no_var_named_this-'"
- }
- -re ".*$gdb_prompt $" {
- fail "complete (2) 'p no_var_named_this-'"
- }
- timeout {
- fail "(timeout) complete (2) 'p no_var_named_this-'"
- }
- }
+ -re "$gdb_prompt " {
+ pass "complete (2) 'p no_var_named_this-' (eat prompt)"
+ }
+ timeout { fail "(timeout) complete (2) 'p no_var_named_this-' (eat prompt)" }
+ }
+
+ gdb_expect {
+ -re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
+ pass "complete (2) 'p no_var_named_this-'"
}
-re ".*$gdb_prompt $" {
- fail "complete (2) 'p no_var_named_this-'"
- }
+ fail "complete (2) 'p no_var_named_this-'"
+ }
timeout {
- fail "(timeout) complete (2) 'p no_var_named_this-'"
- }
+ fail "(timeout) complete (2) 'p no_var_named_this-'"
+ }
}
}
- -re ".*argv.*$gdb_prompt p no_var_named_this-$" {
+ -re ".*argv.*$gdb_prompt $" {
send_gdb "\n"
gdb_expect {
-re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
@@ -638,11 +585,9 @@ gdb_expect {
}
send_gdb "p values\[0\].a\t"
-sleep 3
gdb_expect {
-re "^p values.0..a_field $"\
{ send_gdb "\n"
- sleep 1
gdb_expect {
-re "^.* = 0.*$gdb_prompt $"\
{ pass "complete 'p values\[0\].a'"}
@@ -761,7 +706,6 @@ gdb_test " " "Source directories searche
send_gdb "complete file ./gdb.base/compl\n"
-sleep 1
gdb_expect {
-re "file ./gdb.base/completion\\.exp.*$gdb_prompt $"
{ pass "complete-command 'file ./gdb.base/compl'"}
@@ -770,7 +714,6 @@ gdb_expect {
}
send_gdb "file ./gdb.base/complet\t"
-sleep 1
gdb_expect {
-re "^file ./gdb.base/completion\\.exp $"\
{ send_gdb "\n"
@@ -788,14 +731,12 @@ gdb_expect {
}
send_gdb "info func marke\t"
-sleep 1
gdb_expect {
-re "^info func marke.*r$"\
{
send_gdb "\t\t"
- sleep 3
gdb_expect {
- -re "marker1.*$gdb_prompt info func marker$"\
+ -re "marker1.*$gdb_prompt "\
{ send_gdb "\n"
gdb_expect {
-re "All functions matching regular expression \"marker\":.*File.*break1.c:\r\nint marker1\\((void|)\\);\r\nint marker2\\(int\\).*marker3\\(char.*char.*\\).*marker4\\(long( int)?\\);.*$gdb_prompt $"\
@@ -814,9 +755,8 @@ gdb_expect {
send_gdb "set follow-fork-mode \t\t"
-sleep 1
gdb_expect {
- -re "child.*parent.*$gdb_prompt set follow-fork-mode $"\
+ -re "child.*parent.*$gdb_prompt "\
{ send_gdb "\n"
gdb_expect {
-re "Requires an argument.*child.*parent.*$gdb_prompt $"\
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-04-28 15:14 ` Pedro Alves
@ 2011-04-29 14:10 ` Marek Polacek
2011-05-02 14:58 ` Pedro Alves
2011-05-01 9:17 ` Jan Kratochvil
1 sibling, 1 reply; 28+ messages in thread
From: Marek Polacek @ 2011-04-29 14:10 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 04/28/2011 05:14 PM, Pedro Alves wrote:
> Are you sure? I did't see that when I try it.
For instance this is OK:
gdb_test_no_output "complete hfgfh" "complete 'hfgfh'"
but this is not:
gdb_test_no_output "hfgfh\t" "complete 'hfgfh'"
> And I'd much rather we do this (remote the races) as first step instead
> of completely rewriting the whole test file into something completely
> different...
Good point.
> A follow up step would convert/simplify the send_gdb+gdb_expects
> into gdb_test&friends.
Something like this attempt?
http://sourceware.org/ml/gdb-patches/2011-04/msg00538.html
> Here's a better one.
Perfect! It looks good--seems like all the racy cases are gone. Should this be
applied now?
Thank you.
Marek
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-04-28 15:14 ` Pedro Alves
2011-04-29 14:10 ` Marek Polacek
@ 2011-05-01 9:17 ` Jan Kratochvil
2011-05-02 14:00 ` Marek Polacek
2011-05-02 14:19 ` Pedro Alves
1 sibling, 2 replies; 28+ messages in thread
From: Jan Kratochvil @ 2011-05-01 9:17 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Marek Polacek, Joel Brobecker
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.
> @@ -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.
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.
> @@ -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.
Thanks,
Jan
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-05-01 9:17 ` Jan Kratochvil
@ 2011-05-02 14:00 ` Marek Polacek
2011-05-02 14:19 ` Pedro Alves
1 sibling, 0 replies; 28+ messages in thread
From: Marek Polacek @ 2011-05-02 14:00 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: Pedro Alves, gdb-patches, Joel Brobecker
On 05/01/2011 11:16 AM, Jan Kratochvil wrote:
> 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 removed this.
> 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 see your point. However, even with sleep 1, if GDB outputs "foobar"
after 1.1 sec, the test still incorrectly PASSes. This is a no-win
situation.
> The "complete" command appraoch does introduce this new kind of race.
Yeah :(. Basically, we would like to have some kind of signal, that the
`complete' is all done and thus we can test the result, IIUC.
> 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.
I propose to apply the attached patch. It is basically Pedro's patch, but
I've removed the timeout-fiddling as well as this unnecessary change:
>> - -re "^p \"break1\\..*$"
>> + -re "^p \"break1\\...*$"
I ran this test with read{,1}(). Is this, at least for now, OK?
2011-05-02 Marek Polacek <mpolacek@redhat.com>
* gdb.base/completion.exp: Fix racy tests. Remove `sleep's.
Also do not change the timeout.
($oldtimeout1): Remove variable.
Index: completion.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/completion.exp,v
retrieving revision 1.50
diff -u -r1.50 completion.exp
--- completion.exp 23 Feb 2011 19:20:39 -0000 1.50
+++ completion.exp 2 May 2011 13:49:54 -0000
@@ -91,12 +91,7 @@
perror "tests suppressed"
}
-set oldtimeout1 $timeout
-set timeout 30
-
-
send_gdb "hfgfh\t"
-sleep 1
gdb_expect {
-re "^hfgfh\\\x07$"\
{ send_gdb "\n"
@@ -114,7 +109,6 @@
#exp_internal 0
send_gdb "show output\t"
-sleep 1
gdb_expect {
-re "^show output-radix $"\
{ send_gdb "\n"
@@ -125,16 +119,6 @@
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'" }
@@ -142,7 +126,6 @@
send_gdb "show output-\t"
-sleep 1
gdb_expect {
-re "^show output-radix $"\
{ send_gdb "\n"
@@ -153,27 +136,15 @@
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-'" }
}
send_gdb "p\t"
-sleep 1
gdb_expect {
-re "^p\\\x07$"\
{ send_gdb "\n"
- sleep 1
gdb_expect {
-re "The history is empty\\..*$gdb_prompt $"\
{ pass "complete 'p'"}
@@ -186,11 +157,9 @@
}
send_gdb "p \t"
-sleep 3
gdb_expect {
-re "^p \\\x07$"\
{ send_gdb "\n"
- sleep 1
gdb_expect {
-re "The history is empty\\..*$gdb_prompt $"\
{ pass "complete 'p '"}
@@ -204,7 +173,6 @@
send_gdb "info t foo\t"
-sleep 1
gdb_expect {
-re "^info t foo\\\x07$"\
{ send_gdb "\n"
@@ -220,7 +188,6 @@
}
send_gdb "info t\t"
-sleep 1
gdb_expect {
-re "^info t\\\x07$"\
{ send_gdb "\n"
@@ -238,7 +205,6 @@
send_gdb "info t \t"
-sleep 1
gdb_expect {
-re "^info t \\\x07$"\
{ send_gdb "\n"
@@ -256,7 +222,6 @@
send_gdb "info asdfgh\t"
-sleep 1
gdb_expect {
-re "^info asdfgh\\\x07$"\
{ send_gdb "\n"
@@ -274,7 +239,6 @@
send_gdb "info asdfgh \t"
-sleep 1
gdb_expect {
-re "^info asdfgh \\\x07$"\
{ send_gdb "\n"
@@ -291,7 +255,6 @@
}
send_gdb "info\t"
-sleep 1
gdb_expect {
-re "^info $"\
{ send_gdb "\n"
@@ -307,7 +270,6 @@
}
send_gdb "info \t"
-sleep 1
gdb_expect {
-re "^info \\\x07$"\
{ send_gdb "\n"
@@ -322,14 +284,12 @@
timeout { fail "(timeout) complete 'info '" }
}
-
send_gdb "info \t"
-sleep 1
gdb_expect {
-re "^info \\\x07$"\
{ send_gdb "\t"
gdb_expect {
- -re "address.*types.*$gdb_prompt info $"\
+ -re "address.*types.*$gdb_prompt "\
{ send_gdb "\n"
gdb_expect {
-re "\"info\".*unambiguous\\..*$gdb_prompt $"\
@@ -365,7 +325,6 @@
send_gdb "p \"break1\t"
-sleep 1
gdb_expect {
-re "^p \"break1\\\x07$"\
{ send_gdb "\n"
@@ -381,20 +340,12 @@
timeout {fail "(timeout) complete 'p \"break1'"}
}
}
- -re "^p \"break1.*$"
- { send_gdb "\n"
- gdb_expect {
- -re ".*$gdb_prompt $" { fail "complete 'p \"break1'"}
- timeout {fail "(timeout) complete 'p \"break1'"}
- }
- }
-re ".*$gdb_prompt $" { fail "complete 'p \"break1'" }
timeout { fail "(timeout) complete 'p \"break1'" }
}
setup_xfail "*-*-*"
send_gdb "p \"break1.\t"
-sleep 1
gdb_expect {
-re "^p \"break1\\.\\\x07$"\
{ send_gdb "\n"
@@ -422,7 +373,6 @@
}
send_gdb "p 'arg\t"
-sleep 1
gdb_expect {
-re "^p 'arg\\\x07$"\
{ send_gdb "\n"
@@ -438,12 +388,11 @@
}
send_gdb "p 'arg\t"
-sleep 1
gdb_expect {
-re "^p 'arg\\\x07$" {
send_gdb "\t"
gdb_expect {
- -re ".*argv.*$gdb_prompt p 'arg$" {
+ -re ".*argv.*$gdb_prompt " {
send_gdb "\n"
gdb_expect {
-re "(Invalid character constant\\.|Unmatched single quote\\.).*$gdb_prompt $" {
@@ -503,7 +452,6 @@
# So, I'm hoping that there is no system with a static library variable named
# `no_var_by_this_name'.
send_gdb "p no_var_named_this-arg\t"
-sleep 1
gdb_expect {
-re "^p no_var_named_this-arg\\\x07$" {
send_gdb "\n"
@@ -528,12 +476,11 @@
}
send_gdb "p no_var_named_this-arg\t"
-sleep 1
gdb_expect {
-re "^p no_var_named_this-arg\\\x07$" {
send_gdb "\t"
gdb_expect {
- -re ".*argv.*$gdb_prompt p no_var_named_this-arg$" {
+ -re ".*argv.*$gdb_prompt " {
send_gdb "\n"
gdb_expect {
-re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
@@ -548,28 +495,26 @@
}
}
-re "(There are $decimal possibilities\\. Do you really\r\nwish to see them all.|Display all $decimal possibilities.) \\(y or n\\)$" {
- send_gdb "n"
+ send_gdb "n\n"
+
+ # Eat the prompt
gdb_expect {
- -re "\\(gdb\\) p no_var_named_this-arg$" {
- send_gdb "\n"
- gdb_expect {
- -re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
- pass "complete (2) 'p no_var_named_this-arg'"
- }
- -re ".*$gdb_prompt $" {
- fail "complete (2) 'p no_var_named_this-arg'"
- }
- timeout {
- fail "(timeout) complete (2) 'p no_var_named_this-arg'"
- }
- }
+ -re "$gdb_prompt " {
+ pass "complete (2) 'p no_var_named_this-arg' (eat prompt)"
+ }
+ timeout { fail "(timeout) complete (2) 'p no_var_named_this-' (eat prompt)" }
+ }
+
+ gdb_expect {
+ -re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
+ pass "complete (2) 'p no_var_named_this-arg'"
}
-re ".*$gdb_prompt $" {
- fail "complete (2) 'p no_var_named_this-arg'"
- }
+ fail "complete (2) 'p no_var_named_this-arg'"
+ }
timeout {
- fail "(timeout) complete (2) 'p no_var_named_this-arg'"
- }
+ fail "(timeout) complete (2) 'p no_var_named_this-arg'"
+ }
}
}
-re ".*$gdb_prompt $" {
@@ -583,37 +528,35 @@
}
send_gdb "p no_var_named_this-\t"
-sleep 1
gdb_expect {
-re "^p no_var_named_this-\\\x07$" {
send_gdb "\t"
+
gdb_expect {
-re "(There are $decimal possibilities\\. Do you really\r\nwish to see them all.|Display all $decimal possibilities.) \\(y or n\\)$" {
- send_gdb "n"
+ send_gdb "n\n"
+
+ # Eat the prompt
gdb_expect {
- -re "\\(gdb\\) p no_var_named_this-$" {
- send_gdb "\n"
- gdb_expect {
- -re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
- pass "complete (2) 'p no_var_named_this-'"
- }
- -re ".*$gdb_prompt $" {
- fail "complete (2) 'p no_var_named_this-'"
- }
- timeout {
- fail "(timeout) complete (2) 'p no_var_named_this-'"
- }
- }
+ -re "$gdb_prompt " {
+ pass "complete (2) 'p no_var_named_this-' (eat prompt)"
+ }
+ timeout { fail "(timeout) complete (2) 'p no_var_named_this-' (eat prompt)" }
+ }
+
+ gdb_expect {
+ -re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
+ pass "complete (2) 'p no_var_named_this-'"
}
-re ".*$gdb_prompt $" {
- fail "complete (2) 'p no_var_named_this-'"
- }
+ fail "complete (2) 'p no_var_named_this-'"
+ }
timeout {
- fail "(timeout) complete (2) 'p no_var_named_this-'"
- }
+ fail "(timeout) complete (2) 'p no_var_named_this-'"
+ }
}
}
- -re ".*argv.*$gdb_prompt p no_var_named_this-$" {
+ -re ".*argv.*$gdb_prompt $" {
send_gdb "\n"
gdb_expect {
-re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
@@ -638,11 +581,9 @@
}
send_gdb "p values\[0\].a\t"
-sleep 3
gdb_expect {
-re "^p values.0..a_field $"\
{ send_gdb "\n"
- sleep 1
gdb_expect {
-re "^.* = 0.*$gdb_prompt $"\
{ pass "complete 'p values\[0\].a'"}
@@ -761,7 +702,6 @@
send_gdb "complete file ./gdb.base/compl\n"
-sleep 1
gdb_expect {
-re "file ./gdb.base/completion\\.exp.*$gdb_prompt $"
{ pass "complete-command 'file ./gdb.base/compl'"}
@@ -770,7 +710,6 @@
}
send_gdb "file ./gdb.base/complet\t"
-sleep 1
gdb_expect {
-re "^file ./gdb.base/completion\\.exp $"\
{ send_gdb "\n"
@@ -788,14 +727,12 @@
}
send_gdb "info func marke\t"
-sleep 1
gdb_expect {
-re "^info func marke.*r$"\
{
send_gdb "\t\t"
- sleep 3
gdb_expect {
- -re "marker1.*$gdb_prompt info func marker$"\
+ -re "marker1.*$gdb_prompt "\
{ send_gdb "\n"
gdb_expect {
-re "All functions matching regular expression \"marker\":.*File.*break1.c:\r\nint marker1\\((void|)\\);\r\nint marker2\\(int\\).*marker3\\(char.*char.*\\).*marker4\\(long( int)?\\);.*$gdb_prompt $"\
@@ -814,9 +751,8 @@
send_gdb "set follow-fork-mode \t\t"
-sleep 1
gdb_expect {
- -re "child.*parent.*$gdb_prompt set follow-fork-mode $"\
+ -re "child.*parent.*$gdb_prompt "\
{ send_gdb "\n"
gdb_expect {
-re "Requires an argument.*child.*parent.*$gdb_prompt $"\
@@ -839,8 +775,4 @@
# If there is only a deprecated completion, then it should be returned.
gdb_test "complete save-t" "save-tracepoints" "test deprecated completion"
-
-# Restore globals modified in this test...
-set timeout $oldtimeout1
-
return 0
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-05-01 9:17 ` Jan Kratochvil
2011-05-02 14:00 ` Marek Polacek
@ 2011-05-02 14:19 ` Pedro Alves
2011-05-02 14:53 ` Jan Kratochvil
1 sibling, 1 reply; 28+ messages in thread
From: Pedro Alves @ 2011-05-02 14:19 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Marek Polacek, Joel Brobecker
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-05-02 14:19 ` Pedro Alves
@ 2011-05-02 14:53 ` Jan Kratochvil
2011-05-02 15:30 ` Pedro Alves
2011-05-05 15:11 ` Tom Tromey
0 siblings, 2 replies; 28+ messages in thread
From: Jan Kratochvil @ 2011-05-02 14:53 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Marek Polacek, Joel Brobecker
On Mon, 02 May 2011 16:19:25 +0200, Pedro Alves wrote:
> On Sunday 01 May 2011 10:16:30, Jan Kratochvil wrote:
> > 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?
Replacing this send_gdb + gdb_expect by gdb_test "complete ..." makes the GDB
codebase/testsuite more maintainable so I thought it could be changed now.
I found easier to replace the current constructs by gdb_test "complete ..." at
once although one can fix the gdb_expect first and delete it afterwards if you
wish.
Or are you against the replacement by gdb_test "complete ..."?
I have checked the code paths (despite what Tom says) and personally I cannot
imagine a difference between \t and the "complete" command.
> > > @@ -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?
Aha. But this testcase always FAILs (which it always considers as XFAIL)
because:
(1) gdb.base/break1.o prevents the completion (during in-tree build)
(2) GDB 7.3.50.20110502-cvs now completes it (with bundled readline) as:
>p "break1.c < (note the trailing space)
instead of expected
>p "break1.c"<
so it FAILs/XFAILs anyway.
So I am not sure what should be there when it cannot work anyway.
Thanks,
Jan
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-04-29 14:10 ` Marek Polacek
@ 2011-05-02 14:58 ` Pedro Alves
0 siblings, 0 replies; 28+ messages in thread
From: Pedro Alves @ 2011-05-02 14:58 UTC (permalink / raw)
To: gdb-patches; +Cc: Marek Polacek
On Friday 29 April 2011 15:10:03, Marek Polacek wrote:
> On 04/28/2011 05:14 PM, Pedro Alves wrote:
> > Are you sure? I did't see that when I try it.
>
> For instance this is OK:
> gdb_test_no_output "complete hfgfh" "complete 'hfgfh'"
>
> but this is not:
> gdb_test_no_output "hfgfh\t" "complete 'hfgfh'"
Haven't really looked at exactly why that happens (it's
most likely something withing gdb_test_multiple that causes
it), we can always keep the send_gdb, and pass an empty
command to gdb_test_multiple, like in the patchlet below.
>
> > And I'd much rather we do this (remote the races) as first step instead
> > of completely rewriting the whole test file into something completely
> > different...
>
> Good point.
>
> > A follow up step would convert/simplify the send_gdb+gdb_expects
> > into gdb_test&friends.
>
> Something like this attempt?
> http://sourceware.org/ml/gdb-patches/2011-04/msg00538.html
No, I mean something along the lines of the incomplete patch
below, really convert/simplifying the send_gdb+gdb_expects
into gdb_test&friends.
After that is done, the testfile should quite a bit smaller, and couple
of patterns should emerge. Then, looking at the patterns, we could
consider coming up with some helper functions that would allow
running the tests in several different ways, e.g, once with \t, and
another with "complete", as I suggested a couple of messages ago.
Made possible since we now understand what races were there in
the "\t" original version.
---
gdb/testsuite/gdb.base/completion.exp | 25 +++++++++++--------------
gdb/testsuite/lib/gdb.exp | 2 +-
2 files changed, 12 insertions(+), 15 deletions(-)
Index: src/gdb/testsuite/gdb.base/completion.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/completion.exp 2011-05-02 15:30:52.000000000 +0100
+++ src/gdb/testsuite/gdb.base/completion.exp 2011-05-02 15:50:30.649463000 +0100
@@ -94,21 +94,18 @@ if ![runto_main] then {
set oldtimeout1 $timeout
set timeout 30
-
+set test "complete 'hfgfh'"
send_gdb "hfgfh\t"
-gdb_expect {
- -re "^hfgfh\\\x07$"\
- { send_gdb "\n"
- gdb_expect {
- -re "Undefined command: \"hfgfh\"\\. Try \"help\"\\..*$gdb_prompt $"\
- { pass "complete 'hfgfh'"}
- -re ".*$gdb_prompt $" { fail "complete 'hfgfh'"}
- timeout {fail "(timeout) complete 'hfgfh'"}
- }
- }
- -re ".*$gdb_prompt $" { fail "complete 'hfgfh'" }
- timeout { fail "(timeout) complete 'hfgfh'" }
- }
+gdb_test_multiple "" "$test" {
+ -re "^hfgfh\\\x07$" {
+ send_gdb "\n"
+ gdb_test_multiple "" $test {
+ -re "Undefined command: \"hfgfh\"\\. Try \"help\"\\..*$gdb_prompt $" {
+ pass $test
+ }
+ }
+ }
+}
#exp_internal 0
Index: src/gdb/testsuite/lib/gdb.exp
===================================================================
--- src.orig/gdb/testsuite/lib/gdb.exp 2011-04-28 17:17:41.000000000 +0100
+++ src/gdb/testsuite/lib/gdb.exp 2011-05-02 15:52:55.889463000 +0100
@@ -593,7 +593,7 @@ proc gdb_test_multiple { command message
set message $command
}
- if [string match "*\[\r\n\]" $command] {
+ if { $command != "" && [string match "*\[\r\n\]" $command] } {
error "Invalid trailing newline in \"$message\" test"
}
--
Pedro Alves
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-05-02 14:53 ` Jan Kratochvil
@ 2011-05-02 15:30 ` Pedro Alves
2011-05-02 15:44 ` Joel Brobecker
2011-05-02 15:56 ` Jan Kratochvil
2011-05-05 15:11 ` Tom Tromey
1 sibling, 2 replies; 28+ messages in thread
From: Pedro Alves @ 2011-05-02 15:30 UTC (permalink / raw)
To: gdb-patches; +Cc: Jan Kratochvil, Marek Polacek, Joel Brobecker
On Monday 02 May 2011 15:52:29, Jan Kratochvil wrote:
> On Mon, 02 May 2011 16:19:25 +0200, Pedro Alves wrote:
> > On Sunday 01 May 2011 10:16:30, Jan Kratochvil wrote:
> > > 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?
>
> Replacing this send_gdb + gdb_expect by gdb_test "complete ..." makes the GDB
> codebase/testsuite more maintainable so I thought it could be changed now.
>
> I found easier to replace the current constructs by gdb_test "complete ..." at
> once although one can fix the gdb_expect first and delete it afterwards if you
> wish.
>
> Or are you against the replacement by gdb_test "complete ..."?
The "\t" method of completion interacts with readline, the
"complete command" method doesn't. I think it's useful and
important to test the "\t" version, especially since it's
what CLI users are using.
As principle, I don't like rewriting to make a not understood
problem go away. In this particular case, since it would be desirable
to keep at least one instance of the original form, we get to fix
at least that instance. But when we know how to fix it, we can
fix all of the instances. And then the original motivation to rewrite
using a different method disappears or at least diminishes.
>
> I have checked the code paths (despite what Tom says) and personally I cannot
> imagine a difference between \t and the "complete" command.
>
>
> > > > @@ -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?
>
> Aha. But this testcase always FAILs (which it always considers as XFAIL)
> because:
> (1) gdb.base/break1.o prevents the completion (during in-tree build)
> (2) GDB 7.3.50.20110502-cvs now completes it (with bundled readline) as:
> >p "break1.c < (note the trailing space)
> instead of expected
> >p "break1.c"<
> so it FAILs/XFAILs anyway.
> So I am not sure what should be there when it cannot work anyway.
Thanks. Sounds like break1.c was added (2003) after the test was
originally written (199[89], and since the completion never worked as
intended by the author of test, always XFAILing, it was never
noticed break1.o was in the way. I somehow not noticed the \",
making it so that my change wasn't enough anyway (needed yet one more
dot, or something better). I'll remove it.
--
Pedro Alves
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-05-02 15:30 ` Pedro Alves
@ 2011-05-02 15:44 ` Joel Brobecker
2011-05-02 15:50 ` Pedro Alves
2011-05-02 15:56 ` Jan Kratochvil
1 sibling, 1 reply; 28+ messages in thread
From: Joel Brobecker @ 2011-05-02 15:44 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Jan Kratochvil, Marek Polacek
My 2 cents...
> The "\t" method of completion interacts with readline, the
> "complete command" method doesn't. I think it's useful and
> important to test the "\t" version, especially since it's
> what CLI users are using.
I agree. But at the same time, do we need to only test completion
using this approach only (I initially suggested that we keep 1 test
that uses this approach, and do the rest with gdb_test "complete ...")?
Incidentally, the same argument can be made for testing the "complete"
command as well, as this is what IDEs use.
So, perhaps one possible evolution of the testcase is to write a
procedure that verifies both forms of completion...
--
Joel
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-05-02 15:44 ` Joel Brobecker
@ 2011-05-02 15:50 ` Pedro Alves
0 siblings, 0 replies; 28+ messages in thread
From: Pedro Alves @ 2011-05-02 15:50 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches, Jan Kratochvil, Marek Polacek
On Monday 02 May 2011 16:43:45, Joel Brobecker wrote:
> My 2 cents...
>
> > The "\t" method of completion interacts with readline, the
> > "complete command" method doesn't. I think it's useful and
> > important to test the "\t" version, especially since it's
> > what CLI users are using.
>
> I agree. But at the same time, do we need to only test completion
> using this approach only (I initially suggested that we keep 1 test
> that uses this approach, and do the rest with gdb_test "complete ...")?
> Incidentally, the same argument can be made for testing the "complete"
> command as well, as this is what IDEs use.
Note there are "complete foo" tests already in the testfile,
so we already cover both variants, though it's not systematic.
> So, perhaps one possible evolution of the testcase is to write a
> procedure that verifies both forms of completion...
Yes, agreed. I suggested that at least twice in this thread. :-)
--
Pedro Alves
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-05-02 15:30 ` Pedro Alves
2011-05-02 15:44 ` Joel Brobecker
@ 2011-05-02 15:56 ` Jan Kratochvil
2011-05-02 16:10 ` Pedro Alves
1 sibling, 1 reply; 28+ messages in thread
From: Jan Kratochvil @ 2011-05-02 15:56 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Marek Polacek, Joel Brobecker
On Mon, 02 May 2011 17:30:03 +0200, Pedro Alves wrote:
> The "\t" method of completion interacts with readline, the
> "complete command" method doesn't. I think it's useful and
> important to test the "\t" version, especially since it's
> what CLI users are using.
The question is do we test readline/ or gdb/?
For the readline/ part there is already gdb.base/readline.exp and for the "\t"
interaction there is (soon will be) at least:
[patch] testsuite: Test readline-6.2 "ask" regression
http://sourceware.org/ml/gdb-patches/2011-05/msg00002.html
gdb.base/readline-ask.exp
For the gdb/ part tests I find the "complete" command fully sufficient.
> In this particular case, since it would be desirable to keep at least one
> instance of the original form,
But not required to be in gdb.base/completion.exp .
> And then the original motivation to rewrite
> using a different method disappears or at least diminishes.
generalization over whole gdb/:
It is still very strong as the current codebase state is discouraging possible
contributors keeping the GDB development slow.
I understand one cannot change the whole codebase to a better / more
maintainable form over night but when there are attempts and patches offered
IMO the current codebase should not be actively kept worse.
Thanks,
Jan
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-05-02 15:56 ` Jan Kratochvil
@ 2011-05-02 16:10 ` Pedro Alves
2011-05-02 16:35 ` Jan Kratochvil
0 siblings, 1 reply; 28+ messages in thread
From: Pedro Alves @ 2011-05-02 16:10 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Marek Polacek, Joel Brobecker
On Monday 02 May 2011 16:55:27, Jan Kratochvil wrote:
> > In this particular case, since it would be desirable to keep at least one
> > instance of the original form,
>
> But not required to be in gdb.base/completion.exp .
This is about completion, using one form or the other.
We could move the "\t" form to readline-completion.exp,
but I think a systematic approach to testing all the
completion methods is better, and helps maintenance in the
long run.
> generalization over whole gdb/:
>
> It is still very strong as the current codebase state is discouraging possible
> contributors keeping the GDB development slow.
I'm not sure how to read that...
> I understand one cannot change the whole codebase to a better / more
> maintainable form over night but when there are attempts and patches offered
> IMO the current codebase should not be actively kept worse.
I took the time investigate the original issues with the code, write a patch
to fix them, explain the problems and the proposed fixes, in order to not keep the
knowledge to myself, and I've posted the beginnings of a patch that cleans
up the test further. I don't think it's fair to suggest I'm trying to keep
anything worse.
--
Pedro Alves
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-05-02 16:10 ` Pedro Alves
@ 2011-05-02 16:35 ` Jan Kratochvil
2011-05-02 16:54 ` Pedro Alves
0 siblings, 1 reply; 28+ messages in thread
From: Jan Kratochvil @ 2011-05-02 16:35 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Marek Polacek, Joel Brobecker
On Mon, 02 May 2011 18:09:50 +0200, Pedro Alves wrote:
> This is about completion, using one form or the other.
> We could move the "\t" form to readline-completion.exp,
> but I think a systematic approach to testing all the
> completion methods is better, and helps maintenance in the
> long run.
I was addressing this by the readline/ and gdb/ parts testing differentiation,
the first paragraph of my mail.
> > I understand one cannot change the whole codebase to a better / more
> > maintainable form over night but when there are attempts and patches offered
> > IMO the current codebase should not be actively kept worse.
>
> I took the time investigate the original issues with the code, write a patch
> to fix them, explain the problems and the proposed fixes, in order to not keep the
> knowledge to myself, and I've posted the beginnings of a patch that cleans
> up the test further. I don't think it's fair to suggest I'm trying to keep
> anything worse.
There was the proposal using gdb_test "complete ...":
Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
http://sourceware.org/ml/gdb-patches/2011-04/msg00538.html
(I do not have that mail reviewed but it gives the idea.)
I consider the gdb_test "complete ..." testfile better than the (even fixed)
send_gdb-gdb_expect testfile. Therefore I consider proposing the
send_gdb-gdb_expect testfile over the gdb_test "complete ..." testfile as
"actively trying to keep the testfile worse".
But there is the point that you do not consider the gdb_test "complete ..."
method having the same testing coverage as the "\t" testing method. This is
the point where we do not agree and I agree in such case the gdb_test
"complete ..." change is not acceptable (for you).
Thanks,
Jan
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-05-02 16:35 ` Jan Kratochvil
@ 2011-05-02 16:54 ` Pedro Alves
2011-05-02 17:04 ` Jan Kratochvil
0 siblings, 1 reply; 28+ messages in thread
From: Pedro Alves @ 2011-05-02 16:54 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Marek Polacek, Joel Brobecker
On Monday 02 May 2011 17:34:18, Jan Kratochvil wrote:
> On Mon, 02 May 2011 18:09:50 +0200, Pedro Alves wrote:
> > This is about completion, using one form or the other.
> > We could move the "\t" form to readline-completion.exp,
> > but I think a systematic approach to testing all the
> > completion methods is better, and helps maintenance in the
> > long run.
>
> I was addressing this by the readline/ and gdb/ parts testing differentiation,
> the first paragraph of my mail.
Sorry, I just snipped it as I didn't have anything to add.
I had looked at your new readline test, and it didn't appear
to cover every of interesting completion variant this
completion.exp test exercises, but I think I may have overlooked.
If it does, then I'll agree with putting that in, and switching
the completion.exp test to "complete" only.
--
Pedro Alves
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-05-02 16:54 ` Pedro Alves
@ 2011-05-02 17:04 ` Jan Kratochvil
2011-05-02 17:21 ` Jan Kratochvil
2011-05-02 17:23 ` Pedro Alves
0 siblings, 2 replies; 28+ messages in thread
From: Jan Kratochvil @ 2011-05-02 17:04 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Marek Polacek, Joel Brobecker
On Mon, 02 May 2011 18:54:34 +0200, Pedro Alves wrote:
> I had looked at your new readline test, and it didn't appear
> to cover every of interesting completion variant this
> completion.exp test exercises, but I think I may have overlooked.
I believe the good enough test is that "\t" invokes GDB complete_line (via
readline_line_completion_function). GDB complete_command also obviously leads
to complete_line.
Therefore if one tests "\t" really works in any single case (which is tested
by gdb.base/readline-ask.exp as it completes the inferior symbol names) one
can then test just the "complete" command for all the other completion
variants with the same coverage.
> If it does, then I'll agree with putting that in, and switching
> the completion.exp test to "complete" only.
But maybe the coverage equivalance is not so obvious as I considered it.
Thanks,
Jan
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-05-02 17:04 ` Jan Kratochvil
@ 2011-05-02 17:21 ` Jan Kratochvil
2011-05-02 17:23 ` Pedro Alves
1 sibling, 0 replies; 28+ messages in thread
From: Jan Kratochvil @ 2011-05-02 17:21 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Marek Polacek, Joel Brobecker
On Mon, 02 May 2011 19:03:38 +0200, Jan Kratochvil wrote:
> GDB complete_command also obviously leads to complete_line.
But it does not emulate _rl_find_completion_word perfectly:
/* complete_line assumes that its first argument is somewhere
within, and except for filenames at the beginning of, the word to
be completed. The following crude imitation of readline's
word-breaking tries to accomodate this. */
> But maybe the coverage equivalance is not so obvious as I considered it.
So OK, if you have coded it already and gdb_test "complete ..." is not
a perfect replacement I agree it should be kept just with your patch.
Thanks,
Jan
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-05-02 17:04 ` Jan Kratochvil
2011-05-02 17:21 ` Jan Kratochvil
@ 2011-05-02 17:23 ` Pedro Alves
2011-05-02 17:29 ` Jan Kratochvil
1 sibling, 1 reply; 28+ messages in thread
From: Pedro Alves @ 2011-05-02 17:23 UTC (permalink / raw)
To: gdb-patches; +Cc: Jan Kratochvil, Marek Polacek, Joel Brobecker
On Monday 02 May 2011 18:03:38, Jan Kratochvil wrote:
> On Mon, 02 May 2011 18:54:34 +0200, Pedro Alves wrote:
> > I had looked at your new readline test, and it didn't appear
> > to cover every of interesting completion variant this
> > completion.exp test exercises, but I think I may have overlooked.
>
> I believe the good enough test is that "\t" invokes GDB complete_line (via
> readline_line_completion_function). GDB complete_command also obviously leads
> to complete_line.
>
> Therefore if one tests "\t" really works in any single case (which is tested
> by gdb.base/readline-ask.exp as it completes the inferior symbol names) one
> can then test just the "complete" command for all the other completion
> variants with the same coverage.
I think it's mostly about the readline line buffer <-> gdb interaction.
Some examples come to mind from reading completion.exp, not sure
if your test covers all of these:
- test that the command that was completed works after "\n", lest
we got the completion correctly, but left the line/command buffer
clobbered, and we execute the wrong command, for example.
- test what happens if you write a full command, and try to expand,
and there is no possible completion. execute the command. check that
it did really execute, and that the completion code didn't clobber the
internal line/command buffer.
- test that gdb asks a question on completion request; test that
the question looks sane, and that when we answer it, the correct
answer is actually chosen. I think your test does some of this.
- test that \t completion works correctly (or doesn't work) when there's
a trailing whitespace
--
Pedro Alves
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-05-02 17:23 ` Pedro Alves
@ 2011-05-02 17:29 ` Jan Kratochvil
2011-05-02 17:53 ` Pedro Alves
0 siblings, 1 reply; 28+ messages in thread
From: Jan Kratochvil @ 2011-05-02 17:29 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Marek Polacek, Joel Brobecker
On Mon, 02 May 2011 19:23:20 +0200, Pedro Alves wrote:
> - test that \t completion works correctly (or doesn't work) when there's
> a trailing whitespace
This part works even with complete_command.
But otherwise I agree now complete_command is not a full replacement.
Thanks,
Jan
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-05-02 17:29 ` Jan Kratochvil
@ 2011-05-02 17:53 ` Pedro Alves
2011-05-02 17:56 ` Pedro Alves
0 siblings, 1 reply; 28+ messages in thread
From: Pedro Alves @ 2011-05-02 17:53 UTC (permalink / raw)
To: gdb-patches; +Cc: Jan Kratochvil, Marek Polacek, Joel Brobecker
On Monday 02 May 2011 18:20:39, Jan Kratochvil wrote:
> So OK, if you have coded it already and gdb_test "complete ..." is not
> a perfect replacement I agree it should be kept just with your patch.
On Monday 02 May 2011 18:28:26, Jan Kratochvil wrote:
> But otherwise I agree now complete_command is not a full replacement.
--
Pedro Alves
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-05-02 17:53 ` Pedro Alves
@ 2011-05-02 17:56 ` Pedro Alves
0 siblings, 0 replies; 28+ messages in thread
From: Pedro Alves @ 2011-05-02 17:56 UTC (permalink / raw)
To: gdb-patches; +Cc: Jan Kratochvil, Marek Polacek, Joel Brobecker
[Whoops, sorry, fat-fingered. What I meant to write was:]
On Monday 02 May 2011 18:20:39, Jan Kratochvil wrote:
> So OK, if you have coded it already and gdb_test "complete ..." is not
> a perfect replacement I agree it should be kept just with your patch.
On Monday 02 May 2011 18:28:26, Jan Kratochvil wrote:
> But otherwise I agree now complete_command is not a full replacement.
Thanks, applied as below.
Pedro Alves
2011-05-02 Pedro Alves <pedro@codesourcery.com>
PR testsuite/12649
Fix races.
* gdb.base/completion.exp: Remove all sleep calls. Remove
unnecessary regexs. Don't explicitly expect anything after the
prompt. Eat the prompt if necessary.
---
gdb/testsuite/gdb.base/completion.exp | 137 +++++++++-------------------------
1 file changed, 38 insertions(+), 99 deletions(-)
Index: src/gdb/testsuite/gdb.base/completion.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/completion.exp 2011-05-02 18:40:05.019462999 +0100
+++ src/gdb/testsuite/gdb.base/completion.exp 2011-05-02 18:48:17.719463000 +0100
@@ -96,7 +96,6 @@ set timeout 30
send_gdb "hfgfh\t"
-sleep 1
gdb_expect {
-re "^hfgfh\\\x07$"\
{ send_gdb "\n"
@@ -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"
@@ -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'" }
@@ -142,7 +130,6 @@ gdb_expect {
send_gdb "show output-\t"
-sleep 1
gdb_expect {
-re "^show output-radix $"\
{ send_gdb "\n"
@@ -153,27 +140,15 @@ 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-'" }
}
send_gdb "p\t"
-sleep 1
gdb_expect {
-re "^p\\\x07$"\
{ send_gdb "\n"
- sleep 1
gdb_expect {
-re "The history is empty\\..*$gdb_prompt $"\
{ pass "complete 'p'"}
@@ -186,11 +161,9 @@ gdb_expect {
}
send_gdb "p \t"
-sleep 3
gdb_expect {
-re "^p \\\x07$"\
{ send_gdb "\n"
- sleep 1
gdb_expect {
-re "The history is empty\\..*$gdb_prompt $"\
{ pass "complete 'p '"}
@@ -204,7 +177,6 @@ gdb_expect {
send_gdb "info t foo\t"
-sleep 1
gdb_expect {
-re "^info t foo\\\x07$"\
{ send_gdb "\n"
@@ -220,7 +192,6 @@ gdb_expect {
}
send_gdb "info t\t"
-sleep 1
gdb_expect {
-re "^info t\\\x07$"\
{ send_gdb "\n"
@@ -238,7 +209,6 @@ gdb_expect {
send_gdb "info t \t"
-sleep 1
gdb_expect {
-re "^info t \\\x07$"\
{ send_gdb "\n"
@@ -256,7 +226,6 @@ gdb_expect {
send_gdb "info asdfgh\t"
-sleep 1
gdb_expect {
-re "^info asdfgh\\\x07$"\
{ send_gdb "\n"
@@ -274,7 +243,6 @@ gdb_expect {
send_gdb "info asdfgh \t"
-sleep 1
gdb_expect {
-re "^info asdfgh \\\x07$"\
{ send_gdb "\n"
@@ -291,7 +259,6 @@ gdb_expect {
}
send_gdb "info\t"
-sleep 1
gdb_expect {
-re "^info $"\
{ send_gdb "\n"
@@ -307,7 +274,6 @@ gdb_expect {
}
send_gdb "info \t"
-sleep 1
gdb_expect {
-re "^info \\\x07$"\
{ send_gdb "\n"
@@ -322,14 +288,12 @@ gdb_expect {
timeout { fail "(timeout) complete 'info '" }
}
-
send_gdb "info \t"
-sleep 1
gdb_expect {
-re "^info \\\x07$"\
{ send_gdb "\t"
gdb_expect {
- -re "address.*types.*$gdb_prompt info $"\
+ -re "address.*types.*$gdb_prompt "\
{ send_gdb "\n"
gdb_expect {
-re "\"info\".*unambiguous\\..*$gdb_prompt $"\
@@ -365,7 +329,6 @@ gdb_expect {
send_gdb "p \"break1\t"
-sleep 1
gdb_expect {
-re "^p \"break1\\\x07$"\
{ send_gdb "\n"
@@ -381,20 +344,12 @@ gdb_expect {
timeout {fail "(timeout) complete 'p \"break1'"}
}
}
- -re "^p \"break1.*$"
- { send_gdb "\n"
- gdb_expect {
- -re ".*$gdb_prompt $" { fail "complete 'p \"break1'"}
- timeout {fail "(timeout) complete 'p \"break1'"}
- }
- }
-re ".*$gdb_prompt $" { fail "complete 'p \"break1'" }
timeout { fail "(timeout) complete 'p \"break1'" }
}
setup_xfail "*-*-*"
send_gdb "p \"break1.\t"
-sleep 1
gdb_expect {
-re "^p \"break1\\.\\\x07$"\
{ send_gdb "\n"
@@ -422,7 +377,6 @@ gdb_expect {
}
send_gdb "p 'arg\t"
-sleep 1
gdb_expect {
-re "^p 'arg\\\x07$"\
{ send_gdb "\n"
@@ -438,12 +392,11 @@ gdb_expect {
}
send_gdb "p 'arg\t"
-sleep 1
gdb_expect {
-re "^p 'arg\\\x07$" {
send_gdb "\t"
gdb_expect {
- -re ".*argv.*$gdb_prompt p 'arg$" {
+ -re ".*argv.*$gdb_prompt " {
send_gdb "\n"
gdb_expect {
-re "(Invalid character constant\\.|Unmatched single quote\\.).*$gdb_prompt $" {
@@ -503,7 +456,6 @@ gdb_expect {
# So, I'm hoping that there is no system with a static library variable named
# `no_var_by_this_name'.
send_gdb "p no_var_named_this-arg\t"
-sleep 1
gdb_expect {
-re "^p no_var_named_this-arg\\\x07$" {
send_gdb "\n"
@@ -528,12 +480,11 @@ gdb_expect {
}
send_gdb "p no_var_named_this-arg\t"
-sleep 1
gdb_expect {
-re "^p no_var_named_this-arg\\\x07$" {
send_gdb "\t"
gdb_expect {
- -re ".*argv.*$gdb_prompt p no_var_named_this-arg$" {
+ -re ".*argv.*$gdb_prompt " {
send_gdb "\n"
gdb_expect {
-re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
@@ -548,28 +499,26 @@ gdb_expect {
}
}
-re "(There are $decimal possibilities\\. Do you really\r\nwish to see them all.|Display all $decimal possibilities.) \\(y or n\\)$" {
- send_gdb "n"
+ send_gdb "n\n"
+
+ # Eat the prompt
gdb_expect {
- -re "\\(gdb\\) p no_var_named_this-arg$" {
- send_gdb "\n"
- gdb_expect {
- -re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
- pass "complete (2) 'p no_var_named_this-arg'"
- }
- -re ".*$gdb_prompt $" {
- fail "complete (2) 'p no_var_named_this-arg'"
- }
- timeout {
- fail "(timeout) complete (2) 'p no_var_named_this-arg'"
- }
- }
+ -re "$gdb_prompt " {
+ pass "complete (2) 'p no_var_named_this-arg' (eat prompt)"
+ }
+ timeout { fail "(timeout) complete (2) 'p no_var_named_this-' (eat prompt)" }
+ }
+
+ gdb_expect {
+ -re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
+ pass "complete (2) 'p no_var_named_this-arg'"
}
-re ".*$gdb_prompt $" {
- fail "complete (2) 'p no_var_named_this-arg'"
- }
+ fail "complete (2) 'p no_var_named_this-arg'"
+ }
timeout {
- fail "(timeout) complete (2) 'p no_var_named_this-arg'"
- }
+ fail "(timeout) complete (2) 'p no_var_named_this-arg'"
+ }
}
}
-re ".*$gdb_prompt $" {
@@ -583,37 +532,34 @@ gdb_expect {
}
send_gdb "p no_var_named_this-\t"
-sleep 1
gdb_expect {
-re "^p no_var_named_this-\\\x07$" {
send_gdb "\t"
gdb_expect {
-re "(There are $decimal possibilities\\. Do you really\r\nwish to see them all.|Display all $decimal possibilities.) \\(y or n\\)$" {
- send_gdb "n"
+ send_gdb "n\n"
+
+ # Eat the prompt
gdb_expect {
- -re "\\(gdb\\) p no_var_named_this-$" {
- send_gdb "\n"
- gdb_expect {
- -re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
- pass "complete (2) 'p no_var_named_this-'"
- }
- -re ".*$gdb_prompt $" {
- fail "complete (2) 'p no_var_named_this-'"
- }
- timeout {
- fail "(timeout) complete (2) 'p no_var_named_this-'"
- }
- }
+ -re "$gdb_prompt " {
+ pass "complete (2) 'p no_var_named_this-' (eat prompt)"
+ }
+ timeout { fail "(timeout) complete (2) 'p no_var_named_this-' (eat prompt)" }
+ }
+
+ gdb_expect {
+ -re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
+ pass "complete (2) 'p no_var_named_this-'"
}
-re ".*$gdb_prompt $" {
- fail "complete (2) 'p no_var_named_this-'"
- }
+ fail "complete (2) 'p no_var_named_this-'"
+ }
timeout {
- fail "(timeout) complete (2) 'p no_var_named_this-'"
- }
+ fail "(timeout) complete (2) 'p no_var_named_this-'"
+ }
}
}
- -re ".*argv.*$gdb_prompt p no_var_named_this-$" {
+ -re ".*argv.*$gdb_prompt $" {
send_gdb "\n"
gdb_expect {
-re "No symbol \"no_var_named_this\" in current context\\..*$gdb_prompt $" {
@@ -638,11 +584,9 @@ gdb_expect {
}
send_gdb "p values\[0\].a\t"
-sleep 3
gdb_expect {
-re "^p values.0..a_field $"\
{ send_gdb "\n"
- sleep 1
gdb_expect {
-re "^.* = 0.*$gdb_prompt $"\
{ pass "complete 'p values\[0\].a'"}
@@ -761,7 +705,6 @@ gdb_test " " "Source directories searche
send_gdb "complete file ./gdb.base/compl\n"
-sleep 1
gdb_expect {
-re "file ./gdb.base/completion\\.exp.*$gdb_prompt $"
{ pass "complete-command 'file ./gdb.base/compl'"}
@@ -770,7 +713,6 @@ gdb_expect {
}
send_gdb "file ./gdb.base/complet\t"
-sleep 1
gdb_expect {
-re "^file ./gdb.base/completion\\.exp $"\
{ send_gdb "\n"
@@ -788,14 +730,12 @@ gdb_expect {
}
send_gdb "info func marke\t"
-sleep 1
gdb_expect {
-re "^info func marke.*r$"\
{
send_gdb "\t\t"
- sleep 3
gdb_expect {
- -re "marker1.*$gdb_prompt info func marker$"\
+ -re "marker1.*$gdb_prompt "\
{ send_gdb "\n"
gdb_expect {
-re "All functions matching regular expression \"marker\":.*File.*break1.c:\r\nint marker1\\((void|)\\);\r\nint marker2\\(int\\).*marker3\\(char.*char.*\\).*marker4\\(long( int)?\\);.*$gdb_prompt $"\
@@ -814,9 +754,8 @@ gdb_expect {
send_gdb "set follow-fork-mode \t\t"
-sleep 1
gdb_expect {
- -re "child.*parent.*$gdb_prompt set follow-fork-mode $"\
+ -re "child.*parent.*$gdb_prompt "\
{ send_gdb "\n"
gdb_expect {
-re "Requires an argument.*child.*parent.*$gdb_prompt $"\
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
2011-05-02 14:53 ` Jan Kratochvil
2011-05-02 15:30 ` Pedro Alves
@ 2011-05-05 15:11 ` Tom Tromey
1 sibling, 0 replies; 28+ messages in thread
From: Tom Tromey @ 2011-05-05 15:11 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: Pedro Alves, gdb-patches, Marek Polacek, Joel Brobecker
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
Jan> I have checked the code paths (despite what Tom says) and
Jan> personally I cannot imagine a difference between \t and the
Jan> "complete" command.
See:
http://sourceware.org/bugzilla/show_bug.cgi?id=9007
http://sourceware.org/bugzilla/show_bug.cgi?id=11048
Tom
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2011-05-05 15:11 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-27 14:59 [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649) Marek Polacek
2011-04-27 15:05 ` Joel Brobecker
2011-04-27 15:13 ` Tom Tromey
2011-04-27 15:23 ` Pedro Alves
2011-04-27 17:41 ` Marek Polacek
2011-04-28 14:19 ` Pedro Alves
2011-04-28 15:14 ` Pedro Alves
2011-04-29 14:10 ` Marek Polacek
2011-05-02 14:58 ` Pedro Alves
2011-05-01 9:17 ` Jan Kratochvil
2011-05-02 14:00 ` Marek Polacek
2011-05-02 14:19 ` Pedro Alves
2011-05-02 14:53 ` Jan Kratochvil
2011-05-02 15:30 ` Pedro Alves
2011-05-02 15:44 ` Joel Brobecker
2011-05-02 15:50 ` Pedro Alves
2011-05-02 15:56 ` Jan Kratochvil
2011-05-02 16:10 ` Pedro Alves
2011-05-02 16:35 ` Jan Kratochvil
2011-05-02 16:54 ` Pedro Alves
2011-05-02 17:04 ` Jan Kratochvil
2011-05-02 17:21 ` Jan Kratochvil
2011-05-02 17:23 ` Pedro Alves
2011-05-02 17:29 ` Jan Kratochvil
2011-05-02 17:53 ` Pedro Alves
2011-05-02 17:56 ` Pedro Alves
2011-05-05 15:11 ` Tom Tromey
2011-04-28 11:56 ` Marek Polacek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox