[PATCH] Failures in t/re/regexp.t should show the expected result (more clearly) #15397

p5pRT · 2016-06-18T22:27:03Z

Migrated from rt.perl.org#128432 (status was 'resolved')

Searchable as RT128432$

p5pRT · 2016-06-18T22:27:03Z

From @dcollinsn

The following are the diagnostics from a fake test in re_tests, that I made for the purposes of getting a failure:

not ok 1971 - [perl 128420] recursive matches () aa$|a(?R)a|a:aaastr:y:$&:aaaexp => 'aaa', match=1
# $subject = "aaastr";
#
# $got = "aaa";
#
# ;
# $match = ($subject =~ m'aa$|a(?R)a|a') while $c--;
# $got = "$&";

The "expected" output is not printed separately. This is somewhat confusing, because most tests will show got/expected on failure, but these tests show subject/got. Now, all this data (regex, subject, expected, got) are all on the line with the "not OK", but that's a very dense line, and someone unfamiliar with the format may 1) not even know to look, or 2) not know which field is which.

Would the attached patch, which changes the format to (subject, code, got, expected), break anything that relies on the format of those comments?

The second patch removes the random ';' from the code to be tested when $study is empty.

p5pRT · 2016-06-18T22:27:03Z

From @dcollinsn

0001-t-re-regexp.t-Better-formatting-for-test-failures.patch

From d331a8f82aec09042aec0b37dac008e577ce7b8a Mon Sep 17 00:00:00 2001
From: Dan Collins <dcollinsn@gmail.com>
Date: Sat, 18 Jun 2016 16:52:26 -0400
Subject: [PATCH 1/2] t/re/regexp.t: Better formatting for test failures

On test failure, most of our tests output a description of the test,
followed by the actual result ("got") and the desired result
("expected"). This brings the tests in t/re/re_tests in line by
adding "expected" to the output, and changing the order slightly,
to more naturally describe the test, the output, and the expectation.
---
 t/re/regexp.t | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/re/regexp.t b/t/re/regexp.t
index 8e98e55..38a24d5 100644
--- a/t/re/regexp.t
+++ b/t/re/regexp.t
@@ -442,7 +442,8 @@ EOFCODE
 		else { # better diagnostics
 		    my $s = Data::Dumper->new([$subject],['subject'])->Useqq(1)->Dump;
 		    my $g = Data::Dumper->new([$got],['got'])->Useqq(1)->Dump;
-		    print "not ok $testname$todo ($study) $input => '$got', match=$match\n", _comment("$s\n$g\n$code\n");
+		    my $e = Data::Dumper->new([$expect],['expected'])->Useqq(1)->Dump;
+		    print "not ok $testname$todo ($study) $input => '$got', match=$match\n", _comment("$s\n$code\n$g\n$e\n");
 		}
 		next TEST;
 	    }
-- 
2.8.1

p5pRT · 2016-06-18T22:27:03Z

From @dcollinsn

0002-t-re-regexp.t-Remove-extra-semicolons-from-output.patch

From f887647e783899ff693e1bb047067a3483d5f253 Mon Sep 17 00:00:00 2001
From: Dan Collins <dcollinsn@gmail.com>
Date: Sat, 18 Jun 2016 17:24:22 -0400
Subject: [PATCH 2/2] t/re/regexp.t: Remove extra semicolons from output.

The $code segment in t/re/regexp.t contains an extra ';' for the first iteration
of each test, due to how 'study' and 'utf8::upgrade' testing is implemented.
Since this is the only test likely to fail (what with study being a no-op), this
is almost always line noise. This patch removes that ';'.

It would be nice to remove the back to back newlines as well.
---
 t/re/regexp.t | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/t/re/regexp.t b/t/re/regexp.t
index 38a24d5..5ec6e5c 100644
--- a/t/re/regexp.t
+++ b/t/re/regexp.t
@@ -358,8 +358,8 @@ foreach (@tests) {
         }
     }
 
