TEST: Create test specific output directories
authorRichard Levitte <levitte@openssl.org>
Wed, 12 Feb 2020 19:22:42 +0000 (20:22 +0100)
committerRichard Levitte <levitte@openssl.org>
Tue, 18 Feb 2020 08:45:51 +0000 (09:45 +0100)
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 <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/11080)

.gitignore
CHANGES
Configurations/descrip.mms.tmpl
Configurations/unix-Makefile.tmpl
Configurations/windows-makefile.tmpl
util/perl/OpenSSL/Test.pm

index 52c845300b32e58db51bc9f3ab7f9c772b1177ca..a75914092fcb08364cd87bee6633a08cf39f51cc 100644 (file)
@@ -101,7 +101,7 @@ doc/man1/openssl-x509.pod
 /providers/fipsinstall.conf
 
 # Certain files that get created by tests on the fly
 /providers/fipsinstall.conf
 
 # Certain files that get created by tests on the fly
-/test/test-runs
+/test-runs
 /test/buildtest_*
 
 # Fuzz stuff.
 /test/buildtest_*
 
 # Fuzz stuff.
diff --git a/CHANGES b/CHANGES
index 535269d0a8bdbaecaf924baa46c65564b5ffe08d..d0f72970c87e146d174b48f56f0a852225932cfc 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -9,6 +9,11 @@
 
  Changes between 1.1.1 and 3.0.0 [xx XXX xxxx]
 
 
  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
   *) 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
index d379a8230b8a3d6f559efa0dc272f4bee7513b94..15fefd2502c0480d58172618e20cc4f99e4cc4aa 100644 (file)
       (my $x = shift) =~ s|\]$|...]|;
       $x
   }
       (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,
   }
 
   # 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}; "" -}
 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 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)"
         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
         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}; "" -}
         @ ! {- 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}; "" -}
index 6777bb8de9fc986e377450fe17c210fa2276fe5a..55c1b125735cde5a3d8c052bccd76c552f222cc0 100644 (file)
@@ -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}; "" -}
 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() -} \
          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}; "" -}
        @ : {- if ($disabled{tests}) { output_on(); } else { output_off(); } "" -}
        @echo "Tests are not supported with your chosen Configure options"
        @ : {- output_on() if !$disabled{tests}; "" -}
index 275c93ebc1683541c90b7e9e961eefc2f53a4e7f..074e8f74c5f3fd0fb0e5817e21a6b607c6da05de 100644 (file)
@@ -387,10 +387,8 @@ all: build_sw build_docs
 test: tests
 {- dependmagic('tests'); -}: build_programs_nodep build_modules_nodep
        @{- output_off() if $disabled{tests}; "" -}
 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 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
        set PERL=$(PERL)
        set OPENSSL_ENGINES=$(MAKEDIR)\engines
        set OPENSSL_MODULES=$(MAKEDIR)\providers
index 78e13523c88a89709520a6ac063f07169db08e65..75ba2d3a6356f776b22159224173cd9212e6e314 100644 (file)
@@ -149,6 +149,8 @@ sub setup {
     BAIL_OUT("setup() expects the file Configure in the source top directory")
         unless -f srctop_file("Configure");
 
     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});
 }
 
     __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.
 
 will be created if it doesn't already exist.  This happens before BLOCK
 is executed.
 
-=item B<cleanup =E<gt> 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:
 =back
 
 An example:
@@ -188,7 +184,7 @@ An example:
           is($line, qr/^OpenSSL 1\./,
              "check that we're using OpenSSL 1.x.x");
       }
           is($line, qr/^OpenSSL 1\./,
              "check that we're using OpenSSL 1.x.x");
       }
-  }, create => 1, cleanup => 1;
+  }, create => 1;
 
 =back
 
 
 =back
 
@@ -206,10 +202,6 @@ sub indir {
     $codeblock->();
 
     __cwd($reverse);
     $codeblock->();
 
     __cwd($reverse);
-
-    if ($opts{cleanup}) {
-       rmtree($subdir, { safe => 0 });
-    }
 }
 
 =over 4
 }
 
 =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;
 
 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};
 
     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};
 
     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
 };
 
 # __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.
 # 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);
 
 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);
 
     # 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
     # We put back new values carefully.  Doing the obvious
     # %directories = ( %tmp_directories )
     # will clear out any value that happens to be an absolute path