Skip to content
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

Storable's tests do not run safely in parallel #14946

Open
p5pRT opened this issue Sep 28, 2015 · 10 comments
Open

Storable's tests do not run safely in parallel #14946

p5pRT opened this issue Sep 28, 2015 · 10 comments
Assignees
Labels
dist-Storable issues in the dual-life Storable distribution

Comments

@p5pRT
Copy link

p5pRT commented Sep 28, 2015

Migrated from rt.perl.org#126213 (status was 'open')

Searchable as RT126213$

@p5pRT
Copy link
Author

p5pRT commented Sep 28, 2015

From @karenetheridge

I encountered this failure when installing Storable-2.51 (listed as
blead-upstream, bugtracker=rt.perl.org in Module​::CoreList) on perl 5.8.7 with
HARNESS_OPTIONS=j9.

Tests should be able to be run safely in parallel, by using File​::Temp or some
other mechanism to create unique temp files and directories.

  --> Working on Storable
  Fetching http​://mirrors.gossamer-threads.com/CPAN/authors/id/A/AM/AMS/Storable-2.51.tar.gz
  -> OK
  Unpacking Storable-2.51.tar.gz
  Entering Storable-2.51
  Checking configure dependencies from META.yml
  Checking if you have ExtUtils​::MakeMaker 6.58 ... Yes (7.10)
  Configuring Storable-2.51
  Running Makefile.PL
  Checking if your kit is complete...
  Looks good
  Generating a Unix-style Makefile
  Writing Makefile for Storable
  Writing MYMETA.yml and MYMETA.json
  -> OK
  Checking dependencies from MYMETA.json ...
  Checking if you have XSLoader 0 ... Yes (0.02)
  Checking if you have ExtUtils​::MakeMaker 0 ... Yes (7.10)
  Building and testing Storable-2.51
  cp Storable.pm blib/lib/Storable.pm
  Running Mkbootstrap for Storable ()
  chmod 644 "Storable.bs"
  "/Users/ether/perl5/perlbrew/perls/8.7/bin/perl" "/Users/ether/.perlbrew/libs/8.7@​std/lib/perl5/ExtUtils/xsubpp" -typemap "/Users/ether/perl5/perlbrew/perls/8.7/lib/5.8.7/ExtUtils/typemap" Storable.xs > Storable.xsc && mv Storable.xsc Storable.c
  cc -c -fno-common -DPERL_DARWIN -fno-strict-aliasing -pipe -I/opt/local/include -O3 -DVERSION=\"2.51\" -DXS_VERSION=\"2.51\" "-I/Users/ether/perl5/perlbrew/perls/8.7/lib/5.8.7/darwin-2level/CORE" Storable.c
  rm -f blib/arch/auto/Storable/Storable.bundle
  env MACOSX_DEPLOYMENT_TARGET=10.3 cc -bundle -undefined dynamic_lookup -L/opt/local/lib Storable.o -o blib/arch/auto/Storable/Storable.bundle \
  \
 
  chmod 755 blib/arch/auto/Storable/Storable.bundle
  "/Users/ether/perl5/perlbrew/perls/8.7/bin/perl" -MExtUtils​::Command​::MM -e 'cp_nonempty' -- Storable.bs blib/arch/auto/Storable/Storable.bs 644
  Manifying 1 pod document
  Running Mkbootstrap for Storable ()
  chmod 644 "Storable.bs"
  PERL_DL_NONLAZY=1 "/Users/ether/perl5/perlbrew/perls/8.7/bin/perl" "-MExtUtils​::Command​::MM" "-MTest​::Harness" "-e" "undef *Test​::Harness​::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/*.t
  t/attach_singleton.t .. ok
  # Will use Digest​::MD5
  t/circular_hook.t ..... ok
  t/canonical.t ......... ok
  t/compat01.t .......... skipped​: Test only works for 32 bit little-ending machines
  t/attach_errors.t ..... ok
  t/blessed.t ........... ok
  t/croak.t ............. ok
  t/compat06.t .......... ok
  t/dclone.t ............ ok
  t/destroy.t ........... ok
  t/forgive.t ........... ok
  can't open store​: No such file or directory at t/code.t line 140
  # Looks like you planned 63 tests but ran 16.
  # Looks like your test exited with 2 just after 16.
  t/code.t ..............
  Dubious, test returned 2 (wstat 512, 0x200)
  Failed 47/63 subtests
  t/freeze.t ............ ok
  t/interwork56.t ....... skipped​: Your IVs are no larger than your longs
  t/downgrade.t ......... ok
  t/file_magic.t ........ ok
  t/lock.t .............. ok
  t/overload.t .......... ok
  t/just_plain_nasty.t .. ok
  t/recurse.t ........... ok
  t/retrieve.t .......... ok
  t/robust.t ............ ok
  t/sig_die.t ........... ok
  t/threads.t ........... skipped​: no threads
  t/store.t ............. ok
  t/tied.t .............. ok
  t/tied_hook.t ......... ok
  t/malice.t ............ ok
  t/restrict.t .......... ok
  t/tied_items.t ........ ok
  t/utf8.t .............. ok
  t/integer.t ........... ok
  t/weak.t .............. ok
  t/utf8hash.t .......... ok

  Test Summary Report
  -------------------
  t/code.t (Wstat​: 512 Tests​: 16 Failed​: 0)
  Non-zero exit status​: 2
  Parse errors​: Bad plan. You planned 63 tests but ran 16.
  Files=34, Tests=2535, 0 wallclock secs ( 0.36 usr 0.09 sys + 1.53 cusr 0.26 csys = 2.24 CPU)
  Result​: FAIL
  Failed 1/34 test programs. 0/2535 subtests failed.
  make​: *** [test_dynamic] Error 255
  -> FAIL Installing Storable failed. See /Users/ether/.cpanm/work/1443464862.18825/build.log for details. Retry with --force to force install it.

