From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 55C9C383943C for ; Wed, 18 May 2022 14:49:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 55C9C383943C Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id BFFD321B4D; Wed, 18 May 2022 14:49:57 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 9C5C7133F5; Wed, 18 May 2022 14:49:57 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id GTXQJJUHhWJYOQAAMHmgww (envelope-from ); Wed, 18 May 2022 14:49:57 +0000 Message-ID: <2a77687e-9817-b048-2a2a-be6b91737ad3@suse.de> Date: Wed, 18 May 2022 16:49:57 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0 Subject: Re: [pushed] Support -prompt and -lbl in gdb_test (Re: [PATCH 5/5] Make gdb_test's question non-optional if specified) Content-Language: en-US To: Pedro Alves , Tom Tromey Cc: gdb-patches@sourceware.org References: <20220330192929.3161015-1-pedro@palves.net> <20220330192929.3161015-6-pedro@palves.net> <87o7zxeaa6.fsf@tromey.com> <87sfp7kc78.fsf@tromey.com> <8def622c-5396-d1e1-dfbb-a5732dd8183a@palves.net> From: Tom de Vries In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-6.2 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, SPF_HELO_NONE, SPF_PASS, 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 May 2022 14:50:00 -0000 On 5/18/22 16:13, Pedro Alves wrote: > On 2022-05-18 13:36, Pedro Alves wrote: >> On 2022-05-18 13:15, Tom de Vries wrote: >>> On 5/18/22 13:01, Pedro Alves wrote: >>>> -    if [llength $args]>2 then { >>>> -    set message [lindex $args 2] >>>> -    } else { >>>> -    set message [lindex $args 0] >>>> +    if { $message == "" } { >>>> +    set message $command >>>>       } >>> >>> This seems to cause: >>> ... >>> PATH: gdb.ada/exec_changed.exp: shell mv /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/first /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common >>> PATH: gdb.ada/exec_changed.exp: shell mv /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/first >>> PATH: gdb.ada/exec_changed.exp: shell mv /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/second /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common >>> PATH: gdb.ada/exec_changed.exp: shell touch /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/common >>> PATH: gdb.ada/exec_changed.exp: shell touch /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.ada/exec_changed/first >>> ... >>> because the interpretation of message "" changed: >>> ... >>> gdb_test "shell mv ${binfile} ${common_binfile}" ".*" "" >>> ... >> >> Hmm... Yeah... We could revert most of the changes to gdb_test and just keep the parse_args >> part. However, IMO the old behavior is a misfeature, though. I think tests should >> always have a name. E.g., if such a test hits an internal error, what message would >> be used? >> >> The documentation of the option even says that if message is omitted, use the command >> string as message: >> >> # MESSAGE is an optional message to be printed. If this is >> # omitted, then the pass/fail messages use the command string as the >> # message. (If this is the empty string, then sometimes we don't >> # call pass or fail at all; I don't understand this at all.) >> >> Also, gdb_test_multiple doesn't a distinction between explicit "" message and >> not specified message, the only way to end up with an empty message is if command >> is empty as well. So AFAICS, this change (inadvertently) made gdb_test and >> gdb_test_multiple behave the same in this respect. >> >> So how about we just fix the affected gdb_test invocations? > > So I'm diffing testruns from before the patch vs after, and I think the vast > majority of cases that weren't issuing a pass should do so. With the new > messages, we now get a ton of DUPLICATEs, which I'm fixing. > > However, there are a few spots here and there where we really would prefer to > not issue a pass, such as when we're testing something in a loop, and we don't > know how many iterations there will be. > Ack. We could do something fancy like this, which would mean not having to update the msg or rewrite into gdb_test_multiple ... message_squash { foreach var [seq 1 20] { gdb_test } } ... and get: ... PASS: (20x) ... but perhaps that'll just confusion. > Instead of going back to how it used to be, I'm thinking of adding a new > option to gdb_test, "gdb_test -nopass". The advantage of this approach is > that we always have a message for the FAIL case this way, and, it's more > explicit. We could fix the "no message for the FAIL case" in the old implementation > by delaying defaulting message to the command until after the pass case is added > to user_code. But the explict option still seems better to me, as it let's you > specify a message different from the command, and only print it on FAIL. The other > approach would force the message to be the same as the command. Makes sense to me. Thanks, - Tom