-    for my $study ('', 'study $subject', 'utf8::upgrade($subject)',
-		   'utf8::upgrade($subject); study $subject') {
+    for my $study ('', 'study $subject;', 'utf8::upgrade($subject);',
+		   'utf8::upgrade($subject); study $subject;') {
 	# Need to make a copy, else the utf8::upgrade of an already studied
 	# scalar confuses things.
 	my $subject = $subject;
@@ -367,7 +367,7 @@ foreach (@tests) {
 	my ($code, $match, $got);
         if ($repl eq 'pos') {
             $code= <<EOFCODE;
-                $study;
+                $study
                 pos(\$subject)=0;
                 \$match = ( \$subject =~ m${pat}g );
                 \$got = pos(\$subject);
@@ -376,7 +376,7 @@ EOFCODE
         elsif ($qr_embed) {
             $code= <<EOFCODE;
                 my \$RE = qr$pat;
-                $study;
+                $study
                 \$match = (\$subject =~ /(?:)\$RE(?:)/) while \$c--;
                 \$got = "$repl";
 EOFCODE
@@ -386,14 +386,14 @@ EOFCODE
 		# Can't run the match in a subthread, but can do this and
 	 	# clone the pattern the other way.
                 my \$RE = threads->new(sub {qr$pat})->join();
-                $study;
+                $study
                 \$match = (\$subject =~ /(?:)\$RE(?:)/) while \$c--;
                 \$got = "$repl";
 EOFCODE
         }
         else {
             $code= <<EOFCODE;
-                $study;
+                $study
                 \$match = (\$subject =~ $OP$pat) while \$c--;
                 \$got = "$repl";
 EOFCODE
-- 
2.8.1

p5pRT · 2016-06-19T07:00:59Z

From @demerphq

On 19 June 2016 at 00:27, Dan Collins <perlbug-followup@perl.org> wrote:

# New Ticket Created by Dan Collins
# Please include the string: [perl #128432]
# in the subject line of all future correspondence about this issue.
# <URL: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=128432 >

The following are the diagnostics from a fake test in re_tests, that I made for the purposes of getting a failure:
not ok 1971 \- \[perl 128420\] recursive matches  aa$|a$?R$a|a&#8203;:aaastr&#8203;:y&#8203;:$&&#8203;:aaaexp => 'aaa'\, match=1
\# $subject = "aaastr";
\#
\# $got = "aaa";
\#
\#                 ;
\#                 $match = $$subject =~ m'aa$|a\(?R$a|a'\) while $c\-\-;
\#                 $got = "$&";
The "expected" output is not printed separately. This is somewhat confusing, because most tests will show got/expected on failure, but these tests show subject/got. Now, all this data (regex, subject, expected, got) are all on the line with the "not OK", but that's a very dense line, and someone unfamiliar with the format may 1) not even know to look, or 2) not know which field is which.

Would the attached patch, which changes the format to (subject, code, got, expected), break anything that relies on the format of those comments?

FWIW, it would be easier to assess if you included what the output
would look like with your patch.

But at the same time, IMO this patch is fine.

Yves

p5pRT · 2016-06-19T07:01:00Z

The RT System itself - Status changed from 'new' to 'open'

p5pRT · 2016-07-11T00:36:22Z

From @tonycoz

On Sun Jun 19 00:00:59 2016, demerphq wrote:

FWIW, it would be easier to assess if you included what the output
would look like with your patch.

But at the same time, IMO this patch is fine.

A failing test changes from (I changed the expected-expr for the first test):

not ok 10 () abc:abc:y:$&:abx => 'abc', match=1
# $subject = "abc";
#
# $got = "abc";
#
# ;
# $match = ($subject =~ m'abc') while $c--;
# $got = "$&";

to:

not ok 10 () abc:abc:y:$&:abx => 'abc', match=1
# $subject = "abc";
#
# ;
# $match = ($subject =~ m'abc') while $c--;
# $got = "$&";
#
# $got = "abc";
#
# $expected = "abx";

which I think is more readable.

Applied as 6ea2424 and cf549b9.

Tony

p5pRT · 2016-07-11T00:36:22Z

@tonycoz - Status changed from 'open' to 'pending release'

p5pRT · 2017-05-30T20:14:06Z

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release today of Perl 5.26.0, this and 210 other issues have been
resolved.

Perl 5.26.0 may be downloaded via:
https://metacpan.org/release/XSAWYERX/perl-5.26.0

If you find that the problem persists, feel free to reopen this ticket.

p5pRT · 2017-05-30T20:14:07Z

@khwilliamson - Status changed from 'pending release' to 'resolved'

p5pRT closed this as completed May 30, 2017

p5pRT added the Severity Low label Oct 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PATCH] Failures in t/re/regexp.t should show the expected result (more clearly) #15397

[PATCH] Failures in t/re/regexp.t should show the expected result (more clearly) #15397

p5pRT commented Jun 18, 2016

p5pRT commented Jun 18, 2016

p5pRT commented Jun 18, 2016

p5pRT commented Jun 18, 2016

p5pRT commented Jun 19, 2016

p5pRT commented Jun 19, 2016

p5pRT commented Jul 11, 2016

p5pRT commented Jul 11, 2016

p5pRT commented May 30, 2017

p5pRT commented May 30, 2017

[PATCH] Failures in t/re/regexp.t should show the expected result (more clearly) #15397

[PATCH] Failures in t/re/regexp.t should show the expected result (more clearly) #15397

Comments

p5pRT commented Jun 18, 2016

p5pRT commented Jun 18, 2016

From @dcollinsn

p5pRT commented Jun 18, 2016

From @dcollinsn

p5pRT commented Jun 18, 2016

From @dcollinsn

p5pRT commented Jun 19, 2016

From @demerphq

p5pRT commented Jun 19, 2016

p5pRT commented Jul 11, 2016

From @tonycoz

p5pRT commented Jul 11, 2016

p5pRT commented May 30, 2017

From @khwilliamson

p5pRT commented May 30, 2017