Some tests call fresh_perl* incorrectly

[p5pRT]

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

Searchable as RT128574$

[p5pRT]

From @dcollinsn

This is mostly a TODO for myself, but some tests call fresh_perl{,_is,_like} without the third "args" argument, and instead as fresh_perl_is($code, $expected, $name). This results in the test "name" not being passed properly, since the third argument is supposed to be a hashref.

I'll probably either grep for them or temporarily instrument fresh_perl to fail if it looks like the third argument is a string.

--
Respectfully,
Dan Collins

[p5pRT]

From @tonycoz

On Thu Jul 07 21​:06​:54 2016, dcollinsn@​gmail.com wrote​:

This is mostly a TODO for myself, but some tests call
fresh_perl{,_is,_like} without the third "args" argument, and instead
as fresh_perl_is($code, $expected, $name). This results in the test
"name" not being passed properly, since the third argument is supposed
to be a hashref.

I'll probably either grep for them or temporarily instrument
fresh_perl to fail if it looks like the third argument is a string.

It's probably better to have fresh_perl.*() always fail when the third argument is a string, to prevent future changes re-introducing the problem.

Tony

[p5pRT]

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

[p5pRT]

From @dcollinsn

On Sun Jul 10 17​:57​:22 2016, tonyc wrote​:

It's probably better to have fresh_perl.*() always fail when the third
argument is a string, to prevent future changes re-introducing the
problem.

Tony

Good point, Tony. The attached patch does so, and fixes all issues in my Linux builds, including LD, Quadmath, Threads, and Multiplicity tests. However, this should be smoked before being committed to blead in order to make sure that there aren't any more bad calls from os-specific tests.

--
Respectfully,
Dan Collins

[p5pRT]

From @dcollinsn

0001-RT-128574-Fix-use-of-fresh_perl-in-tests.patch

From 73f701b99ac67c736ea6d896cc8a8b2af479032c Mon Sep 17 00:00:00 2001
From: Dan Collins <dcollinsn@gmail.com>
Date: Sun, 10 Jul 2016 22:05:40 -0400
Subject: [PATCH] [RT #128574] Fix use of fresh_perl in tests

---
 t/io/layers.t  | 2 +-
 t/op/lex.t     | 1 +
 t/op/threads.t | 2 +-
 t/op/tr.t      | 2 +-
 t/test.pl      | 3 +++
 5 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/t/io/layers.t b/t/io/layers.t
index 86d171c..5e62800 100644
--- a/t/io/layers.t
+++ b/t/io/layers.t
@@ -223,7 +223,7 @@ __EOH__
 
     # Check that PL_sigwarn's reference count is correct, and that 
     # &PerlIO::Layer::NoWarnings isn't prematurely freed.
-    fresh_perl_like (<<"EOT", qr/^CODE/);
+    fresh_perl_like (<<"EOT", qr/^CODE/, {}, "Check PL_sigwarn's reference count");
 open(UTF, "<:raw:encoding(utf8)", '$afile') or die \$!;
 print ref *PerlIO::Layer::NoWarnings{CODE};
 EOT
diff --git a/t/op/lex.t b/t/op/lex.t
index 00e64fc..6c207d7 100644
--- a/t/op/lex.t
+++ b/t/op/lex.t
@@ -214,5 +214,6 @@ fresh_perl_is(
   '$_ = q-strict.pm-; 1 ? require : die;'
  .' print qq-ok\n- if $INC{q-strict.pm-}',
   "ok\n",
+  {},
   'foo ? require : bar [perl #128307]'
 );
diff --git a/t/op/threads.t b/t/op/threads.t
index 123ad27..2acb8d8 100644
--- a/t/op/threads.t
+++ b/t/op/threads.t
@@ -399,7 +399,7 @@ fresh_perl_is(
   'no crash when deleting $::{INC} in thread'
 );
 
-fresh_perl_is(<<'CODE', 'ok', 'no crash modifying extended array element');
+fresh_perl_is(<<'CODE', 'ok', {}, 'no crash modifying extended array element');
 use threads;
 my @a = 1;
 threads->create(sub { $#a = 1; $a[1] = 2; print qq/ok\n/ })->join;
diff --git a/t/op/tr.t b/t/op/tr.t
index b50ac42..b53d2bf 100644
--- a/t/op/tr.t
+++ b/t/op/tr.t
@@ -502,7 +502,7 @@ is( ref $x, 'SCALAR', "    doesn't stringify its argument" );
 
 # rt.perl.org 36622.  Perl didn't like a y/// at end of file.  No trailing
 # newline allowed.
-fresh_perl_is(q[$_ = "foo"; y/A-Z/a-z/], '');
+fresh_perl_is(q[$_ = "foo"; y/A-Z/a-z/], '', {}, 'RT #36622 y/// at end of file');
 
 
 { # [perl #38293] chr(65535) should be allowed in regexes
diff --git a/t/test.pl b/t/test.pl
index 41b77f4..6ef9770 100644
--- a/t/test.pl
+++ b/t/test.pl
@@ -959,6 +959,9 @@ my $tmpfile = tempfile();
 sub _fresh_perl {
     my($prog, $action, $expect, $runperl_args, $name) = @_;
 
+    die sprintf "Third argument to fresh_perl_.* must be hashref of args to fresh_perl (or {})"
+        unless !(defined $runperl_args) || ref($runperl_args) eq 'HASH';
+
     # Given the choice of the mis-parsable {}
     # (we want an anon hash, but a borked lexer might think that it's a block)
     # or relying on taking a reference to a lexical
-- 
2.8.1

[p5pRT]

From @tonycoz

On Sun Jul 10 19​:22​:24 2016, dcollinsn@​gmail.com wrote​:

On Sun Jul 10 17​:57​:22 2016, tonyc wrote​:

It's probably better to have fresh_perl.*() always fail when the
third
argument is a string, to prevent future changes re-introducing the
problem.

Tony

Good point, Tony. The attached patch does so, and fixes all issues in
my Linux builds, including LD, Quadmath, Threads, and Multiplicity
tests. However, this should be smoked before being committed to blead
in order to make sure that there aren't any more bad calls from os-
specific tests.

Pushed to smoke-me/tonyc/128574-fresh_perl

Tony

[p5pRT]

From @dcollinsn

This one was hidden behind a skip unless run in a threading perl

--
Respectfully,
Dan Collins

[p5pRT]

From @dcollinsn

0002-RT-128574-Missed-one-incorrect-usage-of-fresh_perl_.patch

From 82ced08ef67357b1ac7c480907cf94b1e8bba652 Mon Sep 17 00:00:00 2001
From: Dan Collins <dcollinsn@gmail.com>
Date: Wed, 13 Jul 2016 21:50:28 -0400
Subject: [PATCH] [RT #128574] Missed one incorrect usage of fresh_perl_

---
 t/op/reset.t | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/op/reset.t b/t/op/reset.t
index 227c84a..9b35c1a 100644
--- a/t/op/reset.t
+++ b/t/op/reset.t
@@ -184,7 +184,7 @@ SKIP:
 	    my $copy = $prog;
 	    $copy =~ s/8/$eight/gm;
 	    $copy =~ s/9/$nine/gm;
-	    fresh_perl_is($copy, "pass", "",
+	    fresh_perl_is($copy, "pass", {},
 			  "first pattern $eight$eight, second $nine$nine");
 	}
     }
-- 
2.8.1

[p5pRT]

From @tonycoz

On Wed Jul 13 18​:55​:18 2016, dcollinsn@​gmail.com wrote​:

This one was hidden behind a skip unless run in a threading perl

Thanks, did some more testing and applied both to blead as 2a91eb1 and ec6a838.

Tony

[p5pRT]

@dcollinsn - Status changed from 'open' to 'resolved'