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 62DBF38560BA for ; Fri, 24 Jun 2022 18:41:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 62DBF38560BA 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 g18so4214253wrb.10 for ; Fri, 24 Jun 2022 11:41:55 -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:to:references:from:in-reply-to :content-transfer-encoding; bh=WS+XldRXGI49bnZ6Q9HWgdMpKXmvuVGOYVV1fiBXn8M=; b=au/ETnH9wvoPgY7afYp7tqEL6BKXlCeOQeWahGtMiytwlu2Th6y3CClBCkdU2K3k5K W0oVDttSvBk/AwiF4xRRh8agwuyKMmDvUJpJ8rZi05mjweO3bZHukEvOCJngUD2VYLaw DaHFEGacLD5uuT7l71Am3rF+XumNpA1Wtbj3z8t9n7tjocemRSstZB/B3lvtjaa2VJ60 kBNROpQKmFcxmoI0lT/Ckpeyy+vtdxBS1FoxYq4Torvsr99HLe4K6TKmsBlMUlSSeTIJ OtCN1YgTCPd0Cr3vlqtsDxYll75MJKPtKVH8surnD2wkSGJjMBdVWFC4W1f+SMpBQx6I u66Q== X-Gm-Message-State: AJIora8HhOXcDMMH8QIOos/xt2zPxeruBBIfHiF504hNwtn8s3fKPIIi rEABKd5pMAAyXoEivhRba+dU4JHauN0= X-Google-Smtp-Source: AGRyM1s0kCoIA/SVoDFDI/puUpoC/Vl2A3KDmfIsNVbNcFd53za8PczxGt5h4DMAu7NwgBRVG8mZXA== X-Received: by 2002:a05:6000:1446:b0:218:5a5d:6cb5 with SMTP id v6-20020a056000144600b002185a5d6cb5mr457249wrx.629.1656096114272; Fri, 24 Jun 2022 11:41:54 -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 r3-20020adff703000000b0021b9cfca781sm3461180wrp.45.2022.06.24.11.41.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 24 Jun 2022 11:41:52 -0700 (PDT) Message-ID: <716c08f6-d4ff-094b-013e-b2dc1ef17546@palves.net> Date: Fri, 24 Jun 2022 19:41:52 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 Subject: Re: [PATCH] gdb/testsuite: modernize gdb.base/maint.exp Content-Language: en-US To: Keith Seitz , Andrew Burgess , Bruno Larsen , gdb-patches@sourceware.org References: <20220509180431.31032-1-blarsen@redhat.com> <87edzic8v1.fsf@redhat.com> <2d43e796-7228-f69a-c15b-5463653fe07b@palves.net> From: Pedro Alves In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.6 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=no 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: Fri, 24 Jun 2022 18:41:57 -0000 Hi Keith, On 2022-06-24 18:51, Keith Seitz wrote: > On 6/24/22 08:22, Pedro Alves wrote: >> On 2022-06-21 16:52, Andrew Burgess via Gdb-patches wrote: >>>> +                pass "$test_name 1" >>> You could use: >>> >>>      pass "$gdb_test_name, pattern 1" >>> >>> here, and similar, with ', pattern 2' for the next 'pass' call. >>> >> >> How about >> >>     pass "$gdb_test_name (pattern 1)" >> >>     pass "$gdb_test_name (pattern 2)" >> >> ? >> >> The idea being that the text in the trail parens is not considered part of the >> test name, so when comparing gdb.sum files and matching test names, that parens part >> should be discarded.  Whether this test passes with pattern 1 or 2 should make >> no difference IIUC, thus I think it should not be part of the (part that counts >> as real) test name. >> > I think it no surprise that I disdain this " (whatever)" idiom in test names. There > is also a long-standing guideline in the wiki against this: > > https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages > > Has there been some unannounced, sekrit change to this policy? There has not. As mentioned in the wiki, the idea of not putting trailing " (foo)" in the test names, is that timeouts, xfail, kfail, internal error, etc. info is appended to the test name in gdb.sum. So this means that tools that compare test results and track regressions/progressions at the individual test level should consider these the same test: PASS: check foo thing FAIL: check foo thing (timeout) FAIL: check foo thing (GDB internal error) KFAIL: check foo thing (PRMS: gdb/99999) (etc.) So what you shouldn't do is use trailing parens to de-DUPLICATE test names. Like, this is wrong: gdb_test "foo cmd" " = 1" "foo cmd (1)" gdb_test "foo cmd" " = 2" "foo cmd (2)" because the test analysis tooling should consider that those two tests have the same name, thus, they are duplicates. The duplication-detection machinery we have today directly in the testsuite doesn't flag that, but I think it should. You should thus write instead something like: gdb_test "foo cmd" " = 1" "foo cmd, 1st" gdb_test "foo cmd" " = 2" "foo cmd, 2nd" Now, the case at hand is different -- we have a single test, with multiple regexps, like: gdb_test_multiple "foo cmd" "some test name" { -re -wrap "pattern 1" { # machine/run 1 hits this one. pass "$gdb_test_name - pattern 1" } -re -wrap "pattern 2" { # machine/run 2 hits this one. pass "$gdb_test_name - pattern 2" } } I think that's wrong, because depending on whatever pattern you get, you get a different test name, making it harder to automatically compare test results. Depending on machines or testsuite runs, you get one of these: PASS: some test name - pattern 1 PASS: some test name - pattern 2 FAIL: some test name (timeout) FAIL: some test name (GDB internal error) By making it instead: gdb_test_multiple "foo cmd" "some test name" { -re -wrap "pattern 1" { # machine 1 hits this one. pass "$gdb_test_name (pattern 1)" } -re -wrap "pattern 2" { # machine 2 hits this one. pass "$gdb_test_name (pattern 2)" } } and given the rule that trailing (foo) doesn't count, all the pass calls above, and the pass/fail calls inside gdb_test_multiple have the same test message. It's effectively the same as: gdb_test_multiple "foo cmd" "some test name" { -re -wrap "pattern 1" { # machine 1 hits this one. pass $gdb_test_name } -re -wrap "pattern 2" { # machine 2 hits this one. pass $gdb_test_name } } An alternative is to really write "pass $gdb_test_name", and precede it with 'verbose -log "got pattern 1"' or some such. I'm fine with that, but given the fact that tooling should already be ignoring the trailing parens, I don't think we need to stop using trailing parens in scenarios like these. > > How is, e.g., "- pattern 1" any less desirable/informative than "(pattern 1)"? I hope I've clarified it above.