Message ID | 20230224175433.CF5002043D@pchp3.se.axis.com |
---|---|
State | Committed |
Commit | a24b2720de5d888a936c350378ae864b81f1a022 |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 21BEF385B528 for <patchwork@sourceware.org>; Fri, 24 Feb 2023 17:55:05 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 21BEF385B528 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1677261305; bh=HYW2swfbfyjVGMZyYaQ4i04dvJv3iRSC2KwJsIO6ljQ=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=yDqok6i0nPYKdoOWFL4sQ50RRHVGfYzByGVOrn+SrFgRJ0I1T8TyXKBY5XajSMBUg y1wnD4kLyRifE/1onY6Pblpjc80vTeY1Z7YQTG5ZloiErGvtgP1pBBSMpzs3TsSVjs C2dpW51NbVHXRmYNOXP2RH0JU3833tYBrnS40Ivs= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp1.axis.com (smtp1.axis.com [195.60.68.17]) by sourceware.org (Postfix) with ESMTPS id 4FB053857C45 for <gcc-patches@gcc.gnu.org>; Fri, 24 Feb 2023 17:54:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4FB053857C45 To: <gcc-patches@gcc.gnu.org>, <dmalcolm@redhat.com> Subject: [PATCH] testsuite: Don't include multiline regexps in the the pass/fail log MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 8BIT Message-ID: <20230224175433.CF5002043D@pchp3.se.axis.com> Date: Fri, 24 Feb 2023 18:54:33 +0100 X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_PASS, SPF_PASS, TXREP 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: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Hans-Peter Nilsson via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Hans-Peter Nilsson <hp@axis.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
testsuite: Don't include multiline regexps in the the pass/fail log
|
|
Commit Message
Hans-Peter Nilsson
Feb. 24, 2023, 5:54 p.m. UTC
Ok to commit? -- >8 -- I see overlong lines in the output when a test fails, for example for a bug exposed for cris-elf and pru-elf in gcc.dg/analyzer/allocation-size-multiline-3.c: Running /x/gcc/testsuite/gcc.dg/analyzer/analyzer.exp ... FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c expected multiline pattern lines 16-25 not found: "\s*int32_t \*ptr = alloca \(99\);[^\n\r]*\n \^~~~~~\n 'test_constant_99': events 1-2[^\n\r]*\n \|[^\n\r]*\n \| int32_t \*ptr = alloca \(99\);[^\n\r]*\n \| \^~~~~~\n \| \|[^\n\r]*\n \| \(1\) allocated 99 bytes here[^\n\r]*\n \| \(2\) assigned to 'int32_t \*' \{aka 'int \*'\} here; 'sizeof \(int32_t \{aka int\}\)' is '4'[^\n\r]*\n \|[^\n\r]*\n" FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c expected multiline pattern lines 34-43 not found: " int32_t \*ptr = alloca \(n \* 2\);[^\n\r]*\n \^~~~~~\n 'test_symbolic': events 1-2[^\n\r]*\n \|[^\n\r]*\n \| int32_t \*ptr = alloca \(n \* 2\);[^\n\r]*\n \| \^~~~~~\n \| \|[^\n\r]*\n \| \(1\) allocated 'n \* 2' bytes here[^\n\r]*\n \| \(2\) assigned to 'int32_t \*' \{aka 'int \*'\} here; 'sizeof \(int32_t \{aka int\}\)' is '4'[^\n\r]*\n \|[^\n\r]*\n" FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c (test for excess errors) That multiline-regexp-quoted-on-a-single-line is redundant when also outputting "lines 16-25" and "lines 34-43". It's also so noisy that it can be mistaken for a testsuite error. If there's a need to inspect it, it can be seen at verbose-level 4, i.e. persons interested in seeing it without editing sources can just add "-v -v -v -v". Let's "prune" the regexp from regular output, instead producing: Running /x/gcc/testsuite/gcc.dg/analyzer/analyzer.exp ... FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c expected multiline pattern lines 16-25 not found FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c expected multiline pattern lines 34-43 not found FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c (test for excess errors) * lib/multiline.exp (handle-multiline-outputs): Don't include the quoted multiline regexp in the pass/fail output. --- gcc/testsuite/lib/multiline.exp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Comments
On Fri, 2023-02-24 at 18:54 +0100, Hans-Peter Nilsson wrote: > Ok to commit? Looks good to me [1] Thanks Dave [1] though FWIW although I wrote this code, my DejaGnu skills are weak and I'm not a testsuite maintainer :-/ > > -- >8 -- > I see overlong lines in the output when a test fails, for > example for a bug exposed for cris-elf and pru-elf in > gcc.dg/analyzer/allocation-size-multiline-3.c: > > Running /x/gcc/testsuite/gcc.dg/analyzer/analyzer.exp ... > FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c expected > multiline pattern lines 16-25 not found: "\s*int32_t \*ptr = alloca > \(99\);[^\n\r]*\n \^~~~~~\n 'test_constant_99': > events 1-2[^\n\r]*\n \|[^\n\r]*\n \| int32_t \*ptr = alloca > \(99\);[^\n\r]*\n \| \^~~~~~\n > \| \|[^\n\r]*\n \| \(1\) > allocated 99 bytes here[^\n\r]*\n \| \(2\) > assigned to 'int32_t \*' \{aka 'int \*'\} here; 'sizeof \(int32_t > \{aka int\}\)' is '4'[^\n\r]*\n \|[^\n\r]*\n" > FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c expected > multiline pattern lines 34-43 not found: " int32_t \*ptr = alloca > \(n \* 2\);[^\n\r]*\n \^~~~~~\n 'test_symbolic': > events 1-2[^\n\r]*\n \|[^\n\r]*\n \| int32_t \*ptr = alloca > \(n \* 2\);[^\n\r]*\n \| \^~~~~~\n > \| \|[^\n\r]*\n \| \(1\) > allocated 'n \* 2' bytes here[^\n\r]*\n \| \(2\) > assigned to 'int32_t \*' \{aka 'int \*'\} here; 'sizeof \(int32_t > \{aka int\}\)' is '4'[^\n\r]*\n \|[^\n\r]*\n" > FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c (test for excess > errors) > > That multiline-regexp-quoted-on-a-single-line is redundant > when also outputting "lines 16-25" and "lines 34-43". It's > also so noisy that it can be mistaken for a testsuite error. > If there's a need to inspect it, it can be seen at > verbose-level 4, i.e. persons interested in seeing it > without editing sources can just add "-v -v -v -v". > > Let's "prune" the regexp from regular output, instead producing: > Running /x/gcc/testsuite/gcc.dg/analyzer/analyzer.exp ... > FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c expected > multiline pattern lines 16-25 not found > FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c expected > multiline pattern lines 34-43 not found > FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c (test for excess > errors) > > * lib/multiline.exp (handle-multiline-outputs): Don't include > the > quoted multiline regexp in the pass/fail output. > --- > gcc/testsuite/lib/multiline.exp | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/gcc/testsuite/lib/multiline.exp > b/gcc/testsuite/lib/multiline.exp > index 84ba9216642e..5eccf2bbebc1 100644 > --- a/gcc/testsuite/lib/multiline.exp > +++ b/gcc/testsuite/lib/multiline.exp > @@ -169,9 +169,9 @@ proc handle-multiline-outputs { text } { > # Use "regsub" to attempt to prune the pattern from $text > if {[regsub -line $rexp $text "" text]} { > # The multiline pattern was pruned. > - ${maybe_x}pass "$title was found: \"$escaped_regex\"" > + ${maybe_x}pass "$title was found" > } else { > - ${maybe_x}fail "$title not found: \"$escaped_regex\"" > + ${maybe_x}fail "$title not found" > } > > set index [expr $index + 1]
> From: David Malcolm <dmalcolm@redhat.com> > Date: Fri, 24 Feb 2023 14:07:02 -0500 > Old-Content-Type: text/plain; charset="UTF-8" > Old-Content-Transfer-Encoding: base64 > Content-Type: TEXT/plain; charset=iso-8859-1 > > On Fri, 2023-02-24 at 18:54 +0100, Hans-Peter Nilsson wrote: > > Ok to commit? > > Looks good to me [1] > > Thanks > Dave Thanks! > [1] though FWIW although I wrote this code, my DejaGnu skills are weak > and I'm not a testsuite maintainer :-/ But that "ok", reflecting your logging intent, makes the patch obvious! So, committed, per below. Also, I noticed overuse of the term "regexp", so s/regexp/pattern/g, where g also includes the subject of this mail. My mind was on the follow-up patchset starting at "https://gcc.gnu.org/pipermail/gcc-patches/2023-February/612810.html" where I accidentally left you out for CC, but now inviting to have a look. JFTR: Regarding the "escaped_regex" in the context of this patch, that's when the pattern has been frobbed into a simple rexexp, a glob, but that's internal. brgds, H-P (PS. quoted contents below also reworded) > > > > -- >8 -- > > I see overlong lines in the output when a test fails, for > > example for a bug exposed for cris-elf and pru-elf in > > gcc.dg/analyzer/allocation-size-multiline-3.c: > > > > Running /x/gcc/testsuite/gcc.dg/analyzer/analyzer.exp ... > > FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c expected > > multiline pattern lines 16-25 not found: "\s*int32_t \*ptr = alloca > > \(99\);[^\n\r]*\n \^~~~~~\n 'test_constant_99': > > events 1-2[^\n\r]*\n \|[^\n\r]*\n \| int32_t \*ptr = alloca > > \(99\);[^\n\r]*\n \| \^~~~~~\n > > \| \|[^\n\r]*\n \| \(1\) > > allocated 99 bytes here[^\n\r]*\n \| \(2\) > > assigned to 'int32_t \*' \{aka 'int \*'\} here; 'sizeof \(int32_t > > \{aka int\}\)' is '4'[^\n\r]*\n \|[^\n\r]*\n" > > FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c expected > > multiline pattern lines 34-43 not found: " int32_t \*ptr = alloca > > \(n \* 2\);[^\n\r]*\n \^~~~~~\n 'test_symbolic': > > events 1-2[^\n\r]*\n \|[^\n\r]*\n \| int32_t \*ptr = alloca > > \(n \* 2\);[^\n\r]*\n \| \^~~~~~\n > > \| \|[^\n\r]*\n \| \(1\) > > allocated 'n \* 2' bytes here[^\n\r]*\n \| \(2\) > > assigned to 'int32_t \*' \{aka 'int \*'\} here; 'sizeof \(int32_t > > \{aka int\}\)' is '4'[^\n\r]*\n \|[^\n\r]*\n" > > FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c (test for excess > > errors) > > > > That multiline-pattern-quoted-on-a-single-line is redundant > > when also outputting "lines 16-25" and "lines 34-43". It's > > also so noisy that it can be mistaken for a testsuite error. > > If there's a need to inspect it, it can be seen at > > verbose-level 4, i.e. persons interested in seeing it > > without editing sources can just add "-v -v -v -v". > > > > Let's "prune" the pattern from regular output, instead producing: > > Running /x/gcc/testsuite/gcc.dg/analyzer/analyzer.exp ... > > FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c expected > > multiline pattern lines 16-25 not found > > FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c expected > > multiline pattern lines 34-43 not found > > FAIL: gcc.dg/analyzer/allocation-size-multiline-3.c (test for excess > > errors) > > > > * lib/multiline.exp (handle-multiline-outputs): Don't include > > the > > quoted multiline pattern in the pass/fail output. > > --- > > gcc/testsuite/lib/multiline.exp | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/gcc/testsuite/lib/multiline.exp > > b/gcc/testsuite/lib/multiline.exp > > index 84ba9216642e..5eccf2bbebc1 100644 > > --- a/gcc/testsuite/lib/multiline.exp > > +++ b/gcc/testsuite/lib/multiline.exp > > @@ -169,9 +169,9 @@ proc handle-multiline-outputs { text } { > > # Use "regsub" to attempt to prune the pattern from $text > > if {[regsub -line $rexp $text "" text]} { > > # The multiline pattern was pruned. > > - ${maybe_x}pass "$title was found: \"$escaped_regex\"" > > + ${maybe_x}pass "$title was found" > > } else { > > - ${maybe_x}fail "$title not found: \"$escaped_regex\"" > > + ${maybe_x}fail "$title not found" > > } > > > > set index [expr $index + 1] >
On Feb 24, 2023, at 9:54 AM, Hans-Peter Nilsson via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Ok to commit? Ok. Thanks. > diff --git a/gcc/testsuite/lib/multiline.exp b/gcc/testsuite/lib/multiline.exp > index 84ba9216642e..5eccf2bbebc1 100644 > --- a/gcc/testsuite/lib/multiline.exp > +++ b/gcc/testsuite/lib/multiline.exp > - ${maybe_x}pass "$title was found: \"$escaped_regex\"" > + ${maybe_x}pass "$title was found" > } else { > - ${maybe_x}fail "$title not found: \"$escaped_regex\"" > + ${maybe_x}fail "$title not found" Side remark: So, the string on pass and the string on fail are supposed to be exactly the same. Regression analysis works only if the string is the same. "regexp test", might be suggestive enough and can be the same spelling for both.
> From: Mike Stump <mikestump@comcast.net> > Date: Mon, 27 Feb 2023 09:41:18 -0800 > > diff --git a/gcc/testsuite/lib/multiline.exp b/gcc/testsuite/lib/multiline.exp > > index 84ba9216642e..5eccf2bbebc1 100644 > > --- a/gcc/testsuite/lib/multiline.exp > > +++ b/gcc/testsuite/lib/multiline.exp > > > - ${maybe_x}pass "$title was found: \"$escaped_regex\"" > > + ${maybe_x}pass "$title was found" > > } else { > > - ${maybe_x}fail "$title not found: \"$escaped_regex\"" > > + ${maybe_x}fail "$title not found" > > Side remark: > > So, the string on pass and the string on fail are supposed > to be exactly the same. Regression analysis works only if > the string is the same. "regexp test", might be > suggestive enough and can be the same spelling for both. Right. Should I changed it now? (Pro: see above. Con: again meddling with regression-test history.) Like (editing on the fly here) as the "found" part seems redundant: > > - ${maybe_x}pass "$title was found" > > + ${maybe_x}pass "$title" > > } else { > > - ${maybe_x}fail "$title not found" > > + ${maybe_x}fail "$title" brgds, H-P
On Feb 27, 2023, at 9:59 AM, Hans-Peter Nilsson <hp@axis.com> wrote: > >> From: Mike Stump <mikestump@comcast.net> >> Date: Mon, 27 Feb 2023 09:41:18 -0800 > >>> diff --git a/gcc/testsuite/lib/multiline.exp b/gcc/testsuite/lib/multiline.exp >>> index 84ba9216642e..5eccf2bbebc1 100644 >>> --- a/gcc/testsuite/lib/multiline.exp >>> +++ b/gcc/testsuite/lib/multiline.exp >> >>> - ${maybe_x}pass "$title was found: \"$escaped_regex\"" >>> + ${maybe_x}pass "$title was found" >>> } else { >>> - ${maybe_x}fail "$title not found: \"$escaped_regex\"" >>> + ${maybe_x}fail "$title not found" >> >> Side remark: >> >> So, the string on pass and the string on fail are supposed >> to be exactly the same. Regression analysis works only if >> the string is the same. "regexp test", might be >> suggestive enough and can be the same spelling for both. > > Right. Should I changed it now? Sure. Not required, but would be nice. > (Pro: see above. Con: again meddling with regression-test history.) Only new tests that don't have any polish do this sort of thing. :-) > Like (editing on the fly here) as the "found" part seems > redundant: > >>> - ${maybe_x}pass "$title was found" >>> + ${maybe_x}pass "$title" >>> } else { >>> - ${maybe_x}fail "$title not found" >>> + ${maybe_x}fail "$title" Yes, same difference. Both strings should be identical. Thanks.
diff --git a/gcc/testsuite/lib/multiline.exp b/gcc/testsuite/lib/multiline.exp index 84ba9216642e..5eccf2bbebc1 100644 --- a/gcc/testsuite/lib/multiline.exp +++ b/gcc/testsuite/lib/multiline.exp @@ -169,9 +169,9 @@ proc handle-multiline-outputs { text } { # Use "regsub" to attempt to prune the pattern from $text if {[regsub -line $rexp $text "" text]} { # The multiline pattern was pruned. - ${maybe_x}pass "$title was found: \"$escaped_regex\"" + ${maybe_x}pass "$title was found" } else { - ${maybe_x}fail "$title not found: \"$escaped_regex\"" + ${maybe_x}fail "$title not found" } set index [expr $index + 1]