[testsuite] require readline for gdb.linespec/explicit.exp tab-completion tests
Commit Message
On 09/17/2015 12:28 AM, Sandra Loosemore wrote:
> Tab-completion tests require readline support. The ones in
> gdb.base/completion.exp are already conditional, but the recently-added
> gdb.linespec/explicit.exp tests are not. This patch fixes that, and
> also an obvious typo ("faill" -> "fail"). The patch is kind of long
> because I had to insert an "if" block and reindent, but there's nothing
> else going on here. OK to commit?
I'm not a maintainer, but I wouldn't hesitate to commit this as obvious.
Thank you for catching (and fixing) that.
> The immediate problem is that the response to the "break -function
> mai\t" test doesn't always include the ^G character in the output
> pattern, and the other FAILs are cascading off of the testcase not being
> structured to recover from a pattern match failure (it's sending more
> commands without a newline to terminate the one that failed). But, I
> have no clue why GDB is giving a ^G in some configurations and not
> others (I observed that this test fails on nios2-elf target but passes
> on nios2-linux-gnu, for instance), or whether this difference is
> indicative of an actual bug. So I've not touched that part of the
> testcase.
Can you reliably reproduce the failure? I have been unable to do so
locally, but I have a couple of guesses why this test might fail.
I've attached a patch where I've made two changes. First, I've switched
from using "main" as a unique completion to some goofy new function
where the name is hopefully truly unique.
Second I believe the test is incorrect (or at least in combination with
some readline versions?). ^G is output only for non-unique completions.
So with this particular test, I don't think it should be present at all.
I've removed it in the patch.
If you can reliably reproduce the failure, I sure would love to know
whether this patch helps at all.
Keith
Comments
On 09/17/2015 12:50 PM, Keith Seitz wrote:
> On 09/17/2015 12:28 AM, Sandra Loosemore wrote:
>
>> The immediate problem is that the response to the "break -function
>> mai\t" test doesn't always include the ^G character in the output
>> pattern, and the other FAILs are cascading off of the testcase not being
>> structured to recover from a pattern match failure (it's sending more
>> commands without a newline to terminate the one that failed). But, I
>> have no clue why GDB is giving a ^G in some configurations and not
>> others (I observed that this test fails on nios2-elf target but passes
>> on nios2-linux-gnu, for instance), or whether this difference is
>> indicative of an actual bug. So I've not touched that part of the
>> testcase.
>
> Can you reliably reproduce the failure? I have been unable to do so
> locally, but I have a couple of guesses why this test might fail.
>
> I've attached a patch where I've made two changes. First, I've switched
> from using "main" as a unique completion to some goofy new function
> where the name is hopefully truly unique.
>
> Second I believe the test is incorrect (or at least in combination with
> some readline versions?). ^G is output only for non-unique completions.
> So with this particular test, I don't think it should be present at all.
> I've removed it in the patch.
>
> If you can reliably reproduce the failure, I sure would love to know
> whether this patch helps at all.
Your patch makes these tests all PASS on both nios2-elf (where it
formerly FAILed due to not beeping) and nios2-linux-gnu (where it used
to beep and PASS). I had come to pretty much the same conclusions -- I
didn't think it was supposed to beep if there was a unique completion,
and I thought there must be some sort of a duplicate symbol table entry
for "main" on the Linux target coming from the startup code or something
like that. But it looks to me like the completer *is* finding a unique
completion after beeping anyway.
So, while your patch does fix the FAILs, I'm wondering if there is a
genuine code bug in the completer here; is it using different logic for
deciding whether to beep than in what it does afterwards? Personally,
I'd feel uncomfortable changing the testcase to use something other than
"main" without understanding why it beeps in some configurations and not
others, because that might just be papering over the bug instead of
fixing a bad testcase.
-Sandra
Keith Seitz <keiths@redhat.com> writes:
> On 09/17/2015 12:28 AM, Sandra Loosemore wrote:
>> Tab-completion tests require readline support. The ones in
>> gdb.base/completion.exp are already conditional, but the recently-added
>> gdb.linespec/explicit.exp tests are not. This patch fixes that, and
>> also an obvious typo ("faill" -> "fail"). The patch is kind of long
>> because I had to insert an "if" block and reindent, but there's nothing
>> else going on here. OK to commit?
>
> I'm not a maintainer, but I wouldn't hesitate to commit this as obvious.
> Thank you for catching (and fixing) that.
Agreed.
On 09/17/2015 10:30 PM, Doug Evans wrote:
> Keith Seitz <keiths@redhat.com> writes:
>> On 09/17/2015 12:28 AM, Sandra Loosemore wrote:
>>> Tab-completion tests require readline support. The ones in
>>> gdb.base/completion.exp are already conditional, but the recently-added
>>> gdb.linespec/explicit.exp tests are not. This patch fixes that, and
>>> also an obvious typo ("faill" -> "fail"). The patch is kind of long
>>> because I had to insert an "if" block and reindent, but there's nothing
>>> else going on here. OK to commit?
>>
>> I'm not a maintainer, but I wouldn't hesitate to commit this as obvious.
>> Thank you for catching (and fixing) that.
>
> Agreed.
OK, since this seems "obvious" and uncontroversial I've gone ahead and
pushed this patch to get it out of my pile.
-Sandra
@@ -1,6 +1,6 @@
/* This testcase is part of GDB, the GNU debugger.
- Copyright 2012-2013 Free Software Foundation, Inc.
+ Copyright 2012-2015 Free Software Foundation, Inc.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -42,6 +42,12 @@ myfunction (int arg)
return r;
}
+static int
+explicit_exp_really_unique_function_name (int arg)
+{
+ return arg;
+}
+
int
main (void)
{
@@ -52,5 +58,5 @@ main (void)
for (i = 0, j = 0; i < 1000; ++i)
j += myfunction (0);
- return myfunction2 (j);
+ return explicit_exp_really_unique_function_name (myfunction2 (j));
}
@@ -181,9 +181,9 @@ namespace eval $testfile {
}
set tst "complete unique function name"
- send_gdb "break -function mai\t"
+ send_gdb "break -function explicit_exp_really_unique_f\t"
gdb_test_multiple "" $tst {
- "break -function mai\\\x07n" {
+ "break -function explicit_exp_really_unique_function_name " {
send_gdb "\n"
gdb_test "" ".*Breakpoint \[0-9\]+.*" $tst
gdb_test_no_output "delete \$bpnum" "delete $tst breakpoint"