From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) by sourceware.org (Postfix) with ESMTPS id E8B2A3858434 for ; Wed, 18 May 2022 14:13:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E8B2A3858434 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-f51.google.com with SMTP id r30so2878651wra.13 for ; Wed, 18 May 2022 07:13:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:from:to:cc:references:in-reply-to :content-transfer-encoding; bh=VQzNKLhHKJ9wH5SPJ8jomGqi7RfwJ4uaAKrHngHyxws=; b=tQ4+wS2YuGxlk2fXXs9ROfyzOxBr+M9jv2a/nyRnhQawoUXls4EoD9n8qrFabrdU2J 1qnvY/dZFocwB2ebiMOPHVdW6zXn8L6kS28OYQ5NlXUAjC2nA0qQIYXz4l3B5O28VdIQ apCdhQuEEl+703gR5gJ68i75zS0kPNAWBs16+OQHZRZf5NtC9OocL8GgLxa3rZrfWYvT EnhVdv434pnh0RQXUguYvqLrlb88RXLl1YbwcKjY1x+yJPHRqTQvExuFF3cQN7tE8c49 ZsKt1QORU20NFXvgoELqkT8RkJIFP1jp1waeG664BO88IN9yjBDCNTElA2iPPIBIlmWO JoPQ== X-Gm-Message-State: AOAM533+dM5H+/ATtEUFLFFRzYL10rHBwWHFNcdm6VLGme2Fc6Hc4P3c 2JOzvg2G8jusupT60Is0nFQ= X-Google-Smtp-Source: ABdhPJzBWTI2MEAvx9XrJ6XRd8A1eY0gFcgvgUpqHoKfPA7IiaFHwZzz/Js3z1eFjUU2IIS6lGYOvQ== X-Received: by 2002:adf:bc47:0:b0:20d:2de:ff83 with SMTP id a7-20020adfbc47000000b0020d02deff83mr16122731wrh.535.1652883205672; Wed, 18 May 2022 07:13:25 -0700 (PDT) Received: from ?IPV6:2001:8a0:f924:2600:209d:85e2:409e:8726? ([2001:8a0:f924:2600:209d:85e2:409e:8726]) by smtp.gmail.com with ESMTPSA id d17-20020a05600c34d100b00397259c2a64sm691497wmq.38.2022.05.18.07.13.24 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 18 May 2022 07:13:24 -0700 (PDT) Message-ID: Date: Wed, 18 May 2022 15:13:23 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.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 From: Pedro Alves To: Tom de Vries , 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> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00, BODY_8BITS, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, 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:13:29 -0000 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. 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.