This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
- From: Pedro Alves <pedro at codesourcery dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Cc: gdb-patches at sourceware dot org, Marek Polacek <mpolacek at redhat dot com>, Joel Brobecker <brobecker at adacore dot com>
- Date: Mon, 2 May 2011 15:19:25 +0100
- Subject: Re: [RFC] Fixing gdb.base/completion.exp (PR testsuite/12649)
- References: <4DB82F26.30801@redhat.com> <201104281614.31962.pedro@codesourcery.com> <20110501091630.GA16372@host1.jankratochvil.net>
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