New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multiple testsuite fixup #16149
Comments
From @atoomicThis is a bug report for perl from atoomic@cpan.org, This ticket is about updating a few unit tests for several reasons: 1 - use set_up_inc helper to set @INC Each idea comes with its own commit as some might be more difficult to Commits can be read online here: Flags: Site configuration information for perl 5.24.1: Configured by cPanel at Mon Aug 21 10:13:10 CDT 2017. Summary of my perl5 (revision 5 version 24 subversion 1) configuration: Platform: Locally applied patches: @INC for perl 5.24.1: /usr/local/cpanel/3rdparty/perl/524/lib64/perl5/cpanel_lib/x86_64-linux-64int /usr/local/cpanel/3rdparty/perl/524/lib64/perl5/5.24.1/x86_64-linux-64int Environment for perl 5.24.1: PATH=/usr/local/cpanel/3rdparty/perl/526/bin:/usr/local/cpanel/3rdparty/perl/524/bin:/usr/local/cpanel/3rdparty/perl/522/bin:/usr/local/cpanel/3rdparty/perl/514/bin:/usr/local/cpanel/3rdparty/bin:/root/bin/:/opt/local/bin:/opt/local/sbin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/opt/cpanel/composer/bin:/root/.dotfiles/bin:/root/perl5/bin:/root/.rvm/bin:/root/bin |
From @atoomic0005-test-Can-run-B-with-the-test.patchFrom 09dbc2b5bb41cbbb14e656fdf16cb1382aa77ac0 Mon Sep 17 00:00:00 2001
From: Nicolas R <atoomic@cpan.org>
Date: Thu, 14 Sep 2017 14:52:59 -0600
Subject: [PATCH 5/6] test - Can run B with the test
This test is using B for testing purpose.
But if we run this test using a B module,
this is going to raise some errors.
Adjust the test to be compliant with B, while
preserving the original logic.
---
t/op/stash.t | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/t/op/stash.t b/t/op/stash.t
index c9634a370a..b4fe67fd48 100644
--- a/t/op/stash.t
+++ b/t/op/stash.t
@@ -63,9 +63,9 @@ SKIP: {
}
SKIP: {
- eval { require B; 1 } or skip "no B", 29;
+ eval q{ delete $INC{'B.pm'}; require B; 1 } or skip "no B", 29;
- *b = \&B::svref_2object;
+ *b = 'B'->can('svref_2object');
my $CVf_ANON = B::CVf_ANON();
my $sub = do {
@@ -110,7 +110,7 @@ SKIP: {
};
%four:: = ();
- my $gv = B::svref_2object($sub)->GV;
+ my $gv = 'B'->can('svref_2object')->($sub)->GV;
ok($gv->isa(q/B::GV/), "cleared stash leaves anon CV with valid GV");
my $st = eval { $gv->STASH->NAME };
@@ -122,7 +122,7 @@ SKIP: {
};
undef %five::;
- $gv = B::svref_2object($sub)->GV;
+ $gv = 'B'->can('svref_2object')->($sub)->GV;
ok($gv->isa(q/B::GV/), "undefed stash leaves anon CV with valid GV");
$st = eval { $gv->STASH->NAME };
@@ -137,7 +137,7 @@ SKIP: {
my $stash_glob = delete $::{"six::"};
# Now free the GV while the stash still exists (though detached)
delete $$stash_glob{"six"};
- $gv = B::svref_2object($sub)->GV;
+ $gv = 'B'->can('svref_2object')->($sub)->GV;
ok($gv->isa(q/B::GV/),
'anonymised CV whose stash is detached still has a GV');
is $gv->STASH->NAME, '__ANON__',
@@ -150,7 +150,7 @@ SKIP: {
my $rfoo = \&foo;
package main;
delete $::{'FOO::'};
- my $cv = B::svref_2object($rfoo);
+ my $cv = 'B'->can('svref_2object')->($rfoo);
# (is there a better way of testing for NULL ?)
my $stash = $cv->STASH;
like($stash, qr/B::SPECIAL/, "NULL CvSTASH on named sub");
@@ -167,7 +167,7 @@ SKIP: {
*f = sub {};
];
delete $FOO2::{f};
- my $cv = B::svref_2object($r);
+ my $cv = 'B'->can('svref_2object')->($r);
my $gv = $cv->GV;
ok($gv->isa(q/B::GV/), "orphaned CV has valid GV");
is($gv->NAME, '__ANON__', "orphaned CV has anon GV");
@@ -185,12 +185,12 @@ SKIP: {
delete $FOO3::{__ANON__}; # whoops!
my ($cv,$gv);
- $cv = B::svref_2object($named);
+ $cv = 'B'->can('svref_2object')->($named);
$gv = $cv->GV;
ok($gv->isa(q/B::GV/), "ex-named CV has valid GV");
is($gv->NAME, '__ANON__', "ex-named CV has anon GV");
- $cv = B::svref_2object($anon);
+ $cv = 'B'->can('svref_2object')->($anon);
$gv = $cv->GV;
ok($gv->isa(q/B::GV/), "anon CV has valid GV");
is($gv->NAME, '__ANON__', "anon CV has anon GV");
@@ -206,7 +206,7 @@ SKIP: {
}
}
- my $br = B::svref_2object($r);
+ my $br = 'B'->can('svref_2object')->($r);
is ($br->STASH->NAME, 'bloop',
'stub records the package it was compiled in');
# Arguably this shouldn't quite be here, but it's easy to add it
@@ -216,7 +216,7 @@ SKIP: {
# We need to take this reference "late", after the subroutine is
# defined.
- $br = B::svref_2object(eval 'sub whack {}; \&whack');
+ $br = 'B'->can('svref_2object')->(eval 'sub whack {}; \&whack');
die $@ if $@;
is ($br->STASH->NAME, 'main',
@@ -256,7 +256,7 @@ fresh_perl_is(
# effectively rename a stash
*slin:: = *rile::; *rile:: = *zor::;
-
+
::is *$globref, "*rile::tat",
'globs stringify the same way when stashes are moved';
::is ref $obj, "rile",
--
2.14.1
|
From @atoomic0006-t-op-symbolcache.t-should-return-true.patchFrom 62cc5551e4965ab5965ed9e7d1d08cc384275b71 Mon Sep 17 00:00:00 2001
From: Nicolas R <atoomic@cpan.org>
Date: Thu, 14 Sep 2017 14:53:20 -0600
Subject: [PATCH 6/6] t/op/symbolcache.t should return true
Add missing return 1
---
t/op/symbolcache.t | 2 ++
1 file changed, 2 insertions(+)
diff --git a/t/op/symbolcache.t b/t/op/symbolcache.t
index 20b522676f..1daabd8f82 100644
--- a/t/op/symbolcache.t
+++ b/t/op/symbolcache.t
@@ -41,3 +41,5 @@ is( replaced2(), 'meth', 'original function not bound, was replaced' );
ok( main->replaced2 eq 'meth', 'method is replaced function' );
BEGIN { undef $main::{replaced2} }
sub replaced2 { 'meth' }
+
+1;
--
2.14.1
|
From @atoomic0001-tests-Use-set_up_inc-helper-to-set-INC.patchFrom a5b6ec1564e563a3428d1e3059a78036bb4c11bb Mon Sep 17 00:00:00 2001
From: Nicolas R <atoomic@cpan.org>
Date: Thu, 14 Sep 2017 14:51:13 -0600
Subject: [PATCH 1/6] tests - Use set_up_inc helper to set @INC
Test plan should not be declared at compile time
---
t/comp/parser_run.t | 4 ++--
t/op/threads-dirh.t | 16 ++++++++--------
t/re/anyof.t | 6 ++++--
3 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/t/comp/parser_run.t b/t/comp/parser_run.t
index af35758bee..eba4b9fba8 100644
--- a/t/comp/parser_run.t
+++ b/t/comp/parser_run.t
@@ -5,11 +5,11 @@
# Note that this should still be runnable under miniperl.
BEGIN {
- @INC = qw(. ../lib );
chdir 't' if -d 't';
+ require './test.pl';
+ set_up_inc( qw(. ../lib ) );
}
-require './test.pl';
plan(4);
# [perl #130814] can reallocate lineptr while looking ahead for
diff --git a/t/op/threads-dirh.t b/t/op/threads-dirh.t
index e1d5c996ad..82d90768ab 100644
--- a/t/op/threads-dirh.t
+++ b/t/op/threads-dirh.t
@@ -4,21 +4,21 @@
BEGIN {
chdir 't' if -d 't';
- @INC = '../lib';
require './test.pl';
+ set_up_inc('../lib');
$| = 1;
-
require Config;
- skip_all_without_config('useithreads');
- skip_all_if_miniperl("no dynamic loading on miniperl, no threads");
-
- plan(6);
}
+skip_all_without_config('useithreads');
+skip_all_if_miniperl("no dynamic loading on miniperl, no threads");
+
+plan(6);
+
use strict;
use warnings;
-use threads;
-use threads::shared;
+eval q/use threads/;
+eval q/use threads::shared/;
use File::Path;
use File::Spec::Functions qw 'updir catdir';
use Cwd 'getcwd';
diff --git a/t/re/anyof.t b/t/re/anyof.t
index 84f2881e0d..d24e4a71a8 100644
--- a/t/re/anyof.t
+++ b/t/re/anyof.t
@@ -1,3 +1,5 @@
+#!./perl
+
use utf8;
# This tests that the ANYOF nodes generated by bracketed character classes are
@@ -10,9 +12,9 @@ use utf8;
BEGIN {
chdir 't' if -d 't';
- @INC = ('../lib','.','../ext/re');
- require Config; import Config;
require './test.pl';
+ set_up_inc('../lib','.','../ext/re');
+ require Config; import Config;
skip_all('no re module') unless defined &DynaLoader::boot_DynaLoader;
}
--
2.14.1
|
From @atoomic0002-tests-remove-useless-setting-in-BEGIN.patchFrom f882d1b083e298bbc3dfabcefc84a656fb8074e8 Mon Sep 17 00:00:00 2001
From: Nicolas R <atoomic@cpan.org>
Date: Thu, 14 Sep 2017 14:51:42 -0600
Subject: [PATCH 2/6] tests - remove useless setting in BEGIN
These two tests are not using test.pl
and do not need to load any special file.
No need for fancy setup in BEGIN, all the most
we cannot use the generic set_up_inc helper.
Simply remove the useless BEGIN block.
---
t/comp/bproto.t | 6 +-----
t/opbasic/cmp.t | 5 -----
2 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/t/comp/bproto.t b/t/comp/bproto.t
index 8d11b915c1..cc91242140 100644
--- a/t/comp/bproto.t
+++ b/t/comp/bproto.t
@@ -3,11 +3,7 @@
# check if builtins behave as prototyped
#
-BEGIN {
- chdir 't' if -d 't';
- @INC = '../lib';
-}
-
+# Ideally tests in t/comp wouldn't use require, as require isn't tested yet
print "1..16\n";
my $i = 1;
diff --git a/t/opbasic/cmp.t b/t/opbasic/cmp.t
index 241eb491a6..5a88d21d0b 100644
--- a/t/opbasic/cmp.t
+++ b/t/opbasic/cmp.t
@@ -1,10 +1,5 @@
#!./perl
-BEGIN {
- chdir 't' if -d 't';
- @INC = '../lib';
-}
-
# This file has been placed in t/opbasic to indicate that it should not use
# functions imported from t/test.pl or Test::More, as those programs/libraries
# use operators which are what is being tested in this file.
--
2.14.1
|
From @atoomic0003-test-Do-not-use-B-which-is-a-reserved-namespace.patchFrom 5a3d29cf575b11888c2f1e390f1c5d264ee1e897 Mon Sep 17 00:00:00 2001
From: Nicolas R <atoomic@cpan.org>
Date: Thu, 14 Sep 2017 14:52:19 -0600
Subject: [PATCH 3/6] test - Do not use B which is a reserved namespace
B is already a reserved namespace. This is a bad idea
to use B during unit test, as this increase the complexity
when using one of the B subpackage to run the test.
Simply rename B to BB ( and A to AA ).
---
t/mro/next_inanon.t | 18 +++++++++---------
t/mro/next_ineval.t | 16 ++++++++--------
t/op/method.t | 52 ++++++++++++++++++++++++++--------------------------
3 files changed, 43 insertions(+), 43 deletions(-)
diff --git a/t/mro/next_inanon.t b/t/mro/next_inanon.t
index b6f0451611..4c3c007d6e 100644
--- a/t/mro/next_inanon.t
+++ b/t/mro/next_inanon.t
@@ -13,26 +13,26 @@ anonymous subroutine.
=cut
{
- package A;
+ package AA;
use mro 'c3';
sub foo {
- return 'A::foo';
+ return 'AA::foo';
}
sub bar {
- return 'A::bar';
+ return 'AA::bar';
}
}
{
- package B;
- use base 'A';
+ package BB;
+ use base 'AA';
use mro 'c3';
sub foo {
my $code = sub {
- return 'B::foo => ' . (shift)->next::method();
+ return 'BB::foo => ' . (shift)->next::method();
};
return (shift)->$code;
}
@@ -40,7 +40,7 @@ anonymous subroutine.
sub bar {
my $code1 = sub {
my $code2 = sub {
- return 'B::bar => ' . (shift)->next::method();
+ return 'BB::bar => ' . (shift)->next::method();
};
return (shift)->$code2;
};
@@ -48,10 +48,10 @@ anonymous subroutine.
}
}
-is(B->foo, "B::foo => A::foo",
+is(BB->foo, "BB::foo => AA::foo",
'method resolved inside anonymous sub');
-is(B->bar, "B::bar => A::bar",
+is(BB->bar, "BB::bar => AA::bar",
'method resolved inside nested anonymous subs');
diff --git a/t/mro/next_ineval.t b/t/mro/next_ineval.t
index f8c13a6413..14a49b1c6b 100644
--- a/t/mro/next_ineval.t
+++ b/t/mro/next_ineval.t
@@ -12,23 +12,23 @@ This tests the use of an eval{} block to wrap a next::method call.
=cut
{
- package A;
+ package AA;
use mro 'c3';
sub foo {
- die 'A::foo died';
- return 'A::foo succeeded';
+ die 'AA::foo died';
+ return 'AA::foo succeeded';
}
}
{
- package B;
- use base 'A';
+ package BB;
+ use base 'AA';
use mro 'c3';
sub foo {
eval {
- return 'B::foo => ' . (shift)->next::method();
+ return 'BB::foo => ' . (shift)->next::method();
};
if ($@) {
@@ -37,8 +37,8 @@ This tests the use of an eval{} block to wrap a next::method call.
}
}
-like(B->foo,
- qr/^A::foo died/,
+like(BB->foo,
+ qr/^AA::foo died/,
'method resolved inside eval{}');
diff --git a/t/op/method.t b/t/op/method.t
index ef181c4ce0..e5cdce03e3 100644
--- a/t/op/method.t
+++ b/t/op/method.t
@@ -15,8 +15,8 @@ no warnings 'once';
plan(tests => 151);
-@A::ISA = 'B';
-@B::ISA = 'C';
+@A::ISA = 'BB';
+@BB::ISA = 'C';
sub C::d {"C::d"}
sub D::d {"D::d"}
@@ -55,7 +55,7 @@ is(method $obj, "method");
is( A->d, "C::d"); # Update hash table;
-*B::d = \&D::d; # Import now.
+*BB::d = \&D::d; # Import now.
is(A->d, "D::d"); # Update hash table;
{
@@ -67,42 +67,42 @@ is(A->d, "D::d"); # Update hash table;
is(A->d, "D::d");
{
- local *B::d;
- eval 'sub B::d {"B::d1"}'; # Import now.
- is(A->d, "B::d1"); # Update hash table;
- undef &B::d;
+ local *BB::d;
+ eval 'sub BB::d {"BB::d1"}'; # Import now.
+ is(A->d, "BB::d1"); # Update hash table;
+ undef &BB::d;
is((eval { A->d }, ($@ =~ /Undefined subroutine/)), 1);
}
is(A->d, "D::d"); # Back to previous state
-eval 'no warnings "redefine"; sub B::d {"B::d2"}'; # Import now.
-is(A->d, "B::d2"); # Update hash table;
+eval 'no warnings "redefine"; sub BB::d {"BB::d2"}'; # Import now.
+is(A->d, "BB::d2"); # Update hash table;
# What follows is hardly guarantied to work, since the names in scripts
-# are already linked to "pruned" globs. Say, 'undef &B::d' if it were
-# after 'delete $B::{d}; sub B::d {}' would reach an old subroutine.
+# are already linked to "pruned" globs. Say, 'undef &BB::d' if it were
+# after 'delete $BB::{d}; sub BB::d {}' would reach an old subroutine.
-undef &B::d;
-delete $B::{d};
+undef &BB::d;
+delete $BB::{d};
is(A->d, "C::d");
-eval 'sub B::d {"B::d2.5"}';
+eval 'sub BB::d {"BB::d2.5"}';
A->d; # Update hash table;
-my $glob = \delete $B::{d}; # non-void context; hang on to the glob
+my $glob = \delete $BB::{d}; # non-void context; hang on to the glob
is(A->d, "C::d"); # Update hash table;
-eval 'sub B::d {"B::d3"}'; # Import now.
-is(A->d, "B::d3"); # Update hash table;
+eval 'sub BB::d {"BB::d3"}'; # Import now.
+is(A->d, "BB::d3"); # Update hash table;
-delete $B::{d};
+delete $BB::{d};
*dummy::dummy = sub {}; # Mark as updated
is(A->d, "C::d");
-eval 'sub B::d {"B::d4"}'; # Import now.
-is(A->d, "B::d4"); # Update hash table;
+eval 'sub BB::d {"BB::d4"}'; # Import now.
+is(A->d, "BB::d4"); # Update hash table;
-delete $B::{d}; # Should work without any help too
+delete $BB::{d}; # Should work without any help too
is(A->d, "C::d");
{
@@ -119,16 +119,16 @@ my $counter;
eval <<'EOF';
sub C::e;
-BEGIN { *B::e = \&C::e } # Shouldn't prevent AUTOLOAD in original pkg
+BEGIN { *BB::e = \&C::e } # Shouldn't prevent AUTOLOAD in original pkg
sub Y::f;
$counter = 0;
@X::ISA = 'Y';
-@Y::ISA = 'B';
+@Y::ISA = 'BB';
-sub B::AUTOLOAD {
+sub BB::AUTOLOAD {
my $c = ++$counter;
- my $method = $B::AUTOLOAD;
+ my $method = $BB::AUTOLOAD;
my $msg = "B: In $method, $c";
eval "sub $method { \$msg }";
goto &$method;
@@ -157,7 +157,7 @@ is(Y->f(), "B: In Y::f, 3"); # Which sticks
{
no warnings 'redefine';
-*B::AUTOLOAD = sub {
+*BB::AUTOLOAD = sub {
use warnings;
my $c = ++$counter;
my $method = $::AUTOLOAD;
--
2.14.1
|
From @atoomic0004-test-Do-not-run-test-output-at-compilation-time.patchFrom f0ae221c72599e6d6c5fa53ae34f7b97f6d28f9a Mon Sep 17 00:00:00 2001
From: Nicolas R <atoomic@cpan.org>
Date: Thu, 14 Sep 2017 14:52:37 -0600
Subject: [PATCH 4/6] test - Do not run test output at compilation time
Even if in most of the cases it seems ok to print
output during compilation time, this is a pretty
bad idea (when compiling the test for example).
Run all the tests at compile time, but only
print the test status at run time.
---
t/op/array_base.t | 27 ++++++++++++----------
t/op/caller.t | 67 ++++++++++++++++++++++++++++++++++++++++---------------
2 files changed, 64 insertions(+), 30 deletions(-)
diff --git a/t/op/array_base.t b/t/op/array_base.t
index a30236d955..fc15ac830a 100644
--- a/t/op/array_base.t
+++ b/t/op/array_base.t
@@ -1,22 +1,25 @@
#!perl -w
use strict;
+my %begin_tests;
BEGIN {
chdir 't' if -d 't';
require './test.pl';
-
- plan (tests => my $tests = 11);
-
- # Run these at BEGIN time, before arybase loads
use v5.15;
- is(eval('$[ = 1; 123'), undef);
- like($@, qr/\AAssigning non-zero to \$\[ is no longer possible/);
+ # Run these at BEGIN time, before arybase loads
+ $begin_tests{123} = eval('$[ = 1; 123');
+ $begin_tests{error} = $@;
+}
+
+plan (tests => my $tests = 11); # plan should not be set at compile time
+
+is($begin_tests{123}, undef);
+like($begin_tests{error}, qr/\AAssigning non-zero to \$\[ is no longer possible/);
- if (is_miniperl()) {
+if (is_miniperl()) {
# skip the rest
SKIP: { skip ("no arybase.xs on miniperl", $tests-2) }
exit;
- }
}
no warnings 'deprecated';
@@ -25,17 +28,17 @@ is(eval('$['), 0);
is(eval('$[ = 0; 123'), 123);
is(eval('$[ = 1; 123'), 123);
$[ = 1;
-ok $INC{'arybase.pm'};
+ok($INC{'arybase.pm'}, "arybase is in INC");
use v5.15;
is(eval('$[ = 1; 123'), undef);
like($@, qr/\AAssigning non-zero to \$\[ is no longer possible/);
-is $[, 0, '$[ is 0 under 5.16';
+is($[, 0, '$[ is 0 under 5.16');
$_ = "hello";
/l/g;
my $pos = \pos;
-is $$pos, 3;
+is($$pos, 3);
$$pos = 1;
-is $$pos, 1;
+is($$pos, 1);
1;
diff --git a/t/op/caller.t b/t/op/caller.t
index 1ffb5b3443..3a38415053 100644
--- a/t/op/caller.t
+++ b/t/op/caller.t
@@ -5,12 +5,20 @@ BEGIN {
chdir 't' if -d 't';
require './test.pl';
set_up_inc('../lib');
- plan( tests => 100 ); # some tests are run in a BEGIN block
}
-my @c;
+my @tests;
+plan( tests => 100 );
+
+print "# Tests with caller(0)\n";
-BEGIN { print "# Tests with caller(0)\n"; }
+foreach my $t ( @tests ) {
+ my $s = \&{'main::'.$t->{type}};
+ $s->( @{$t->{args}}, $t->{txt} );
+}
+print "# end of BEGIN tests\n";
+
+my @c;
@c = caller(0);
ok( (!@c), "caller(0) in main program" );
@@ -36,8 +44,8 @@ ok( $c[4], "hasargs true with deleted sub" );
BEGIN {
require strict;
- is +(caller 0)[1], __FILE__,
- "[perl #68712] filenames after require in a BEGIN block"
+ push @tests, { type => 'is', args => [ +(caller 0)[1], __FILE__ ],
+ txt => "[perl #68712] filenames after require in a BEGIN block" };
}
print "# Tests with caller(1)\n";
@@ -97,6 +105,18 @@ sub testwarn {
check_bits( (caller(0))[9], $w, "warnings match caller ($id)");
}
+sub get_caller_0_9 {
+ return (caller(0))[9];
+}
+
+sub get_caller_0_9 {
+ return (caller(0))[9];
+}
+
+sub get_caller_0_9 {
+ return (caller(0))[9];
+}
+
{
no warnings;
# Build the warnings mask dynamically
@@ -109,20 +129,27 @@ sub testwarn {
vec($registered, $warnings::LAST_BIT/2, 2) = 1;
}
- BEGIN { check_bits( ${^WARNING_BITS}, "\0" x $warnings::BYTES, 'all bits off via "no warnings"' ) }
+ BEGIN {
+ push @tests, { type => 'check_bits', args => [ ${^WARNING_BITS}, "\0" x $warnings::BYTES ],
+ txt => 'all bits off via "no warnings"' };
+ }
testwarn("\0" x $warnings::BYTES, 'no bits');
use warnings;
- BEGIN { check_bits( ${^WARNING_BITS}, $default,
- 'default bits on via "use warnings"' ); }
- BEGIN { testwarn($default, 'all'); }
+ BEGIN {
+ push @tests, { type => 'check_bits', args => [ ${^WARNING_BITS}, $default ], txt => 'default bits on via "use warnings"' };
+ }
+ BEGIN {
+ push @tests, { type => 'check_bits', args => [ get_caller_0_9(), $default ], txt => 'warnings match caller' };
+ }
# run-time :
# the warning mask has been extended by warnings::register
testwarn($registered, 'ahead of w::r');
use warnings::register;
- BEGIN { check_bits( ${^WARNING_BITS}, $registered,
- 'warning bits on via "use warnings::register"' ) }
+ BEGIN {
+ push @tests, { type => 'check_bits', args => [ ${^WARNING_BITS}, $registered ], txt => 'warning bits on via "use warnings::register"' };
+ }
testwarn($registered, 'following w::r');
}
@@ -316,7 +343,7 @@ is $line, "3000000000", "check large line numbers are preserved";
# This was fixed with commit d4d03940c58a0177, which fixed bug #78742
fresh_perl_is <<'END', "__ANON__::doof\n", {},
package foo;
-BEGIN {undef %foo::}
+INIT {undef %foo::} # adjust test for B::C
sub doof { caller(0) }
print +(doof())[3];
END
@@ -357,12 +384,16 @@ do './op/caller.pl' or die $@;
package RT129239;
BEGIN {
my ($pkg, $file, $line) = caller;
- ::is $file, 'virtually/op/caller.t', "BEGIN block sees correct caller filename";
- ::is $line, 12345, "BEGIN block sees correct caller line";
- TODO: {
- local $::TODO = "BEGIN blocks have wrong caller package [perl #129239]";
- ::is $pkg, 'RT129239', "BEGIN block sees correct caller package";
- }
+# push @tests, { type => 'is', args => [ +(caller 0)[1], __FILE__ ],
+# txt => "[perl #68712] filenames after require in a BEGIN block" };
+
+ push @tests, { type => 'is', args => [ $file, 'virtually/op/caller.t' ], txt => "BEGIN block sees correct caller filename" };
+ push @tests, { type => 'is', args => [ $line, 12345 ], txt => "BEGIN block sees correct caller line" };
+ #TODO: {
+ # local $::TODO = "BEGIN blocks have wrong caller package [perl #129239]";
+ # push @tests, { type => is, args => [ $pkg, 'RT129239' ], txt => "BEGIN block sees correct caller package" };
+ #}
+ push @tests, { type => 'ok', txt => 'SKIPPING the BEGIN TODO test above' };
#line 12345 "virtually/op/caller.t"
}
}
--
2.14.1
|
From @jkeenanOn Thu, 14 Sep 2017 22:46:27 GMT, atoomic@cpan.org wrote:
Good thinking! I scanned each patch. I immediately grokked #1, 2, 3 and 6. 4 and 5 will need a second look. Can we get additional eyeballs please, especially on 4 and 5. Thank you very much. |
The RT System itself - Status changed from 'new' to 'open' |
From @cpansproutOn Thu, 14 Sep 2017 17:55:57 -0700, jkeenan wrote:
4 makes sense to me. If you want to compile the test script, dump it to a file, and then run it, you will be missing the output of the first few tests when you run it. 5 makes changes to stash.t that are wholly non-obvious. Why cannot B::C run a script that loads B? It sounds like a problem with B::C. As for 6, why does a test script need to return any value? -- Father Chrysostomos |
From @cpansprout
I should have said, ‘that loads B in a roundabout way’. I have no idea why one way of loading it works for you, while the other does not. -- Father Chrysostomos |
From @cpansproutOn Thu, 14 Sep 2017 18:00:48 -0700, sprout wrote:
Grrr. s/roundabout/straightforward/; -- Father Chrysostomos |
From @atoomicOn Thu, 14 Sep 2017 17:59:41 -0700, sprout wrote:
You are right this sounds like a problem with B::C, and I've just realized that we've already fixed it correctly, so we do not need this patch #5 anymore, we should discard it. [ should not have been part of my merge request ]
yes I agree 6 it is stupid lol we should also discard it |
From @jkeenanOn Fri, 15 Sep 2017 15:27:02 GMT, atoomic wrote:
Applied 0001 thru 0004 to blead. Rejecting 0005 and 0006. Marking ticket resolved. Thank you very much. -- |
@jkeenan - Status changed from 'open' to 'resolved' |
From @jkeenanWe have to re-open this ticket because the changes to t/op/threads-dirh.t which were part of commit 1ce8be8 earlier today are experiencing failures in threaded builds on Linux. Example: http://perl5.test-smoke.org/report/57946 tux++ for all those smoke-testing rigs. I'm currently testing a revert to just this one file and will push when it PASSes. Thank you very much. |
@jkeenan - Status changed from 'resolved' to 'open' |
From @jkeenanOn Fri, 15 Sep 2017 23:36:46 GMT, jkeenan wrote:
The reversion has been made in commit 07d51b5. Before reversion, the test failure output looked like this: ##### Test Summary Report op/threads-dirh.t (Wstat: 6400 Tests: 1 Failed: 0) On IRC, Tony Cook pointed out this commit from August 2016, which is relevant to the problem: https://perl5.git.perl.org/perl.git/commitdiff/0cf18b74ec1005ca929e64dd9158674d5163ff0a Thank you very much. -- |
From @atoomicThanks for the partial revert via 07d51b5, and catching the error on windows. thanks On Fri, 15 Sep 2017 17:00:46 -0700, jkeenan wrote:
|
From @jkeenanOn Mon, 18 Sep 2017 15:06:44 GMT, atoomic wrote:
Thanks. Note to you (as contributor) and (especially) to me (as committer): If one of the files being changed has the string 'thread' in its name, be sure to test on a threaded build! |
@jkeenan - Status changed from 'open' to 'resolved' |
From @genehack
A further note: during the course of releasing 5.27.4, I discovered that one of the commits in this set (d190dde) made changes to t/op/array_base.t that broke it, but only when running under miniperl (i.e., ‘make test’ would pass but ‘make minitest’ was failing). After discussion with Zefram in #p5p, I reverted the whole commit with commit a8b7548. I will land this shortly, with the other 5.27.4 release-related changes. Before revision, the test failure looked like: % ../miniperl -I../lib op/array_base.t Further reminder to contributors and committers: run ‘make minitest’ in addition to ‘make test’. ;^) thanks, |
@jkeenan - Status changed from 'resolved' to 'open' |
From zefram@fysh.orgJohn SJ Anderson wrote:
Specifically, there had been code in a BEGIN block to exit after the Both of the test files edited by that commit have good reason to perform -zefram |
From @jkeenanOn Wed, 20 Sep 2017 20:07:18 GMT, genehack wrote:
John, as the person who committed d190dde, I send my regrets for the time and effort that commit's problems cost you in the preparation of a monthly release. And I don't disagree with Zefram's critique of the content of that commit, though I should note that before committing that patch I requested additional eyeballs on it and got a thumbs up from one other committer. However, to expect committers to run 'make minitest' before any given commit requires further discussion. That's not cost-free; here's why. 1. Commits which run into failures during 'make minitest' but not during 'make test' or 'make test_harness' are very infrequent. A search through my perl5-porters newsgroup backlog shows that 'minitest' has not appeared in the Subject line of a message in 14 months. (In fact, I found a message from April 26 2015 in which I myself wrote that I had never heard of 'make minitest' until the previous day.) I infer from that that such failures are rare. 2. pod/perlhack.pod has nothing to say about the 'minitest' target except to define it with respect to 'miniperl' -- and that latter concept is not further discussed in this document. 3. pod/perlgit.pod does have this to say about 'minitest': ##### But this, of course, assumes that the author and committer can identify which changes in source code that "will affect miniperl or core routines that have different code paths for miniperl." Well, I concede that I cannot instantly identify such changes, so I am unlikely to be able to identify those rare circumstances that warrant a run of 'make minitest'. And I doubt I'm the only author or committer in that position. I suspect that there are fewer than a dozen people in the world who could look at a patch for perl 5 core and say, right off the bat, "That needs 'make minitest'." 4. We rely on our smoke-testers to detect problems that don't turn up in a typical author's or committer's run of 'make test'. Indeed, smoke-test reports led to the reversion of one other patch originally submitted to this ticket. Further, if you grep the source code of Test-Smoke, you do find a 'make_minitest' method. But Test-Smoke only calls this method when an overall 'make' has failed but 'make miniperl' has succeeded. So it's not surprising that Test-Smoke did not detect the problem with t/op/array_base.t. So this is a case where we have to weigh the benefit of advance detection of a relatively rare type of error against the cost of the changes to committers' work flows and (potentially) to Test-Smoke. I'm not going to dig in my heels to oppose such changes. I simply want it recognized that they will not be cost-free. Thank you very much. -- |
From @atoomicThanks for taking action there, I trend to agree to Zefram comment about the value of the commit itself, which add the end increase its maintenance cost, so reverting it is fine. I would keep in mind the "make minitest", thanks for your time and effort there, nicolas On Wed, 20 Sep 2017 14:40:41 -0700, jkeenan wrote:
|
From zefram@fysh.orgJames E Keenan via RT wrote:
The file in question contained an explicit call to is_miniperl(). -zefram |
From @cpansproutOn Wed, 20 Sep 2017 14:40:41 -0700, jkeenan wrote:
I, for one, think that occasional minitest bit-rot is perfectly fine. minitest is mainly a tool for people doing deep hacking (stuff that will break things at a low level, for which minitest is helpful in figuring out why the more complicated tests are failing) and for people porting perl to another system (who presumably are knowledgeable enough to handle it). When doing the former (I have never done the latter), I have fixed up minitest when I was about to use it. I have no problem with that. Jarkko, on the other hand, would probably disagree with me.... -- Father Chrysostomos |
From @xsawyerx[Top-posted] Jim, this work was done fully in public with all the best intentions and It's all good. :) <3 On 09/20/2017 11:40 PM, James E Keenan via RT wrote:
|
From @jkeenanSince the tests added in this RT that have not been reverted appear to be stable, I am once again marking this ticket Resolved. Thank you very much. |
@jkeenan - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#132092 (status was 'resolved')
Searchable as RT132092$
The text was updated successfully, but these errors were encountered: