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 E43DF3858C41 for ; Tue, 14 Nov 2023 10:40:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E43DF3858C41 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 E43DF3858C41 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=1699958415; cv=none; b=cbC3c4CS0PbuQwSWgyvkR/rkOGSeEWap/R2czMXZASR/+lacA6JEHL92br0dZwD96g3bMMUlGloHWD0CuJ/upJ5u3zOOqDhEMndRdadKZ6KH79FG3yrsIM11ze1h/n99jpqvaGVbgvOUHfcF1eydvJAEbTuyu5MoIPZiQ4Ja4eA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699958415; c=relaxed/simple; bh=wv7Qr0ShU9GFpTQifKEZ6LRwk2Qtd6qCTSlBXSHT8Zk=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=f5YihxhNvWkSunBdfJzs7I1F8GpqO0ROEJl+3YdXwD7OfEosP0BKuIfi5nElpLMaSBmV6iIm/9GuYFx6sVRtzqLUasb9gnFCR63EzGrZChg0PJmJBVz60axbzXOlOyDT70YJ/+NTjKuQsAMOvz3tKHCyNmVNvudB5D6/jhnaD34= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1699958413; 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=CPZZw7LaksdEi5LQVRd3S2NRTWbpoGoR7cnLqX/2XjQ=; b=O251tzegehewBrVCV+dy74a46MVzQA/4jInREM/j+FuTZ5bEc3foZTw0v9AYqPYfF9zC9f BHNQdPxZAw/I1AVssnIUCs9lnySVEXJh9u1LHOzD461T8KVCvMZbEjYiUiPXhDLYosLEqH m9+cK8y4kagxqTfZxH/vxYx+AAb4Pow= Received: from mail-ej1-f72.google.com (mail-ej1-f72.google.com [209.85.218.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-455-zbe7EWMdMnWP2lZJ05_yKQ-1; Tue, 14 Nov 2023 05:40:12 -0500 X-MC-Unique: zbe7EWMdMnWP2lZJ05_yKQ-1 Received: by mail-ej1-f72.google.com with SMTP id a640c23a62f3a-9e5dd91b0ebso262589966b.1 for ; Tue, 14 Nov 2023 02:40:11 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699958410; x=1700563210; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=CPZZw7LaksdEi5LQVRd3S2NRTWbpoGoR7cnLqX/2XjQ=; b=AKGFshwPDXQ6WG2zex+MQjf/gsStu/24iBMB5zuBwVLmKRhsvjZlApSvxOl9H8D351 ttFsvMWaLDEaVMbTxxpQ1VQcY8LzfxShW/z+sYq8BUL4zLUa8yhbUwQ07DGKRNF0vG7+ ucDD335SfEzhc/c8hmUYb8ljeqQf+JqEc11417eTD/zvbSr1LUfNFHIeogqXPKpuPMPA OcS5CylhVMbO+mSQHhMFOj870W2Khk00U96JDIcPL+5cVdINUwbCM4n+S1XLs7IDl3hG 8Y7OdKRkcN/AwVQw0ktmz0NWRbuDHt+38rbL98G9/wIdWrnXgrVDmJfBWjLNCm4P4cey DUrg== X-Gm-Message-State: AOJu0Yw5rhOYVAQVZhKDZ19MVrthgii1KztFu/g9cHW8ylON5QL1jC0Y MDFWpEg7sq5oaCF4+3kBRUU5mdJK2dytHt6L0HBhsoImcCdeqJN7fJ1wmWBf88pQpEhL9Q6SKqS cX54Mza6l+hnxFgE0x9K+3g== X-Received: by 2002:a17:906:b49:b0:9c4:4b20:44a1 with SMTP id v9-20020a1709060b4900b009c44b2044a1mr7051931ejg.65.1699958410744; Tue, 14 Nov 2023 02:40:10 -0800 (PST) X-Google-Smtp-Source: AGHT+IFOZb1NavoW994T6woV3N4WJRy/v28civjAiYOSnVfPok2NfMeEorKGCWJ3WYhU1sqO5lvIDw== X-Received: by 2002:a17:906:b49:b0:9c4:4b20:44a1 with SMTP id v9-20020a1709060b4900b009c44b2044a1mr7051917ejg.65.1699958410342; Tue, 14 Nov 2023 02:40:10 -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 dk18-20020a170906f0d200b009c921a8aae2sm5293679ejb.7.2023.11.14.02.40.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 14 Nov 2023 02:40:09 -0800 (PST) Message-ID: Date: Tue, 14 Nov 2023 11:40:08 +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 v4] gdb/testsuite: fix completion tests when using READ1 To: Andrew Burgess , gdb-patches@sourceware.org References: <20230828113026.76323-1-blarsen@redhat.com> <20231108165650.1224706-1-blarsen@redhat.com> <87cywdsxiz.fsf@redhat.com> From: Guinevere Larsen In-Reply-To: <87cywdsxiz.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=-11.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 13/11/2023 12:28, 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 | 32 ++++++++++++++++-------- >> 1 file changed, 21 insertions(+), 11 deletions(-) >> >> diff --git a/gdb/testsuite/lib/completion-support.exp b/gdb/testsuite/lib/completion-support.exp >> index fdc512838c3..cc34ef51e22 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,26 @@ 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 -1 >> 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" >> + set res 0 >> } > If you want to return -1 for timeout, then I would follow the pattern of > how test_gdb_complete_tab_unique does it. Inside the gdb_test_multiple, > add a new block: > > timeout { > fail "$test (timeout)" > set res -1 > } > > Then you can initialise 'res' to 0 instead of -1, delete the 'set res 0' > for the existing fail case, and now, if any of the additional failure > cases that are added by calling gdb_test_multiple trigger, you'll get a > return value of 0. For the one passing case you'll get 1, and for the > one timeout case you'll get -1. That's a good point. I always forget that there are more ways that gdb_test_multiple can fail. I'll update this! > >> } >> + return $res >> } >> >> # Test that completing "CMD_PREFIX + COMPLETION_WORD" with the >> @@ -263,12 +264,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,7 +283,22 @@ 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 >> + # 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] >> + } > This seems wrong. You probably mean: > > if { [readline_is_used] } { > if { [test_gdb_complete_tab_unique $input_line \ > $complete_line_re $append_char_re] == -1 } { > return -1 > } > } > > Otherwise, if test_gdb_complete_tab_unique times out -- the only timeout > we really care about -- you'll not return -1. No we won't. if test_gdb_complete_cmd_unique returned -1, the previous if condition will cause the function to exit early. This is expected to always return 1, unless there is a difference in the results between tab and cmd completion. Also, since I'm removing the partial completion part, the only way for that function to return 0 is if it hits one of the default failures from gdb_test_multiple, so I would think that the rare cases where 0 is returned should be captured too. Otherwise we might see an internal error and still return that the test was successful. -- Cheers, Guinevere Larsen She/Her/Hers > > Thanks, > Andrew > >> + } >> return 1 >> } >> >> -- >> 2.41.0