From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id TGPXLXuaP2FaLgAAWB0awg (envelope-from ) for ; Mon, 13 Sep 2021 14:37:47 -0400 Received: by simark.ca (Postfix, from userid 112) id AB1951EE25; Mon, 13 Sep 2021 14:37:47 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,NICE_REPLY_A,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 985291EDDB for ; Mon, 13 Sep 2021 14:37:46 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E73B43858437 for ; Mon, 13 Sep 2021 18:37:45 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E73B43858437 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1631558265; bh=7oyoxJrVu9mk9V0G7JnGYP7G/gxbHXPrDk9Fku9C8So=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=TDZwYWsUAoT6HH0aHeVGXj400x9hPsuiCJddeFQVayJis3gVVcXSYNTzw8Jgq4rx1 rWSIwf+X9Dn4Q+ToDfa2uKv0ptbxE7+Q9RUmj5Qg/+c+JYO96cvTS8QDRGNynX1Z7C tSroWe074f9oWhwnPRU62hMlqhbmhaLLOnCod5SM= Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id F10BF3858402 for ; Mon, 13 Sep 2021 18:37:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org F10BF3858402 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 3081821FA1; Mon, 13 Sep 2021 18:37:26 +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 1542213B69; Mon, 13 Sep 2021 18:37:26 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id CwkbBGaaP2GsZgAAMHmgww (envelope-from ); Mon, 13 Sep 2021 18:37:26 +0000 Subject: Re: [PATCH][gdb/testsuite] Check for valid test name To: Andrew Burgess References: <20210913071908.GA3469@delia.home> <20210913142627.GA61506@embecosm.com> Message-ID: <729a2c1e-1361-0a8a-6cee-646a9d17d991@suse.de> Date: Mon, 13 Sep 2021 20:37:25 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <20210913142627.GA61506@embecosm.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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: , From: Tom de Vries via Gdb-patches Reply-To: Tom de Vries Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 9/13/21 4:26 PM, Andrew Burgess wrote: > * Tom de Vries via Gdb-patches [2021-09-13 09:19:11 +0200]: > >> Hi, >> >> When running gdb.base/batch-exit-status.exp I noticed that the test name >> contains a newline: >> ... >> PASS: gdb.base/batch-exit-status.exp: : No such file or directory\.^M >> : No such file or directory\.: [lindex $result 2] == 0 >> ... >> >> Check for this in ::CheckTestNames::check, such that we have a warning: >> ... >> PASS: gdb.base/batch-exit-status.exp: : No such file or directory\.^M >> : No such file or directory\.: [lindex $result 2] == 0 >> WARNING: Newline in test name >> ... >> >> Tested on x86_64-linux. >> >> Any comments? >> >> Thanks, >> - Tom >> >> [gdb/testsuite] Check for valid test name >> >> --- >> gdb/testsuite/lib/check-test-names.exp | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/gdb/testsuite/lib/check-test-names.exp b/gdb/testsuite/lib/check-test-names.exp >> index 1de7624d5c5..db76abab990 100644 >> --- a/gdb/testsuite/lib/check-test-names.exp >> +++ b/gdb/testsuite/lib/check-test-names.exp >> @@ -93,6 +93,13 @@ namespace eval ::CheckTestNames { >> return $message >> } >> >> + # Check if MESSAGE is a well-formed test name. >> + proc _check_name { message } { > > I think `_check_newline` would be a better name. > >> + if { [regexp \n $message]} { >> + warning "Newline in test name" >> + } > Hi Andrew, thanks for the review. What I tried to do here is pick a proc name that: - describes what the current implementation is doing, but - is still broad enough to not only describe what the implementation is doing, such that other checks could be added to it without having to change the name of the proc. The name _check_newline fails to achieve the latter goal. So, I've renamed to "_check_well_formed_name", perhaps that's more clear than '_check_name'. > If I run the entire testsuite I guess these warnings will likely get > lost in the output, unlike the DUPLICATE/PATH warnings, which are > listed along with the other PASS/FAIL/etc results. > We can trigger the warning like this: ... diff --git a/gdb/testsuite/gdb.base/batch-exit-status.exp b/gdb/testsuite/gdb.base/batch-ex it-status.exp index 520083f7989..a55e92acf5f 100644 --- a/gdb/testsuite/gdb.base/batch-exit-status.exp +++ b/gdb/testsuite/gdb.base/batch-exit-status.exp @@ -94,3 +94,5 @@ set no_such_re ": $test\\." test_exit_status 1 "-batch \"\"" "1x: $test" ^[multi_line $no_such_re ""]$ test_exit_status 1 "-batch \"\" \"\"" "2x: $test" \ ^[multi_line $no_such_re $no_such_re ""]$ + +pass "bla\nbla" ... In the output I see: ... Running batch-exit-status.exp ... WARNING: Newline in test name ... In the gdb.sum, I see: ... PASS: gdb.base/batch-exit-status.exp: bla bla WARNING: Newline in test name ... And in the gdb.log, I see: ... PASS: gdb.base/batch-exit-status.exp: bla bla WARNING: Newline in test name ... So AFAIU these warnings will not get lost in the output. > Did you check to see if this warning occurs any other times in the > testsuite right now? I guess if we're clean right now then having > this as a warning is fine, as we should spot this when adding new > tests. > Yes, AFAIK we're clean, I'll do a retest and verify that, and then commit. Thanks, - Tom > I'm happy with this though (with the name change above). > > Thanks, > Andrew > > >> + } >> + >> # Check if MESSAGE contains either the source path or the build path. >> # This will result in test names that can't easily be compared between >> # different runs of GDB. >> @@ -110,6 +117,8 @@ namespace eval ::CheckTestNames { >> if [ _check_duplicates $message ] { >> clone_output "DUPLICATE: $message" >> } >> + >> + _check_name $message >> } >> >> # If COUNT is greater than zero, disply PREFIX followed by COUNT.