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

[PATCH] Add support for test.valgrind parallel testing #13658

Closed
p5pRT opened this issue Mar 13, 2014 · 10 comments
Closed

[PATCH] Add support for test.valgrind parallel testing #13658

p5pRT opened this issue Mar 13, 2014 · 10 comments

Comments

@p5pRT
Copy link

p5pRT commented Mar 13, 2014

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

Searchable as RT121431$

@p5pRT
Copy link
Author

p5pRT commented Mar 13, 2014

From @wolfsage

Created by wolfsage@gmail.com

The included patch allows test.valgrind to run tests in parallel.

Valgrind output for each test will be printed out after the test
completes, with the name of the test prefixing every line.

Example usage might be​:

  TEST_JOBS=8 make test.valgrind VALGRIND='valgrind -q' 2>&1 | tee out.txt

-q is needed to ensure only *errors* are captured, otherwise the output will
be much louder than it already is. (Perhaps this should be the default mode?)

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.14.2:

Configured by Debian Project at Mon Mar 18 19:16:26 UTC 2013.

Summary of my perl5 (revision 5 version 14 subversion 2) configuration:

  Platform:
    osname=linux, osvers=2.6.42-37-generic,
archname=x86_64-linux-gnu-thread-multi
    uname='linux batsu 2.6.42-37-generic #58-ubuntu smp thu jan 24
15:28:10 utc 2013 x86_64 x86_64 x86_64 gnulinux '
    config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN
-Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr
-Dprivlib=/usr/share/perl/5.14 -Darchlib=/usr/lib/perl/5.14
-Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5
-Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local
-Dsitelib=/usr/local/share/perl/5.14.2
-Dsitearch=/usr/local/lib/perl/5.14.2 -Dman1dir=/usr/share/man/man1
-Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1
-Dsiteman3dir=/usr/local/man/man3 -Duse64bitint -Dman1ext=1
-Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh
-Ud_ualarm -Uusesfio -Uusenm -Ui_libutil -DDEBUGGING=-g -Doptimize=-O2
-Duseshrplib -Dlibperl=libperl.so.5.14.2 -des'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN
-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include
-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2 -g',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing
-pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.6.3', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t',
lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib
/usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib
    libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
    perllibs=-ldl -lm -lpthread -lc -lcrypt
    libc=, so=so, useshrplib=true, libperl=libperl.so.5.14.2
    gnulibc_version='2.15'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -g -L/usr/local/lib
-fstack-protector'

Locally applied patches:



@INC for perl 5.14.2:
    /etc/perl
    /usr/local/lib/perl/5.14.2
    /usr/local/share/perl/5.14.2
    /usr/lib/perl5
    /usr/share/perl5
    /usr/lib/perl/5.14
    /usr/share/perl/5.14
    /usr/local/lib/site_perl
    .


Environment for perl 5.14.2:
    HOME=/home/mhorsfall
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/mhorsfall/perl5/perlbrew/bin:/home/mhorsfall/bin:/home/mhorsfall/bin:/usr/lib/lightdm/lightdm:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
    PERLBREW_BASHRC_VERSION=0.66
    PERLBREW_HOME=/home/mhorsfall/.perlbrew
    PERLBREW_ROOT=/home/mhorsfall/perl5/perlbrew
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Mar 13, 2014

From @wolfsage

0001-Add-support-for-test.valgrind-to-run-in-parallel.patch
From 1821b7d8c0a5989671bd6e63e1b87a7fd02ade55 Mon Sep 17 00:00:00 2001
From: Matthew Horsfall <WolfSage@gmail.com>
Date: Thu, 13 Mar 2014 08:10:07 -0400
Subject: [PATCH] Add support for test.valgrind to run in parallel.

---
 Makefile.SH |    2 +-
 t/TEST      |   11 +++++------
 t/harness   |   20 ++++++++++++++++++++
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/Makefile.SH b/Makefile.SH
index 6e9df1a..3b5d023 100755
--- a/Makefile.SH
+++ b/Makefile.SH
@@ -1510,7 +1510,7 @@ test.valgrind check.valgrind:	test_prep
 	@grep "^usemymalloc='n'" config.sh >/dev/null || exit 1
 	@echo "And of course you have to have valgrind..."
 	$(VALGRIND) $(VG_TEST) || exit 1
-	PERL_VALGRIND=1 VALGRIND='$(VALGRIND)' $(RUN_TESTS) choose
+	PERL_VALGRIND=1 VALGRIND='$(VALGRIND)' TESTFILE=harness $(RUN_TESTS) choose
 !NO!SUBS!
 	;;
 esac
diff --git a/t/TEST b/t/TEST
index 96eb6a4..356bdc2 100755
--- a/t/TEST
+++ b/t/TEST
@@ -284,16 +284,15 @@ sub _cmd {
         if ($ENV{PERL_VALGRIND}) {
             my $perl_supp = $options->{return_dir} ? "$options->{return_dir}/perl.supp" : "perl.supp";
             my $valgrind_exe = $ENV{VALGRIND} // 'valgrind';
+            if ($options->{run_dir}) {
+                $Valgrind_Log = "$options->{run_dir}/$Valgrind_Log";
+            }
             my $vg_opts = $ENV{VG_OPTS}
-              // '--log-fd=3 '
+	       //   "--log-file=$Valgrind_Log "
 		  . "--suppressions=$perl_supp --leak-check=yes "
 		  . "--leak-resolution=high --show-reachable=yes "
-                  . "--num-callers=50 --track-origins=yes";
+		  . "--num-callers=50 --track-origins=yes";
             $perl = "$valgrind_exe $vg_opts $perl";
-            $redir = "3>$Valgrind_Log";
-            if ($options->{run_dir}) {
-                $Valgrind_Log = "$options->{run_dir}/$Valgrind_Log";
-            }
         }
 
         my $args = "$options->{testswitch} $options->{switch} $options->{utf8}";
diff --git a/t/harness b/t/harness
index 1ed70cb..845b270 100644
--- a/t/harness
+++ b/t/harness
@@ -16,6 +16,7 @@ use Config;
 
 $::do_nothing = $::do_nothing = 1;
 require './TEST';
+our $Valgrind_Log;
 
 my $Verbose = 0;
 $Verbose++ while @ARGV && $ARGV[0] eq '-v' && shift;
@@ -224,10 +225,29 @@ my $h = TAP::Harness->new({
 	    $options = $options{$test} = _scan_test($test, $type);
 	}
 
+	(local $Valgrind_Log = "$test.valgrind-current") =~ s/^.*\///;
+
 	return [ split ' ', _cmd($options, $type) ];
     },
 });
 
+# Print valgrind output after test completes
+if ($ENV{PERL_VALGRIND}) {
+    $h->callback(
+		 after_test => sub {
+		     my ($job) = @_;
+		     my $test = $job->[0];
+		     my $vfile = "$test.valgrind-current";
+	             $vfile =~ s/^.*\///;
+
+		     open(my $voutput, '<', $vfile) or return;
+		     print "$test: Valgrind output:\n";
+		     print "$test: $_" for <$voutput>;
+		     close($voutput);
+		 }
+		 );
+}
+
 if ($state) {
     $h->callback(
 		 after_test => sub {
-- 
1.7.9.5

@p5pRT
Copy link
Author

p5pRT commented Mar 15, 2014

From @wolfsage

The previous patch didn't clean out the valgrind-current files before
running tests, so output from previous test runs could interfere.

This patch fixes that.

@p5pRT
Copy link
Author

p5pRT commented Mar 15, 2014

From @wolfsage

0001-Add-support-for-test.valgrind-to-run-in-parallel.patch
From dd41612de0c6c8e66526116acba8fb98f1299609 Mon Sep 17 00:00:00 2001
From: Matthew Horsfall <WolfSage@gmail.com>
Date: Thu, 13 Mar 2014 08:10:07 -0400
Subject: [PATCH] Add support for test.valgrind to run in parallel.

---
 Makefile.SH |    2 +-
 t/TEST      |   24 +++++++++++++++++-------
 t/harness   |   20 ++++++++++++++++++++
 3 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/Makefile.SH b/Makefile.SH
index 6e9df1a..3b5d023 100755
--- a/Makefile.SH
+++ b/Makefile.SH
@@ -1510,7 +1510,7 @@ test.valgrind check.valgrind:	test_prep
 	@grep "^usemymalloc='n'" config.sh >/dev/null || exit 1
 	@echo "And of course you have to have valgrind..."
 	$(VALGRIND) $(VG_TEST) || exit 1
-	PERL_VALGRIND=1 VALGRIND='$(VALGRIND)' $(RUN_TESTS) choose
+	PERL_VALGRIND=1 VALGRIND='$(VALGRIND)' TESTFILE=harness $(RUN_TESTS) choose
 !NO!SUBS!
 	;;
 esac
diff --git a/t/TEST b/t/TEST
index 96eb6a4..d6cfb48 100755
--- a/t/TEST
+++ b/t/TEST
@@ -284,16 +284,15 @@ sub _cmd {
         if ($ENV{PERL_VALGRIND}) {
             my $perl_supp = $options->{return_dir} ? "$options->{return_dir}/perl.supp" : "perl.supp";
             my $valgrind_exe = $ENV{VALGRIND} // 'valgrind';
+            if ($options->{run_dir}) {
+                $Valgrind_Log = "$options->{run_dir}/$Valgrind_Log";
+            }
             my $vg_opts = $ENV{VG_OPTS}
-              // '--log-fd=3 '
+	       //   "--log-file=$Valgrind_Log "
 		  . "--suppressions=$perl_supp --leak-check=yes "
 		  . "--leak-resolution=high --show-reachable=yes "
-                  . "--num-callers=50 --track-origins=yes";
+		  . "--num-callers=50 --track-origins=yes";
             $perl = "$valgrind_exe $vg_opts $perl";
-            $redir = "3>$Valgrind_Log";
-            if ($options->{run_dir}) {
-                $Valgrind_Log = "$options->{run_dir}/$Valgrind_Log";
-            }
         }
 
         my $args = "$options->{testswitch} $options->{switch} $options->{utf8}";
@@ -310,6 +309,16 @@ sub _before_fork {
 	chdir $run_dir or die "Can't chdir to '$run_dir': $!";
     }
 
+    # Remove previous valgrind output otherwise it will interfere
+    my $test = $options->{test};
+
+    (local $Valgrind_Log = "$test.valgrind-current") =~ s/^.*\///;
+
+    if ($ENV{PERL_VALGRIND} && -e $Valgrind_Log) {
+        unlink $Valgrind_Log
+            or warn "$0: Failed to unlink '$Valgrind_Log': $!\n";
+    }
+
     return;
 }
 
@@ -553,7 +562,8 @@ EOT
 	    $te = '';
 	}
 
-        (local $Valgrind_Log = "$test.valgrind-current") =~ s/^.*\///;
+	(local $Valgrind_Log = "$test.valgrind-current") =~ s/^.*\///;
+
 	my $results = _run_test($test, $type);
 
 	my $failure;
diff --git a/t/harness b/t/harness
index 1ed70cb..845b270 100644
--- a/t/harness
+++ b/t/harness
@@ -16,6 +16,7 @@ use Config;
 
 $::do_nothing = $::do_nothing = 1;
 require './TEST';
+our $Valgrind_Log;
 
 my $Verbose = 0;
 $Verbose++ while @ARGV && $ARGV[0] eq '-v' && shift;
@@ -224,10 +225,29 @@ my $h = TAP::Harness->new({
 	    $options = $options{$test} = _scan_test($test, $type);
 	}
 
+	(local $Valgrind_Log = "$test.valgrind-current") =~ s/^.*\///;
+
 	return [ split ' ', _cmd($options, $type) ];
     },
 });
 
+# Print valgrind output after test completes
+if ($ENV{PERL_VALGRIND}) {
+    $h->callback(
+		 after_test => sub {
+		     my ($job) = @_;
+		     my $test = $job->[0];
+		     my $vfile = "$test.valgrind-current";
+	             $vfile =~ s/^.*\///;
+
+		     open(my $voutput, '<', $vfile) or return;
+		     print "$test: Valgrind output:\n";
+		     print "$test: $_" for <$voutput>;
+		     close($voutput);
+		 }
+		 );
+}
+
 if ($state) {
     $h->callback(
 		 after_test => sub {
-- 
1.7.9.5

@p5pRT
Copy link
Author

p5pRT commented Mar 15, 2014

From @wolfsage

While I think this patch makes parallel testing possible and testing
of valgrind faster, it abandons some of the expected behavior of
t/TEST with regards to valgrind tests. (Detecting/reporting on
failures/warnings. Renaming of output files. Etc)

I need to consider this more thoroughly (but by all means, use this
until I've submitted a more correct one, hopefully in the next
day/week.)

-- Matthew Horsfall (alh)

@p5pRT
Copy link
Author

p5pRT commented May 18, 2014

From @jkeenan

On Sat Mar 15 06​:21​:11 2014, alh wrote​:

While I think this patch makes parallel testing possible and testing
of valgrind faster, it abandons some of the expected behavior of
t/TEST with regards to valgrind tests. (Detecting/reporting on
failures/warnings. Renaming of output files. Etc)

I need to consider this more thoroughly (but by all means, use this
until I've submitted a more correct one, hopefully in the next
day/week.)

Matthew​: Will you be submitting a new patch for this ticket?

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented May 18, 2014

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

@p5pRT
Copy link
Author

p5pRT commented May 28, 2014

From @wolfsage

On Sun, May 18, 2014 at 7​:09 PM, James E Keenan via RT
<perlbug-followup@​perl.org> wrote​:

Matthew​: Will you be submitting a new patch for this ticket?

New version committed in be075ca. I need to update perlhacktips next.

-- Matthew Horsfall (alh)

@p5pRT
Copy link
Author

p5pRT commented May 28, 2014

From @wolfsage

And with 037ab3f it's documented.

Closing.

-- Matthew Horsfall (alh)

@p5pRT
Copy link
Author

p5pRT commented May 28, 2014

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant