[PATCH] testsuite: Don't include multiline regexps in the the pass/fail log

[PATCH] testsuite: Don't include multiline regexps in the the pass/fail log

[Hans-Peter Nilsson]

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(-)

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]
-- 
2.30.2

Re: [PATCH] testsuite: Don't include multiline regexps in the the pass/fail log

[David Malcolm]

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]

Re: [PATCH] testsuite: Don't include multiline patterns in the the pass/fail log

[Hans-Peter Nilsson]

> 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]
> 

Re: [PATCH] testsuite: Don't include multiline regexps in the the pass/fail log

[Mike Stump]

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.

Re: [PATCH] testsuite: Don't include multiline regexps in the the pass/fail log

[Hans-Peter Nilsson]

> 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

Re: [PATCH] testsuite: Don't include multiline regexps in the the pass/fail log

[Mike Stump]

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.