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 <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/11080)

1 files changed, 20 insertions, 28 deletions

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<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:
@@ -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