@p5pRT
Copy link
Author

p5pRT commented Sep 28, 2015

From @eserte

Dana Pon 28. Ruj 2015, 11​:42​:27, ether reče​:

I encountered this failure when installing Storable-2.51 (listed as
blead-upstream, bugtracker=rt.perl.org in Module​::CoreList) on perl
5.8.7 with
HARNESS_OPTIONS=j9.

Tests should be able to be run safely in parallel, by using File​::Temp
or some
other mechanism to create unique temp files and directories.

--> Working on Storable
Fetching http​://mirrors.gossamer-
threads.com/CPAN/authors/id/A/AM/AMS/Storable-2.51.tar.gz
-> OK
Unpacking Storable-2.51.tar.gz
Entering Storable-2.51
Checking configure dependencies from META.yml
Checking if you have ExtUtils​::MakeMaker 6.58 ... Yes (7.10)
Configuring Storable-2.51
Running Makefile.PL
Checking if your kit is complete...
Looks good
Generating a Unix-style Makefile
Writing Makefile for Storable
Writing MYMETA.yml and MYMETA.json
-> OK
Checking dependencies from MYMETA.json ...
Checking if you have XSLoader 0 ... Yes (0.02)
Checking if you have ExtUtils​::MakeMaker 0 ... Yes (7.10)
Building and testing Storable-2.51
cp Storable.pm blib/lib/Storable.pm
Running Mkbootstrap for Storable ()
chmod 644 "Storable.bs"
"/Users/ether/perl5/perlbrew/perls/8.7/bin/perl"
"/Users/ether/.perlbrew/libs/8.7@​std/lib/perl5/ExtUtils/xsubpp"
-typemap
"/Users/ether/perl5/perlbrew/perls/8.7/lib/5.8.7/ExtUtils/typemap"
Storable.xs > Storable.xsc && mv Storable.xsc Storable.c
cc -c -fno-common -DPERL_DARWIN -fno-strict-aliasing -pipe
-I/opt/local/include -O3 -DVERSION=\"2.51\" -DXS_VERSION=\"2.51\"
"-I/Users/ether/perl5/perlbrew/perls/8.7/lib/5.8.7/darwin-2level/CORE"
Storable.c
rm -f blib/arch/auto/Storable/Storable.bundle
env MACOSX_DEPLOYMENT_TARGET=10.3 cc -bundle -undefined
dynamic_lookup -L/opt/local/lib Storable.o -o
blib/arch/auto/Storable/Storable.bundle \
\

