From f0790d4d2f92373f68f5b3097eb326505c937831 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Wed, 12 Feb 2020 20:22:42 +0100 Subject: [PATCH] TEST: Create test specific output directories We had all tests run with test/test-runs/ as working directory, and tests cleaned up after themselves... which is well and good, until you want to have a look at what went wrong when a complex test fails, and you have to recreate everything it does manually. To remedy this, we have OpenSSL::Test create the result directory dynamically (and cleaning it up first if it's already there) and let the test recipe have that as working directory. Test recipes are now encouraged to name their diverse output files uniquely, and not to clean them up, to allow a developer to have a look at the files that were produced. With continuous integration that allows this, the result directories could also be archived and be left as a build artifact. Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/11080) --- .gitignore | 2 +- CHANGES | 5 +++ Configurations/descrip.mms.tmpl | 12 ------- Configurations/unix-Makefile.tmpl | 13 +++----- Configurations/windows-makefile.tmpl | 2 -- util/perl/OpenSSL/Test.pm | 48 ++++++++++++---------------- 6 files changed, 31 insertions(+), 51 deletions(-) diff --git a/.gitignore b/.gitignore index 52c845300b..a75914092f 100644 --- a/.gitignore +++ b/.gitignore @@ -101,7 +101,7 @@ doc/man1/openssl-x509.pod /providers/fipsinstall.conf # Certain files that get created by tests on the fly -/test/test-runs +/test-runs /test/buildtest_* # Fuzz stuff. diff --git a/CHANGES b/CHANGES index 535269d0a8..d0f72970c8 100644 --- a/CHANGES +++ b/CHANGES @@ -9,6 +9,11 @@ Changes between 1.1.1 and 3.0.0 [xx XXX xxxx] + *) The test suite is changed to preserve results of each test recipe. + A new directory test-runs/ with subdirectories named like the + test recipes are created in the build tree for this purpose. + [Richard Levitte] + *) X509 certificates signed using SHA1 are no longer allowed at security level 1 and above. In TLS/SSL the default security level is 1. It can be set either diff --git a/Configurations/descrip.mms.tmpl b/Configurations/descrip.mms.tmpl index d379a8230b..15fefd2502 100644 --- a/Configurations/descrip.mms.tmpl +++ b/Configurations/descrip.mms.tmpl @@ -28,14 +28,6 @@ (my $x = shift) =~ s|\]$|...]|; $x } - sub move { - my $f = catdir(@_); - my $b = abs2rel(rel2abs("."),rel2abs($f)); - $sourcedir = catdir($b,$sourcedir) - if !file_name_is_absolute($sourcedir); - $builddir = catdir($b,$builddir) - if !file_name_is_absolute($builddir); - ""; } # Because we need to make two computations of these data, @@ -439,11 +431,8 @@ all : build_sw build_docs test : tests {- dependmagic('tests'); -} : build_programs_nodep, build_modules_nodep @ ! {- output_off() if $disabled{tests}; "" -} - SET DEFAULT [.test]{- move("test") -} - CREATE/DIR [.test-runs] DEFINE SRCTOP {- sourcedir() -} DEFINE BLDTOP {- builddir() -} - DEFINE RESULT_D {- builddir(qw(test test-runs)) -} DEFINE OPENSSL_ENGINES {- builddir("engines") -} DEFINE OPENSSL_MODULES {- builddir("providers") -} IF "$(VERBOSE)" .NES. "" THEN DEFINE VERBOSE "$(VERBOSE)" @@ -452,7 +441,6 @@ test : tests DEASSIGN OPENSSL_ENGINES DEASSIGN BLDTOP DEASSIGN SRCTOP - SET DEFAULT [-]{- move("..") -} @ ! {- if ($disabled{tests}) { output_on(); } else { output_off(); } "" -} @ WRITE SYS$OUTPUT "Tests are not supported with your chosen Configure options" @ ! {- output_on() if !$disabled{tests}; "" -} diff --git a/Configurations/unix-Makefile.tmpl b/Configurations/unix-Makefile.tmpl index 6777bb8de9..55c1b12573 100644 --- a/Configurations/unix-Makefile.tmpl +++ b/Configurations/unix-Makefile.tmpl @@ -464,16 +464,13 @@ all: build_sw build_docs test: tests {- dependmagic('tests'); -}: build_programs_nodep build_modules_nodep link-utils @ : {- output_off() if $disabled{tests}; "" -} - ( cd test; \ - mkdir -p test-runs; \ - SRCTOP=../$(SRCDIR) \ - BLDTOP=../$(BLDDIR) \ - RESULT_D=test-runs \ + ( SRCTOP=$(SRCDIR) \ + BLDTOP=$(BLDDIR) \ PERL="$(PERL)" \ EXE_EXT={- platform->binext() -} \ - OPENSSL_ENGINES=`cd ../$(BLDDIR)/engines 2>/dev/null && pwd` \ - OPENSSL_MODULES=`cd ../$(BLDDIR)/providers 2>/dev/null && pwd` \ - $(PERL) ../$(SRCDIR)/test/run_tests.pl $(TESTS) ) + OPENSSL_ENGINES=`cd $(BLDDIR)/engines 2>/dev/null && pwd` \ + OPENSSL_MODULES=`cd $(BLDDIR)/providers 2>/dev/null && pwd` \ + $(PERL) $(SRCDIR)/test/run_tests.pl $(TESTS) ) @ : {- if ($disabled{tests}) { output_on(); } else { output_off(); } "" -} @echo "Tests are not supported with your chosen Configure options" @ : {- output_on() if !$disabled{tests}; "" -} diff --git a/Configurations/windows-makefile.tmpl b/Configurations/windows-makefile.tmpl index 275c93ebc1..074e8f74c5 100644 --- a/Configurations/windows-makefile.tmpl +++ b/Configurations/windows-makefile.tmpl @@ -387,10 +387,8 @@ all: build_sw build_docs test: tests {- dependmagic('tests'); -}: build_programs_nodep build_modules_nodep @{- output_off() if $disabled{tests}; "" -} - -mkdir $(BLDDIR)\test\test-runs set SRCTOP=$(SRCDIR) set BLDTOP=$(BLDDIR) - set RESULT_D=$(BLDDIR)\test\test-runs set PERL=$(PERL) set OPENSSL_ENGINES=$(MAKEDIR)\engines set OPENSSL_MODULES=$(MAKEDIR)\providers diff --git a/util/perl/OpenSSL/Test.pm b/util/perl/OpenSSL/Test.pm index 78e13523c8..75ba2d3a63 100644 --- a/util/perl/OpenSSL/Test.pm +++ b/util/perl/OpenSSL/Test.pm @@ -149,6 +149,8 @@ sub setup { BAIL_OUT("setup() expects the file Configure in the source top directory") unless -f srctop_file("Configure"); + note "The results of this test will end up in $directories{RESULTS}"; + __cwd($directories{RESULTS}); } @@ -170,12 +172,6 @@ When set to 1 (or any value that perl perceives as true), the subdirectory will be created if it doesn't already exist. This happens before BLOCK is executed. -=item B 0|1> - -When set to 1 (or any value that perl perceives as true), the subdirectory -will be cleaned out and removed. This happens both before and after BLOCK -is executed. - =back An example: @@ -188,7 +184,7 @@ An example: is($line, qr/^OpenSSL 1\./, "check that we're using OpenSSL 1.x.x"); } - }, create => 1, cleanup => 1; + }, create => 1; =back @@ -206,10 +202,6 @@ sub indir { $codeblock->(); __cwd($reverse); - - if ($opts{cleanup}) { - rmtree($subdir, { safe => 0 }); - } } =over 4 @@ -943,17 +935,22 @@ i.e. Some tests may only work in non FIPS mode. sub __env { (my $recipe_datadir = basename($0)) =~ s/\.t$/_data/i; - $directories{SRCTOP} = abs_path($ENV{SRCTOP} || $ENV{TOP}); - $directories{BLDTOP} = abs_path($ENV{BLDTOP} || $ENV{TOP}); - $directories{BLDAPPS} = $ENV{BIN_D} || __bldtop_dir("apps"); - $directories{SRCAPPS} = __srctop_dir("apps"); - $directories{BLDFUZZ} = __bldtop_dir("fuzz"); - $directories{SRCFUZZ} = __srctop_dir("fuzz"); - $directories{BLDTEST} = $ENV{TEST_D} || __bldtop_dir("test"); - $directories{SRCTEST} = __srctop_dir("test"); - $directories{SRCDATA} = __srctop_dir("test", "recipes", - $recipe_datadir); - $directories{RESULTS} = $ENV{RESULT_D} || $directories{BLDTEST}; + $directories{SRCTOP} = abs_path($ENV{SRCTOP} || $ENV{TOP}); + $directories{BLDTOP} = abs_path($ENV{BLDTOP} || $ENV{TOP}); + $directories{BLDAPPS} = $ENV{BIN_D} || __bldtop_dir("apps"); + $directories{SRCAPPS} = __srctop_dir("apps"); + $directories{BLDFUZZ} = __bldtop_dir("fuzz"); + $directories{SRCFUZZ} = __srctop_dir("fuzz"); + $directories{BLDTEST} = $ENV{TEST_D} || __bldtop_dir("test"); + $directories{SRCTEST} = __srctop_dir("test"); + $directories{SRCDATA} = __srctop_dir("test", "recipes", + $recipe_datadir); + $directories{RESULTTOP} = $ENV{RESULT_D} || __bldtop_dir("test-runs"); + $directories{RESULTS} = catdir($directories{RESULTTOP}, $test_name); + + # Create result directory dynamically + rmtree($directories{RESULTS}, { safe => 0, keep_root => 1 }); + mkpath($directories{RESULTS}); push @direnv, "TOP" if $ENV{TOP}; push @direnv, "SRCTOP" if $ENV{SRCTOP}; @@ -962,7 +959,7 @@ sub __env { push @direnv, "TEST_D" if $ENV{TEST_D}; push @direnv, "RESULT_D" if $ENV{RESULT_D}; - $end_with_bailout = $ENV{STOPTEST} ? 1 : 0; + $end_with_bailout = $ENV{STOPTEST} ? 1 : 0; }; # __srctop_file and __srctop_dir are helpers to build file and directory @@ -1079,7 +1076,6 @@ sub __results_file { # hash style arguments to alter __cwd's behavior: # # create = 0|1 The directory we move to is created if 1, not if 0. -# cleanup = 0|1 The directory we move from is removed if 1, not if 0. sub __cwd { my $dir = catdir(shift); @@ -1137,10 +1133,6 @@ sub __cwd { # Should we just bail out here as well? I'm unsure. return undef unless chdir($dir); - if ($opts{cleanup}) { - rmtree(".", { safe => 0, keep_root => 1 }); - } - # We put back new values carefully. Doing the obvious # %directories = ( %tmp_directories ) # will clear out any value that happens to be an absolute path -- 2.34.1