From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) by sourceware.org (Postfix) with ESMTPS id 5834638930C3 for ; Wed, 29 Apr 2020 15:38:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5834638930C3 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wm1-x32b.google.com with SMTP id g12so2526941wmh.3 for ; Wed, 29 Apr 2020 08:38:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=xkxNTbchyMH52Sm3ONPQrwXnCt9GVKNLJ5RhysnalZs=; b=VsikBzJQikPqo2cn9ouns+cljLdJTlaOrulLDZuQLHOe0/miZ2IOLFE5nLcBUFOQ64 oQljgPnkM5r25U+95o93Ti2Dk1MKO/Z+7sAFq/TkROMCdULHU/EVk9FFdm84M9kwegJc xfko6ipuxXNYy31rYJUDZH10F8fVEfJcofdqqxJzYzxfAuiGe2aWZrIpDZeDHoJT5eGX 7ioK3f8zwFFyC/bF91h8K2/nZ2jKvjtB/32FDn4kLKGRWujAapQ5YfmCShsPnfY6t2r7 hNuUxxkiEAvZMfiOZFtJecACUuoenJ7gcXPLrRxy5pzX+KRK/U/2qj7A5JVtdpylIVO3 4v1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=xkxNTbchyMH52Sm3ONPQrwXnCt9GVKNLJ5RhysnalZs=; b=pM/awv0tB2MorujoJXvhsrdiLUk338LWbXb+D19iuyKLUV0wYLMqdBfBgSWdsl9xbF gDPEqeAwtGJNjrngnXZs+9ZVO+LdnsRZ6QeS+WVbp9RnUA5r9Eivhd/fhqegfHW8MrWV dDp67OtwSqcfs77a1lyZQWCBJat5L2LJoAW8vxmb0SK2onN5I4dvIXZJkd5AR1WFcp4T zcctRe/qEc5eRbPZb49Pa9KAYnEwfR/3xK6C7pNLQt2r5zjxJRlwMTQJbrNw1DTTsoaU HESOQjvNE1iuTrLdqrlhmUxCKn6Fh77a0MYni1KSVa1Kr/Ei5VASXHucQ5S0vhLFlHGp 78CQ== X-Gm-Message-State: AGi0PuYmiTY0CqnLuky86GvD8dxzz+Ge0TydoL3w2EQ0XdNjE/k1JhTz hCiXaYpHVD+MPeHjd5Mh2YLt0DsyJOo= X-Google-Smtp-Source: APiQypLBnlFeNflWDIxXIk378ZL3JUR9jvwJXE/DNziX407mJg57WVPSV3BLKP+Q6oodri6SuScPGw== X-Received: by 2002:a05:600c:2a52:: with SMTP id x18mr3780712wme.37.1588174734311; Wed, 29 Apr 2020 08:38:54 -0700 (PDT) Received: from localhost (host81-151-181-184.range81-151.btcentralplus.com. [81.151.181.184]) by smtp.gmail.com with ESMTPSA id m8sm32170417wrx.54.2020.04.29.08.38.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Apr 2020 08:38:53 -0700 (PDT) Date: Wed, 29 Apr 2020 16:38:52 +0100 From: Andrew Burgess To: Simon Marchi Cc: Keith Seitz , gdb-patches@sourceware.org Subject: Re: [PATCHv2 0/3] Automatic detection of test name problems Message-ID: <20200429153852.GL3522@embecosm.com> References: <290d0de3-264b-9a60-487f-ff6b484c35f1@redhat.com> <20200429090216.GI3522@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: Linux/5.5.17-200.fc31.x86_64 (x86_64) X-Uptime: 16:31:57 up 9 days, 1:05, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-24.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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, 29 Apr 2020 15:38:57 -0000 * Simon Marchi [2020-04-29 11:04:40 -0400]: > On 2020-04-29 5:02 a.m., Andrew Burgess wrote: > > * Keith Seitz [2020-04-28 12:08:25 -0700]: > > > >> On 4/27/20 3:01 PM, Andrew Burgess wrote: > >>> Changes since v1: > >>> > >>> 1. Original patch #1 is now merged. > >>> > >>> 2. Functionality is now placed inside a namespace. > >>> > >>> 3. Better counting for multi-variant test runs, this is inline with > >>> how Dejagnu's muti-variant result counting works. > >>> > >>> 4. Use 'string first' instead of 'regexp'. > >>> > >>> 5. Reworded commit message on what is now patch #2. > >>> > >>> Further feedback welcome. > >> > >> Wow, that's almost a complete rewrite! While not what I was really > >> suggesting, I have to admit, a /big/ smile came to my face while > >> reading this. It is so nicely done! > >> > >> Thank you! > >> > >> Keith > >> > >> PS. As you know, IANAM, but you are, so I encourage you to approve > >> your patch. ;-) > > > > Thanks for your feedback, especially your TCL suggestions. > > > > I'll let the patch sit for a while to see if anyone else has any > > input. > > > > Thanks, > > Andrew > > > > Hi Andrew, > > Thanks for doing this, it's nice. And by testing this patchset, I've found an offender > and sent a patch for it :) > > https://sourceware.org/pipermail/gdb-patches/2020-April/168052.html > > When you detect an offender, do you think you could print something? A bit like a "FAIL" > is printed when a test fails. For example: > > DUPLICATE: gdb.base/break.exp: set convenience variable $foo to 81.5 > > That would make it easier to spot the problematic test(s). But even without that, your > patchset looks good and useful to me. Ask and you shall receive! Below is a patch that applies on top of the existing series just to try. If you like this I'll fold this back into the relevant patches. We now print "PATH: ...." or "DUPLICATE: ...." when we see an offending test. On printing DUPLICATE, things are a little .... odd. I currently count the number of unique duplicates, so: PASS: foo PASS: foo PASS: foo PASS: bar PASS: bar PASS: bar Will give a duplicate count of '2', that is 'foo' and 'bar' are duplicated. However, I chose to print "DUPLICATE: ...." for _every_ duplicate, so you would get this: PASS: foo PASS: foo DUPLICATE: foo PASS: foo DUPLICATE: foo PASS: bar PASS: bar DUPLICATE: bar PASS: bar DUPLICATE: bar Obviously we don't get a DUPLICATE message for the first 'foo', or the first 'bar', we can't know by that point that these are going to be duplicated. But, it might be confusing that this test will report 2 duplicates, but contain 4 DUPLICATE lines. Would it in fact be better to report a count of 4 in this case I wonder? Thoughts welcome. Thanks, Andrew --- diff --git a/gdb/testsuite/lib/check-test-names.exp b/gdb/testsuite/lib/check-test-names.exp index 15c5544c8e4..8b525897ba3 100644 --- a/gdb/testsuite/lib/check-test-names.exp +++ b/gdb/testsuite/lib/check-test-names.exp @@ -43,24 +43,27 @@ namespace eval ::CheckTestNames { incr counts($type,total) } - # 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. - # - # Any offending paths in message cause PATHS_IN_TEST_NAMES to be - # incremented. - proc check { message } { + # Check if MESSAGE contains a build or source path, if it does increment + # the relevant counter and return 1, otherwise, return 0. + proc _check_paths { message } { global srcdir objdir - variable all_test_names foreach path [list $srcdir $objdir] { if { [ string first $path $message ] >= 0 } { # Count each test just once. inc_count paths - return + return 1 } } + return 0 + } + + # Check if MESSAGE is a duplicate, if it is then increment the + # duplicates counter and return 1, otherwise, return 0. + proc _check_duplicates { message } { + variable all_test_names + # Initialise a count, or increment the count for this test name. if {![info exists all_test_names($message)]} { set all_test_names($message) 0 @@ -69,6 +72,43 @@ namespace eval ::CheckTestNames { inc_count duplicates } incr all_test_names($message) + return 1 + } + + return 0 + } + + # Remove the leading Dejagnu status marker from MESSAGE, and return the + # remainder of MESSAGE. A status marker is something like 'PASS: '. If + # no status marker is found, then just return MESSAGE unmodified. + proc _strip_status { message } { + foreach status {PASS FAIL XFAIL KFAIL XPASS KPASS UNRESOLVED \ + UNTESTED UNSUPPORTED} { + set txt "${status}: " + if { [string compare -length [string len $txt] \ + $txt $message] == 0 } { + return [string range $message [string len $txt] end] + } + } + + return $message + } + + # 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. + # + # Any offending paths in message cause PATHS_IN_TEST_NAMES to be + # incremented. + proc check { message } { + set message [ _strip_status $message ] + + if [ _check_paths $message ] { + clone_output "PATH: $message" + } + + if [ _check_duplicates $message ] { + clone_output "DUPLICATE: $message" } }