From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 0DDE13858CDB for ; Fri, 1 Dec 2023 12:31:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0DDE13858CDB Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 0DDE13858CDB Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701433914; cv=none; b=u96OUpn1lqmew0bQf1dm1hB9XMpJcYBaxfm/j+w3QgWd7yaLq0zXtBgoQqeaF4IanxkxW/p0aYVxn8KTyC0AEk5S3Ep14nhbnLsJeKgrFgoiowGvtlSSYwe2TguUWhy6mDwgHcY9BLbk25X6V+mmIEL5NLAtcKwtP37YoNf8/JU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701433914; c=relaxed/simple; bh=fteVSsoWd2R5ph4j5jumtHR8PeiICtqYqEcl6cP024w=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=S3ymlw4a/I3eyIjcBZkTNX771awQrWhetBpHQjyzhJLCgYoDytfMpSmHvtqtTpLZRUtUmu+uebnxjuT8JHQi9ikMLarh2dQNt8GQLNd6k+XgS5nwTEblNWZ9laTwyHg5zZ6L7JkdIcAT7MuDL5Ovk51UegzLK3bD6inSd5kNK3s= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1701433912; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ZvAuvtoiTcXYBYn9jNrmdG+xGjYyDTGY7ELyqY3IZxA=; b=hafMcLfr4gJs8agwMvocvevext1cjMrLgVImm6bYeNrzSwN8Hs2ngKNAzDNsxY+dAdDAmP rirEekmHgQPXwGMHo6irbViQx9fPkLK2R8tim/49FaZmaFuyEuBYGNrxQp7k763fzW+eHo QQSwm2Li++4cm+QzbaVZ7Dxdb3UW2/Y= Received: from mail-ej1-f69.google.com (mail-ej1-f69.google.com [209.85.218.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-653-6Cg1XlxXOmG0_9UgOPPYYg-1; Fri, 01 Dec 2023 07:31:51 -0500 X-MC-Unique: 6Cg1XlxXOmG0_9UgOPPYYg-1 Received: by mail-ej1-f69.google.com with SMTP id a640c23a62f3a-a1a4c416d49so18855466b.1 for ; Fri, 01 Dec 2023 04:31:51 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701433909; x=1702038709; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ZvAuvtoiTcXYBYn9jNrmdG+xGjYyDTGY7ELyqY3IZxA=; b=pdreVi3GX+a49ONs6FKW2/d5mBjjnMfBkp9GKWo6MzYndJgnPnEmzIhFd6O6yF+qOr 0FmQmgyGythYK0PCtW6xuxLL/AevdRdo2t4+0r+jgfAlTITcWVpgRewfuyryV7m+knbh CUYpMNNM2EqQCklE04evdFUikpJyOLETdRlm+Q5MN+UsrTFNoe2IdEIu71/qbOziZaMb DISe8V68om5EnCTbzqepA82khmSWo90O+6rTnXY6RmC3U/Z71lNWoXEZ3oKo1/Uze9hN jYeivIYx6EF/p2GvONiY4XSWBmMu+WzwycjbTlrRD2wpsnAuuaHwUHrcu+OQWSG7lzc0 h5Og== X-Gm-Message-State: AOJu0YwzCkw3z1unCaMDAHPFIPcoali9Kt9hY2BYGMDSZPJR2xMbr3Rt c5zxfe57Zf0XU8yEgPwQRYSG7N5TZXOQs1o3+ysPWnK6/eJLkbPgfvnfgW/AoYkRd7sX1/ERQvp U9GC/vd4aP3cxuaJhEejAxGGp7r1uNw== X-Received: by 2002:a17:907:1dea:b0:a18:869e:21e7 with SMTP id og42-20020a1709071dea00b00a18869e21e7mr1177286ejc.1.1701433909768; Fri, 01 Dec 2023 04:31:49 -0800 (PST) X-Google-Smtp-Source: AGHT+IE8Yuq96lZvj8o7PLUfhJMbgvu6aVzVvsTqWGQMmJWlhVXdakE9D+7w5kGfKjpERaSepCQMUw== X-Received: by 2002:a17:907:1dea:b0:a18:869e:21e7 with SMTP id og42-20020a1709071dea00b00a18869e21e7mr1177274ejc.1.1701433909384; Fri, 01 Dec 2023 04:31:49 -0800 (PST) Received: from [192.168.0.129] (ip-94-112-227-180.bb.vodafone.cz. [94.112.227.180]) by smtp.gmail.com with ESMTPSA id ks19-20020a170906f85300b00a11b2677acbsm1826664ejb.163.2023.12.01.04.31.48 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 01 Dec 2023 04:31:48 -0800 (PST) Message-ID: Date: Fri, 1 Dec 2023 13:31:47 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH v5] gdb/testsuite: fix completion tests when using READ1 To: Andrew Burgess , gdb-patches@sourceware.org References: <20231108165650.1224706-1-blarsen@redhat.com> <20231122094415.3150293-2-blarsen@redhat.com> <87il5koa39.fsf@redhat.com> From: Guinevere Larsen In-Reply-To: <87il5koa39.fsf@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_NUMSUBJECT,NICE_REPLY_A,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 29/11/2023 16:25, Andrew Burgess wrote: > Guinevere Larsen writes: > >> The commit a3da2e7e550c4fe79128b5e532dbb90df4d4f418 has introduced >> regressions when testing using the READ1 mechanism. The reason for that >> is the new failure path in proc test_gdb_complete_tab_unique, which >> looks for GDB suggesting more than what the test inputted, but not the >> correct answer, followed by a white space. Consider the following case: >> >> int foo(int bar, int baz); >> >> Sending the command "break foo" to GDB will return >> >> break foo(int, int) >> >> which easily fits the buffer in normal testing, so everything works, but >> when reading one character at a time, the test will find the partial >> "break foo(int, " and assume that there was a mistake, so we get a >> spurious FAIL. >> >> That change was added because we wanted to avoid forcing a completion >> failure to fail through timeout, which it had to do because there is no >> way to verify that the output is done, mostly because when I was trying >> to solve a different problem I kept getting reading errors and testing >> completion was frustrating. >> >> This commit implements a better way to avoid that frustration, by first >> testing gdb's complete command and only if that passes we will test tab >> completion. The difference is that when testing with the complete >> command, we can tell when the output is over when we receive the GDB >> prompt again, so we don't need to rely on timeouts. With this, the >> change to test_gdb_complete_tab_unique has been removed as that test >> will only be run and fail in the very unlikely scenario that tab >> completion is different than command completion. >> --- >> gdb/testsuite/lib/completion-support.exp | 37 ++++++++++++++++-------- >> 1 file changed, 25 insertions(+), 12 deletions(-) >> >> diff --git a/gdb/testsuite/lib/completion-support.exp b/gdb/testsuite/lib/completion-support.exp >> index fdc512838c3..16598aa5a6c 100644 >> --- a/gdb/testsuite/lib/completion-support.exp >> +++ b/gdb/testsuite/lib/completion-support.exp >> @@ -111,15 +111,11 @@ 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 >> @@ -190,21 +186,29 @@ proc test_gdb_complete_cmd_none { line } { >> >> # Test that completing LINE with the complete command completes to >> # COMPLETE_LINE_RE. >> +# Returns 1 if the test passed, 0 if it failed, -1 if it timed out. >> >> proc test_gdb_complete_cmd_unique { input_line complete_line_re } { >> global gdb_prompt >> >> + set res 0 >> set cmd "complete $input_line" >> set cmd_re [string_to_regexp $cmd] >> set test "cmd complete \"$input_line\"" >> gdb_test_multiple $cmd $test { >> -re "^$cmd_re\r\n$complete_line_re\r\n$gdb_prompt $" { >> pass $test >> + set res 1 >> } >> -re "$gdb_prompt $" { >> fail "$test" >> } >> + timeout { >> + fail "$test (timeout)" >> + set res -1 >> + } >> } >> + return $res >> } >> >> # Test that completing "CMD_PREFIX + COMPLETION_WORD" with the >> @@ -263,12 +267,6 @@ proc test_gdb_complete_none { input_line } { >> >> proc test_gdb_complete_unique_re { input_line complete_line_re {append_char " "} {max_completions 0}} { >> set append_char_re [string_to_regexp $append_char] >> - if { [readline_is_used] } { >> - if { [test_gdb_complete_tab_unique $input_line $complete_line_re \ >> - $append_char_re] == -1 } { >> - return -1 >> - } >> - } >> >> # Trim COMPLETE LINE, for the case we're completing a command with leading >> # whitespace. Leading command whitespace is discarded by GDB. >> @@ -288,8 +286,23 @@ proc test_gdb_complete_unique_re { input_line complete_line_re {append_char " "} >> "\r\n$input_line_re $max_completion_reached_msg_re" >> } >> >> - test_gdb_complete_cmd_unique $input_line $expected_output_re >> - return 1 >> + # First test completion with the command, then with tab. >> + # It is done in this order because cmd_complete knows when the output >> + # from GDB is over, so it can fail without requiring a timeout, which >> + # speeds up testing if necessary. >> + >> + set test_result [test_gdb_complete_cmd_unique $input_line\ >> + $expected_output_re] >> + if { $test_result != 1 } { >> + return $test_result >> + } >> + >> + if { [readline_is_used] } { >> + set test_result [test_gdb_complete_tab_unique $input_line \ >> + $complete_line_re $append_char_re] >> + } >> + } > You've added two '}' here instead of one. The first pairs with: > > if { [readline_is_used] } { > > while the second actually closes the function. As a result... > >> + return $test_result > ... this is interpreted at file scope. > > I tested this patch with: > > make check-gdb TESTS="gdb.*/*complete*.exp" > > and you'll notice a few tests failing as a result. oops, sorry about forgetting to rerun the tests. I fixed what you mentioned, and ran the test (though I used gdb.*/*complet*.exp to also get "completion" stuff) and I only get the ada failure that wasn't introduced by my patch. Thanks for the review, I've pushed this patch! -- Cheers, Guinevere Larsen She/Her/Hers > > However, I think the rest of this patch is fine. If you fix this issue > and run at least the set of tests I gave above with no problems, then I > think you can go ahead and check this in without reposting. > > Approved-By: Andrew Burgess > > FYI: When using read1 I still see some timeouts with the test > gdb.ada/complete.exp, but these exist even without this patch. I have a > fix for this, which I'll post soon. > > Thanks, > Andrew > >> } >> >> # Like TEST_GDB_COMPLETE_UNIQUE_RE, but COMPLETE_LINE is a string, not >> -- >> 2.41.0