Message ID | 20230222091110.2995513-1-blarsen@redhat.com |
---|---|
State | Committed |
Commit | a3da2e7e550c4fe79128b5e532dbb90df4d4f418 |
Headers |
Return-Path: <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1FB92385840F for <patchwork@sourceware.org>; Wed, 22 Feb 2023 09:11:38 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1FB92385840F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1677057098; bh=0jrMoA9T2pQo+ujql2xfbLfZQlkJWINVsRbDlg9tocc=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=NsNf485guQV8IRDWrJ68jTSXqxGMvLsAakN1tX6Gz7F2nwIctv7MMvBZm3AYxpYLh XdgZsn3xUp4PurtBKIi0oT4pKDInezaqM9AiLKlYS8aBztFHl72gV5U3wG6qdYkjIc H4ppAnpvjLMJQPnUGDZa4pUVKfNitnwlTJ4sWD7U= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 5BB063858D33 for <gdb-patches@sourceware.org>; Wed, 22 Feb 2023 09:11:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5BB063858D33 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-614-M91m9ZKyNR-jXTmwgljwBg-1; Wed, 22 Feb 2023 04:11:13 -0500 X-MC-Unique: M91m9ZKyNR-jXTmwgljwBg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 782BC18A6462 for <gdb-patches@sourceware.org>; Wed, 22 Feb 2023 09:11:13 +0000 (UTC) Received: from fedora.redhat.com (unknown [10.39.195.8]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 00D1B4024CA0; Wed, 22 Feb 2023 09:11:12 +0000 (UTC) To: gdb-patches@sourceware.org Cc: Bruno Larsen <blarsen@redhat.com> Subject: [PATCH] gdb/testsuite: Improve testing of GDB's completion functions Date: Wed, 22 Feb 2023 10:11:10 +0100 Message-Id: <20230222091110.2995513-1-blarsen@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list <gdb-patches.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=subscribe> From: Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> Reply-To: Bruno Larsen <blarsen@redhat.com> Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> |
Series |
gdb/testsuite: Improve testing of GDB's completion functions
|
|
Commit Message
Guinevere Larsen
Feb. 22, 2023, 9:11 a.m. UTC
When looking at some failures of gdb.linespec/cp-completion-aliases.exp, I noticed that when a completion test will fail, it always fails with a timeout. This is because most completion tests use gdb_test_multiple and only add a check for the correct output. This commit adds new options for both, tab and command completion. For command completion, the new option will check if the prompt was printed, and fail in this case. This is enough to know that the test has failed because the check comes after the PASS path. For tab completion, we have to check if GDB outputted more than just the input line, because sometimes GDB would have printed a partial line before finishing with the correct completion. --- gdb/testsuite/lib/completion-support.exp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
Comments
>>>>> "Bruno" == Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
Bruno> When looking at some failures of gdb.linespec/cp-completion-aliases.exp,
Bruno> I noticed that when a completion test will fail, it always fails with a
Bruno> timeout. This is because most completion tests use gdb_test_multiple
Bruno> and only add a check for the correct output. This commit adds new
Bruno> options for both, tab and command completion.
Looks good to me.
Approved-By: Tom Tromey <tom@tromey.com>
Tom
On 24/02/2023 20:06, Tom Tromey wrote: >>>>>> "Bruno" == Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes: > Bruno> When looking at some failures of gdb.linespec/cp-completion-aliases.exp, > Bruno> I noticed that when a completion test will fail, it always fails with a > Bruno> timeout. This is because most completion tests use gdb_test_multiple > Bruno> and only add a check for the correct output. This commit adds new > Bruno> options for both, tab and command completion. > > Looks good to me. > Approved-By: Tom Tromey <tom@tromey.com> > > Tom > Thanks, pushed!
On 2/22/23 10:11, Bruno Larsen via Gdb-patches wrote: > When looking at some failures of gdb.linespec/cp-completion-aliases.exp, > I noticed that when a completion test will fail, it always fails with a > timeout. This is because most completion tests use gdb_test_multiple > and only add a check for the correct output. This commit adds new > options for both, tab and command completion. > > For command completion, the new option will check if the prompt was > printed, and fail in this case. This is enough to know that the test has > failed because the check comes after the PASS path. For tab completion, > we have to check if GDB outputted more than just the input line, because > sometimes GDB would have printed a partial line before finishing with > the correct completion. This causes quite a few regressions with check-read1. For instance: ... (gdb) break baz(int, FAIL: gdb.cp/cpcompletion.exp: tab complete "break baz(int" double) Quit^M (gdb) ... Thanks, - Tom > --- > gdb/testsuite/lib/completion-support.exp | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/gdb/testsuite/lib/completion-support.exp b/gdb/testsuite/lib/completion-support.exp > index bf9c5ad352c..275f8874f15 100644 > --- a/gdb/testsuite/lib/completion-support.exp > +++ b/gdb/testsuite/lib/completion-support.exp > @@ -94,6 +94,9 @@ proc test_gdb_complete_tab_none { line } { > -re "^$line_re$completion::bell_re$" { > pass "$test" > } > + -re "$line_re\[^ \]+ $" { > + fail "$test" > + } > } > > clear_input_line $test > @@ -108,11 +111,15 @@ proc test_gdb_complete_tab_unique { input_line complete_line_re append_char_re } > > set test "tab complete \"$input_line\"" > send_gdb "$input_line\t" > + set partial_complete [string_to_regexp $input_line] > set res 1 > gdb_test_multiple "" "$test" { > -re "^$complete_line_re$append_char_re$" { > pass "$test" > } > + -re "$partial_complete\[^ \]+ $" { > + fail "$test" > + } > timeout { > fail "$test (timeout)" > set res -1 > @@ -164,6 +171,9 @@ proc test_gdb_complete_tab_multiple { input_line add_completed_line \ > } > } > } > + -re "${maybe_bell}\r\n.+\r\n$gdb_prompt $" { > + fail "$test" > + } > } > } > } > @@ -191,6 +201,9 @@ proc test_gdb_complete_cmd_unique { input_line complete_line_re } { > -re "^$cmd_re\r\n$complete_line_re\r\n$gdb_prompt $" { > pass $test > } > + -re "$gdb_prompt $" { > + fail "$test" > + } > } > } > > @@ -217,6 +230,9 @@ proc test_gdb_complete_cmd_multiple { cmd_prefix completion_word completion_list > -re "^$cmd_re\r\n$expected_re$gdb_prompt $" { > pass $test > } > + -re "$gdb_prompt $" { > + fail "$test" > + } > } > } >
On 15/07/2023 14:13, Tom de Vries wrote: > On 2/22/23 10:11, Bruno Larsen via Gdb-patches wrote: >> When looking at some failures of gdb.linespec/cp-completion-aliases.exp, >> I noticed that when a completion test will fail, it always fails with a >> timeout. This is because most completion tests use gdb_test_multiple >> and only add a check for the correct output. This commit adds new >> options for both, tab and command completion. >> >> For command completion, the new option will check if the prompt was >> printed, and fail in this case. This is enough to know that the test has >> failed because the check comes after the PASS path. For tab completion, >> we have to check if GDB outputted more than just the input line, because >> sometimes GDB would have printed a partial line before finishing with >> the correct completion. > > This causes quite a few regressions with check-read1. > > For instance: > ... > (gdb) break baz(int, FAIL: gdb.cp/cpcompletion.exp: tab complete > "break baz(int" > double) Quit^M > (gdb) > ... Hi! Sorry for taking so long to respond. I'd appreciate some help in solving, if you have the time. > > Thanks, > - Tom > >> --- >> gdb/testsuite/lib/completion-support.exp | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/gdb/testsuite/lib/completion-support.exp >> b/gdb/testsuite/lib/completion-support.exp >> index bf9c5ad352c..275f8874f15 100644 >> --- a/gdb/testsuite/lib/completion-support.exp >> +++ b/gdb/testsuite/lib/completion-support.exp >> @@ -94,6 +94,9 @@ proc test_gdb_complete_tab_none { line } { >> -re "^$line_re$completion::bell_re$" { >> pass "$test" >> } >> + -re "$line_re\[^ \]+ $" { >> + fail "$test" >> + } >> } >> clear_input_line $test >> @@ -108,11 +111,15 @@ proc test_gdb_complete_tab_unique { input_line >> complete_line_re append_char_re } >> set test "tab complete \"$input_line\"" >> send_gdb "$input_line\t" >> + set partial_complete [string_to_regexp $input_line] >> set res 1 >> gdb_test_multiple "" "$test" { >> -re "^$complete_line_re$append_char_re$" { >> pass "$test" >> } >> + -re "$partial_complete\[^ \]+ $" { >> + fail "$test" >> + } This is the specific change that causes the failures. The thinking behind it was that if we receive more characters, but not the whole complete_line, we got a failure. Something like this could detect if we have a unique - but wrong - suggestion or multiple options. This way it doesn't have to go to timeout every time, because it was making clang testing take too long. Is there any other way to detect if GDB is done with the suggestion? Or can we detect that read1 is being used, so this gets special cased?
On 7/25/23 17:40, Bruno Larsen wrote: > On 15/07/2023 14:13, Tom de Vries wrote: >> On 2/22/23 10:11, Bruno Larsen via Gdb-patches wrote: >>> When looking at some failures of gdb.linespec/cp-completion-aliases.exp, >>> I noticed that when a completion test will fail, it always fails with a >>> timeout. This is because most completion tests use gdb_test_multiple >>> and only add a check for the correct output. This commit adds new >>> options for both, tab and command completion. >>> >>> For command completion, the new option will check if the prompt was >>> printed, and fail in this case. This is enough to know that the test has >>> failed because the check comes after the PASS path. For tab completion, >>> we have to check if GDB outputted more than just the input line, because >>> sometimes GDB would have printed a partial line before finishing with >>> the correct completion. >> >> This causes quite a few regressions with check-read1. >> >> For instance: >> ... >> (gdb) break baz(int, FAIL: gdb.cp/cpcompletion.exp: tab complete >> "break baz(int" >> double) Quit^M >> (gdb) >> ... > Hi! Sorry for taking so long to respond. I'd appreciate some help in > solving, if you have the time. >> >> Thanks, >> - Tom >> >>> --- >>> gdb/testsuite/lib/completion-support.exp | 16 ++++++++++++++++ >>> 1 file changed, 16 insertions(+) >>> >>> diff --git a/gdb/testsuite/lib/completion-support.exp >>> b/gdb/testsuite/lib/completion-support.exp >>> index bf9c5ad352c..275f8874f15 100644 >>> --- a/gdb/testsuite/lib/completion-support.exp >>> +++ b/gdb/testsuite/lib/completion-support.exp >>> @@ -94,6 +94,9 @@ proc test_gdb_complete_tab_none { line } { >>> -re "^$line_re$completion::bell_re$" { >>> pass "$test" >>> } >>> + -re "$line_re\[^ \]+ $" { >>> + fail "$test" >>> + } >>> } >>> clear_input_line $test >>> @@ -108,11 +111,15 @@ proc test_gdb_complete_tab_unique { input_line >>> complete_line_re append_char_re } >>> set test "tab complete \"$input_line\"" >>> send_gdb "$input_line\t" >>> + set partial_complete [string_to_regexp $input_line] >>> set res 1 >>> gdb_test_multiple "" "$test" { >>> -re "^$complete_line_re$append_char_re$" { >>> pass "$test" >>> } >>> + -re "$partial_complete\[^ \]+ $" { >>> + fail "$test" >>> + } > > This is the specific change that causes the failures. The thinking > behind it was that if we receive more characters, but not the whole > complete_line, we got a failure. Something like this could detect if we > have a unique - but wrong - suggestion or multiple options. This way it > doesn't have to go to timeout every time, because it was making clang > testing take too long. > > Is there any other way to detect if GDB is done with the suggestion? Or > can we detect that read1 is being used, so this gets special cased? > The purpose of read1 is to reliably exercise FAILs in the test-suite, that are otherwise only occasionally occurring (see also "Race detection" in gdb/testsuite/README). It's typically a test-case problem where it passes or fails depending on how fast the input arrives. When read1 finds such a FAIL, we want to fix it because we want deterministic results. So, I'd say the relevant question is: did the change make the related test-cases racy, and does special casing try to hide the race? Thanks, - Tom
On 15/08/2023 07:45, Tom de Vries wrote: > On 7/25/23 17:40, Bruno Larsen wrote: >> On 15/07/2023 14:13, Tom de Vries wrote: >>> On 2/22/23 10:11, Bruno Larsen via Gdb-patches wrote: >>>> When looking at some failures of >>>> gdb.linespec/cp-completion-aliases.exp, >>>> I noticed that when a completion test will fail, it always fails >>>> with a >>>> timeout. This is because most completion tests use gdb_test_multiple >>>> and only add a check for the correct output. This commit adds new >>>> options for both, tab and command completion. >>>> >>>> For command completion, the new option will check if the prompt was >>>> printed, and fail in this case. This is enough to know that the >>>> test has >>>> failed because the check comes after the PASS path. For tab >>>> completion, >>>> we have to check if GDB outputted more than just the input line, >>>> because >>>> sometimes GDB would have printed a partial line before finishing with >>>> the correct completion. >>> >>> This causes quite a few regressions with check-read1. >>> >>> For instance: >>> ... >>> (gdb) break baz(int, FAIL: gdb.cp/cpcompletion.exp: tab complete >>> "break baz(int" >>> double) Quit^M >>> (gdb) >>> ... >> Hi! Sorry for taking so long to respond. I'd appreciate some help in >> solving, if you have the time. >>> >>> Thanks, >>> - Tom >>> >>>> --- >>>> gdb/testsuite/lib/completion-support.exp | 16 ++++++++++++++++ >>>> 1 file changed, 16 insertions(+) >>>> >>>> diff --git a/gdb/testsuite/lib/completion-support.exp >>>> b/gdb/testsuite/lib/completion-support.exp >>>> index bf9c5ad352c..275f8874f15 100644 >>>> --- a/gdb/testsuite/lib/completion-support.exp >>>> +++ b/gdb/testsuite/lib/completion-support.exp >>>> @@ -94,6 +94,9 @@ proc test_gdb_complete_tab_none { line } { >>>> -re "^$line_re$completion::bell_re$" { >>>> pass "$test" >>>> } >>>> + -re "$line_re\[^ \]+ $" { >>>> + fail "$test" >>>> + } >>>> } >>>> clear_input_line $test >>>> @@ -108,11 +111,15 @@ proc test_gdb_complete_tab_unique { >>>> input_line complete_line_re append_char_re } >>>> set test "tab complete \"$input_line\"" >>>> send_gdb "$input_line\t" >>>> + set partial_complete [string_to_regexp $input_line] >>>> set res 1 >>>> gdb_test_multiple "" "$test" { >>>> -re "^$complete_line_re$append_char_re$" { >>>> pass "$test" >>>> } >>>> + -re "$partial_complete\[^ \]+ $" { >>>> + fail "$test" >>>> + } >> >> This is the specific change that causes the failures. The thinking >> behind it was that if we receive more characters, but not the whole >> complete_line, we got a failure. Something like this could detect if >> we have a unique - but wrong - suggestion or multiple options. This >> way it doesn't have to go to timeout every time, because it was >> making clang testing take too long. >> >> Is there any other way to detect if GDB is done with the suggestion? >> Or can we detect that read1 is being used, so this gets special cased? >> > > The purpose of read1 is to reliably exercise FAILs in the test-suite, > that are otherwise only occasionally occurring (see also "Race > detection" in gdb/testsuite/README). > > It's typically a test-case problem where it passes or fails depending > on how fast the input arrives. > > When read1 finds such a FAIL, we want to fix it because we want > deterministic results. > > So, I'd say the relevant question is: did the change make the related > test-cases racy, and does special casing try to hide the race? > Yeah, I spoke to Andrew off-list and he explained this to me. The test itself wasn't racy on a light machine, but could be if it was under heavy load or if the expected output was too big. I have sent a v2 that fixes this without special casing: https://sourceware.org/pipermail/gdb-patches/2023-August/201361.html
diff --git a/gdb/testsuite/lib/completion-support.exp b/gdb/testsuite/lib/completion-support.exp index bf9c5ad352c..275f8874f15 100644 --- a/gdb/testsuite/lib/completion-support.exp +++ b/gdb/testsuite/lib/completion-support.exp @@ -94,6 +94,9 @@ proc test_gdb_complete_tab_none { line } { -re "^$line_re$completion::bell_re$" { pass "$test" } + -re "$line_re\[^ \]+ $" { + fail "$test" + } } clear_input_line $test @@ -108,11 +111,15 @@ proc test_gdb_complete_tab_unique { input_line complete_line_re append_char_re } set test "tab complete \"$input_line\"" send_gdb "$input_line\t" + set partial_complete [string_to_regexp $input_line] set res 1 gdb_test_multiple "" "$test" { -re "^$complete_line_re$append_char_re$" { pass "$test" } + -re "$partial_complete\[^ \]+ $" { + fail "$test" + } timeout { fail "$test (timeout)" set res -1 @@ -164,6 +171,9 @@ proc test_gdb_complete_tab_multiple { input_line add_completed_line \ } } } + -re "${maybe_bell}\r\n.+\r\n$gdb_prompt $" { + fail "$test" + } } } } @@ -191,6 +201,9 @@ proc test_gdb_complete_cmd_unique { input_line complete_line_re } { -re "^$cmd_re\r\n$complete_line_re\r\n$gdb_prompt $" { pass $test } + -re "$gdb_prompt $" { + fail "$test" + } } } @@ -217,6 +230,9 @@ proc test_gdb_complete_cmd_multiple { cmd_prefix completion_word completion_list -re "^$cmd_re\r\n$expected_re$gdb_prompt $" { pass $test } + -re "$gdb_prompt $" { + fail "$test" + } } }