chmod 755 blib/arch/auto/Storable/Storable.bundle
"/Users/ether/perl5/perlbrew/perls/8.7/bin/perl"
-MExtUtils​::Command​::MM -e 'cp_nonempty' -- Storable.bs
blib/arch/auto/Storable/Storable.bs 644
Manifying 1 pod document
Running Mkbootstrap for Storable ()
chmod 644 "Storable.bs"
PERL_DL_NONLAZY=1 "/Users/ether/perl5/perlbrew/perls/8.7/bin/perl" "-
MExtUtils​::Command​::MM" "-MTest​::Harness" "-e" "undef
*Test​::Harness​::Switches; test_harness(0, 'blib/lib', 'blib/arch')"
t/*.t
t/attach_singleton.t .. ok
# Will use Digest​::MD5
t/circular_hook.t ..... ok
t/canonical.t ......... ok
t/compat01.t .......... skipped​: Test only works for 32 bit little-
ending machines
t/attach_errors.t ..... ok
t/blessed.t ........... ok
t/croak.t ............. ok
t/compat06.t .......... ok
t/dclone.t ............ ok
t/destroy.t ........... ok
t/forgive.t ........... ok
can't open store​: No such file or directory at t/code.t line 140
# Looks like you planned 63 tests but ran 16.
# Looks like your test exited with 2 just after 16.
t/code.t ..............
Dubious, test returned 2 (wstat 512, 0x200)
Failed 47/63 subtests
t/freeze.t ............ ok
t/interwork56.t ....... skipped​: Your IVs are no larger than your
longs
t/downgrade.t ......... ok
t/file_magic.t ........ ok
t/lock.t .............. ok
t/overload.t .......... ok
t/just_plain_nasty.t .. ok
t/recurse.t ........... ok
t/retrieve.t .......... ok
t/robust.t ............ ok
t/sig_die.t ........... ok
t/threads.t ........... skipped​: no threads
t/store.t ............. ok
t/tied.t .............. ok
t/tied_hook.t ......... ok
t/malice.t ............ ok
t/restrict.t .......... ok
t/tied_items.t ........ ok
t/utf8.t .............. ok
t/integer.t ........... ok
t/weak.t .............. ok
t/utf8hash.t .......... ok

Test Summary Report
-------------------
t/code.t (Wstat​: 512 Tests​: 16 Failed​: 0)
Non-zero exit status​: 2
Parse errors​: Bad plan. You planned 63 tests but ran 16.
Files=34, Tests=2535, 0 wallclock secs ( 0.36 usr 0.09 sys + 1.53
cusr 0.26 csys = 2.24 CPU)
Result​: FAIL
Failed 1/34 test programs. 0/2535 subtests failed.
make​: *** [test_dynamic] Error 255
-> FAIL Installing Storable failed. See
/Users/ether/.cpanm/work/1443464862.18825/build.log for details. Retry
with --force to force install it.

Attached patch is not complete (I think both store.t and forgive.t also use "store" as a common file, maybe others, too). But at least it should fix my "sin".

Regards,
  Slaven

@p5pRT
Copy link
Author

p5pRT commented Sep 28, 2015

From @eserte

0001-Use-File-Temp-in-code.t.patch
From c1e1036bc567ab606b7be622d5564a585a0bdb34 Mon Sep 17 00:00:00 2001
From: Slaven Rezic <slaven@rezic.de>
Date: Mon, 28 Sep 2015 21:11:09 +0200
Subject: [PATCH] Use File::Temp in code.t

This (partially) addresses RT#126213.
---
 dist/Storable/t/code.t | 47 +++++++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/dist/Storable/t/code.t b/dist/Storable/t/code.t
index 7fc40ba..bbe7756 100644
--- a/dist/Storable/t/code.t
+++ b/dist/Storable/t/code.t
@@ -1,6 +1,6 @@
 #!./perl
 #
-#  Copyright (c) 2002 Slaven Rezic
+#  Copyright (c) 2002,2015 Slaven Rezic
 #
 #  You may redistribute only under the same terms as Perl 5, as specified
 #  in the README file that comes with the distribution.
@@ -77,6 +77,8 @@ local *FOO;
 $Storable::Deparse = 1;
 $Storable::Eval    = 1;
 
+my $File_Temp_available = eval { require File::Temp; File::Temp->VERSION(0.14) }; # 0.14 introduced OO interface
+
 ######################################################################
 # Test freeze & thaw
 
@@ -125,26 +127,39 @@ is($new_sub->(), $obj[2]->());
 ######################################################################
 # Test retrieve & store
 
-store $obj[0], 'store';
-$thawed = retrieve 'store';
+SKIP: {
+    skip "File::Temp is not available", 5
+        if !$File_Temp_available;
 
-is($thawed->[0]->(), "JAPH");
-is($thawed->[1]->(), 42);
-is($thawed->[2]->(), "blessed");
-is($thawed->[3]->(), "Another::Package");
-is(prototype($thawed->[4]), prototype($obj[0]->[4]));
+    my $temp = File::Temp->new;
+    store $obj[0], $temp;
+    $thawed = retrieve $temp;
+    unlink $temp;
+
+    is($thawed->[0]->(), "JAPH");
+    is($thawed->[1]->(), 42);
+    is($thawed->[2]->(), "blessed");
+    is($thawed->[3]->(), "Another::Package");
+    is(prototype($thawed->[4]), prototype($obj[0]->[4]));
+}
 
 ######################################################################
 
-nstore $obj[0], 'store';
-$thawed = retrieve 'store';
-unlink 'store';
+SKIP: {
+    skip "File::Temp is not available", 5
+        if !$File_Temp_available;
 
-is($thawed->[0]->(), "JAPH");
-is($thawed->[1]->(), 42);
-is($thawed->[2]->(), "blessed");
-is($thawed->[3]->(), "Another::Package");
-is(prototype($thawed->[4]), prototype($obj[0]->[4]));
+    my $temp = File::Temp->new;
+    nstore $obj[0], $temp;
+    $thawed = retrieve $temp;
+    unlink $temp;
+
+    is($thawed->[0]->(), "JAPH");
+    is($thawed->[1]->(), 42);
+    is($thawed->[2]->(), "blessed");
+    is($thawed->[3]->(), "Another::Package");
+    is(prototype($thawed->[4]), prototype($obj[0]->[4]));
+}
 
 ######################################################################
 # Security with
-- 
2.1.2

@p5pRT
Copy link
Author

p5pRT commented Sep 28, 2015

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

@p5pRT
Copy link
Author

p5pRT commented Sep 28, 2015

From @jkeenan

On Mon Sep 28 11​:42​:27 2015, ether wrote​:

I encountered this failure when installing Storable-2.51 (listed as
blead-upstream, bugtracker=rt.perl.org in Module​::CoreList) on perl
5.8.7 with
HARNESS_OPTIONS=j9.

Tests should be able to be run safely in parallel, by using File​::Temp
or some
other mechanism to create unique temp files and directories.

Here's a grep that may be helpful​:

$ grep -nE "'(fetch|store)'" dist/Storable/t/*.t
dist/Storable/t/code.t​:128​:store $obj[0], 'store';
dist/Storable/t/code.t​:129​:$thawed = retrieve 'store';
dist/Storable/t/code.t​:139​:nstore $obj[0], 'store';
dist/Storable/t/code.t​:140​:$thawed = retrieve 'store';
dist/Storable/t/code.t​:141​:unlink 'store';
dist/Storable/t/forgive.t​:39​:eval {$result = store ($bad , 'store')};
dist/Storable/t/forgive.t​:51​:eval {$result = store ($bad , 'store')};
dist/Storable/t/forgive.t​:58​:my $ret = retrieve('store');
dist/Storable/t/forgive.t​:65​:END { 1 while unlink 'store' }
dist/Storable/t/lock.t​:36​:isnt(lock_store(\@​a, 'store'), undef);
dist/Storable/t/lock.t​:40​:$root = lock_retrieve('store');
dist/Storable/t/retrieve.t​:32​:isnt(store(\@​a, 'store'), undef);
dist/Storable/t/retrieve.t​:38​:$root = retrieve('store');
dist/Storable/t/retrieve.t​:57​:END { 1 while unlink('store', 'nstore') }
dist/Storable/t/store.t​:32​:isnt(store(\@​a, 'store'), undef);
dist/Storable/t/store.t​:37​:$root = retrieve('store');
dist/Storable/t/store.t​:45​:1 while unlink 'store';
dist/Storable/t/store.t​:58​:isnt($foo->store('store'), undef);
dist/Storable/t/store.t​:69​:isnt(open(OUT, 'store'), undef);
dist/Storable/t/store.t​:91​:END { 1 while unlink 'store' }

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Sep 29, 2015

From @bulk88

On Mon Sep 28 12​:15​:23 2015, slaven@​rezic.de wrote​:

Attached patch is not complete (I think both store.t and forgive.t
also use "store" as a common file, maybe others, too). But at least it
should fix my "sin".

Regards,
Slaven

That looks flakey. What if an exception is thrown inside the "+my $File_Temp_available = eval { require File​::Temp; File​::Temp->VERSION(0.14) }; # 0.14 introduced OO interface"? What about old File​::Temps, then skip the tests? Why not just delete the tests entirely since that is silently being done on some old configs? Perhaps appending the current PID is more reliable across all perls, old and new.

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented Sep 29, 2015

From @ilmari

"bulk88 via RT" <perlbug-followup@​perl.org> writes​:

On Mon Sep 28 12​:15​:23 2015, slaven@​rezic.de wrote​:

Attached patch is not complete (I think both store.t and forgive.t
also use "store" as a common file, maybe others, too). But at least it
should fix my "sin".

Regards,
Slaven

That looks flakey. What if an exception is thrown inside the "+my
$File_Temp_available = eval { require File​::Temp;
File​::Temp->VERSION(0.14) }; # 0.14 introduced OO interface"?

Data point​: File​::Temp 0.14 was shipped with perl 5.6.2.

What about old File​::Temps, then skip the tests? Why not just delete
the tests entirely since that is silently being done on some old
configs?

You're saying because a test won't be run on a version of perl released
nearly 15 years ago it has no value and shold be deleted?

Is anyone still using perls older than that? Are they likely to be
upgrading Storable? Does Storable even work on such ancient perls? The
highest version with a pass¹ on anything older than 5.6.2 is 2.21,
released six years ago.

[1] http​://matrix.cpantesters.org/?dist=Storable;maxver=1

--
"A disappointingly low fraction of the human race is,
at any given time, on fire." - Stig Sandbeck Mathisen

@p5pRT
Copy link
Author

p5pRT commented Sep 29, 2015

From @bulk88

On Tue Sep 29 01​:12​:13 2015, ilmari wrote​:

"bulk88 via RT" <perlbug-followup@​perl.org> writes​:

On Mon Sep 28 12​:15​:23 2015, slaven@​rezic.de wrote​:

Attached patch is not complete (I think both store.t and forgive.t
also use "store" as a common file, maybe others, too). But at least it
should fix my "sin".

Regards,
Slaven

That looks flakey. What if an exception is thrown inside the "+my
$File_Temp_available = eval { require File​::Temp;
File​::Temp->VERSION(0.14) }; # 0.14 introduced OO interface"?

Data point​: File​::Temp 0.14 was shipped with perl 5.6.2.

What about old File​::Temps, then skip the tests? Why not just delete
the tests entirely since that is silently being done on some old
configs?

You're saying because a test won't be run on a version of perl released
nearly 15 years ago it has no value and shold be deleted?

Then properly drop support for that old perl (<5.6.2 or <0.14), with a hard error. Not a silent false positive (skipping tests).

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented Sep 29, 2015

From @eserte

"bulk88 via RT" <perlbug-followup@​perl.org> writes​:

On Tue Sep 29 01​:12​:13 2015, ilmari wrote​:

"bulk88 via RT" <perlbug-followup@​perl.org> writes​:

On Mon Sep 28 12​:15​:23 2015, slaven@​rezic.de wrote​:

Attached patch is not complete (I think both store.t and forgive.t
also use "store" as a common file, maybe others, too). But at least it
should fix my "sin".

Regards,
Slaven

That looks flakey. What if an exception is thrown inside the "+my
$File_Temp_available = eval { require File​::Temp;
File​::Temp->VERSION(0.14) }; # 0.14 introduced OO interface"?

Data point​: File​::Temp 0.14 was shipped with perl 5.6.2.

What about old File​::Temps, then skip the tests? Why not just delete
the tests entirely since that is silently being done on some old
configs?

You're saying because a test won't be run on a version of perl released
nearly 15 years ago it has no value and shold be deleted?

Then properly drop support for that old perl (<5.6.2 or <0.14), with a hard error. Not a silent false positive (skipping tests).

Storable is a dual-life module and apparently has no minimum perl
version, and does not depend on the existence of File​::Temp. This commit
will not change this. Also the goal of this test isn't to check if
File​::Temp works at all. Either it's there and loads, or it's not there.

If it's possible to detect that this test run happens within a perl
build directory (and thus it's likely that File​::Temp is available),
then this condition could be rewritten. Can PERL_CORE be used for this?
Then it would be​:

  $File_Temp_available = $ENV{PERL_CORE} || eval { require File​::Temp; File​::Temp->VERSION(0.14) }

Regards,
  Slaven

--
Slaven Rezic - slaven <at> rezic <dot> de

  Berlin Perl Mongers - http​://berlin.pm.org

@p5pRT
Copy link
Author

p5pRT commented Aug 6, 2019

From @tonycoz

On Mon, 28 Sep 2015 11​:42​:27 -0700, ether wrote​:

I encountered this failure when installing Storable-2.51 (listed as
blead-upstream, bugtracker=rt.perl.org in Module​::CoreList) on perl
5.8.7 with
HARNESS_OPTIONS=j9.

Tests should be able to be run safely in parallel, by using File​::Temp
or some
other mechanism to create unique temp files and directories.

I believe this is no longer a problem.

All tests either use a name with the pid included or a name specific to that test files (since perl 5.28/Storable 3.06).

Tony

@karenetheridge karenetheridge added the dist-Storable issues in the dual-life Storable distribution label Jan 31, 2020
@karenetheridge karenetheridge self-assigned this Jan 31, 2020
@xenu xenu removed the Severity Low label Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dist-Storable issues in the dual-life Storable distribution
Projects
None yet
Development

No branches or pull requests

3 participants