Save diff output for {stdout,stderr}.diff

3 files changed, 136 insertions, 61 deletions

diff --git a/build2/test/script/runner.cxx b/build2/test/script/runner.cxx
index 08d358c..6eb2447 100644
--- a/build2/test/script/runner.cxx
+++ b/build2/test/script/runner.cxx
@@ -5,7 +5,7 @@
 #include <build2/test/script/runner>
 
 #include <set>
-#include <iostream> // cerr
+#include <ios> // streamsize
 
 #include <butl/fdstream> // fdopen_mode, fdnull(), fddup()
 
@@ -45,6 +45,57 @@ namespace build2
         }
       }
 
+      // If the file exists, not empty and not larger than 4KB print it to the
+      // diag record. The file content goes from the new line and is not
+      // indented.
+      //
+      static void
+      print_file (diag_record& d, const path& p, const location& ll)
+      {
+        if (exists (p))
+        {
+          try
+          {
+            ifdstream is (p, ifdstream::in, ifdstream::badbit);
+
+            if (is.peek () != ifdstream::traits_type::eof ())
+            {
+              char buf[4096 + 1]; // Extra byte is for terminating '\0'.
+
+              // Note that the string is always '\0'-terminated with a maximum
+              // sizeof (buf) - 1 bytes read.
+              //
+              is.getline (buf, sizeof (buf), '\0');
+
+              // Print if the file fits 4KB-size buffer. Note that if it
+              // doesn't the failbit is set.
+              //
+              if (is.eof ())
+              {
+                // Suppress the trailing newline character as the diag record
+                // adds it's own one when flush.
+                //
+                streamsize n (is.gcount ());
+                assert (n > 0);
+
+                // Note that if the file contains '\0' it will also be counted
+                // by gcount(). But even in the worst case we will stay in the
+                // buffer boundaries (and so not crash).
+                //
+                if (buf[n - 1] == '\n')
+                  buf[n - 1] = '\0';
+
+                d << "\n" << buf;
+              }
+            }
+          }
+          catch (const io_error& e)
+          {
+            fail (ll) << "unable to read " << p << ": " << e.what ();
+          }
+        }
+      }
+
       // Check if the test command output matches the expected result (redirect
       // value). Noop for redirect types other than none, here_*.
       //
@@ -198,39 +249,44 @@ namespace build2
 
             try
             {
-              // Diff utility prints the differences to stdout. But for the
-              // user it is a part of the test failure diagnostics so let's
-              // redirect stdout to stderr.
+              // Save diff's stdout to a file for troubleshooting and for the
+              // optional (if not too large) printing (at the end of
+              // diagnostics).
               //
-              process p (pp, args.data (), 0, 2);
+              path ep (op + ".diff");
+              auto_fd efd;
 
               try
               {
-                if (p.wait ())
-                  return;
+                efd = fdopen (ep, fdopen_mode::out | fdopen_mode::create);
+                sp.clean ({cleanup_type::always, ep}, true);
+              }
+              catch (const io_error& e)
+              {
+                fail (ll) << "unable to write " << ep << ": " << e.what ();
+              }
 
-                // Output doesn't match the expected result.
-                //
-                diag_record d (error (ll));
-                d << pr << " " << what << " doesn't match the expected output";
+              // Diff utility prints the differences to stdout. But for the
+              // user it is a part of the test failure diagnostics so let's
+              // redirect stdout to stderr.
+              //
+              process p (pp, args.data (), 0, 2, efd.get ());
+              efd.reset ();
 
-                output_info (d, op);
-                output_info (d, opp, "expected ");
-                input_info  (d);
+              if (p.wait ())
+                return;
 
-                // Fall through.
-                //
-              }
-              catch (const io_error&)
-              {
-                // Child exit status doesn't matter. Assume the child process
-                // issued diagnostics. Just wait for the process completion.
-                //
-                p.wait (); // Check throw.
+              // Output doesn't match the expected result.
+              //
+              diag_record d (error (ll));
+              d << pr << " " << what << " doesn't match the expected output";
 
-                error (ll) << "failed to compare " << what
-                           << " with the expected output";
-              }
+              output_info (d, op);
+              output_info (d, opp, "expected ");
+              output_info (d, ep, "", " diff");
+              input_info  (d);
+
+              print_file (d, ep, ll);
 
               // Fall through.
               //
@@ -743,8 +799,9 @@ namespace build2
 
         const path& p (c.program);
 
-        // If there is no correct exit status by whatever reason then dump
-        // stderr (if cached), print the proper diagnostics and fail.
+        // If there is no correct exit status by whatever reason then print the
+        // proper diagnostics, dump stderr (if cached and not too large) and
+        // fail.
         //
         // Comparison *status >= 0 causes "always true" warning on Windows
         // where process::status_type is defined as uint32_t.
@@ -755,22 +812,6 @@ namespace build2
 
         if (!correct_status)
         {
-          // Dump cached stderr.
-          //
-          if (exists (esp))
-          {
-            try
-            {
-              ifdstream is (esp);
-              if (is.peek () != ifdstream::traits_type::eof ())
-                cerr << is.rdbuf ();
-            }
-            catch (const io_error& e)
-            {
-              fail (ll) << "unable to read " << esp << ": " << e.what ();
-            }
-          }
-
           // Fail with a proper diagnostics.
           //
           diag_record d (fail (ll));
@@ -794,6 +835,10 @@ namespace build2
 
           if (non_empty (isp, ll))
             d << info << "stdin: " << isp;
+
+          // Dump cached stderr.
+          //
+          print_file (d, esp, ll);
         }
 
         // Check if the standard outputs match expectations.
diff --git a/tests/test/script/runner/redirect.test b/tests/test/script/runner/redirect.test
index ec3c960..7aa4ec0 100644
--- a/tests/test/script/runner/redirect.test
+++ b/tests/test/script/runner/redirect.test
@@ -44,13 +44,16 @@ $b
 :
 $c <'$* -i 1 <foo >bar';
 $b 2>>~%EOE% != 0
-%.{3}
--bar
-+foo
 %testscript:1: error: \.\.[/\\]\.\.[/\\]\.\.[/\\]driver(\.exe)? stdout doesn't match the expected output%
 %  info: stdout: test[/\\]1[/\\]stdout%
 %  info: expected stdout: test[/\\]1[/\\]stdout\.orig%
+%  info: stdout diff: test[/\\]1[/\\]stdout\.diff%
 %  info: stdin: test[/\\]1[/\\]stdin%
+%--- .*%
+%\+\+\+ .*%
+%@@ .*%
+-bar
++foo
 EOE
 
 : inerr-str
@@ -141,6 +144,28 @@ EOO
 EOI
 $b
 
+: doc-fail-largediff
+:
+: Make sure that the large (>4KB) expected/real output difference is not
+: printed as a part of diagnostics.
+:
+$c <<EOI;
+s="------------------------------------------------------------------------";
+s="$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s$s";
+$* -i 1 <<"EOF" >>"EOO"
+$s
+EOF
+x$s
+EOO
+EOI
+$b 2>>~%EOE% != 0
+%testscript:3: error: \.\.[/\\]\.\.[/\\]\.\.[/\\]driver(\.exe)? stdout doesn't match the expected output%
+%  info: stdout: test[/\\]1[/\\]stdout%
+%  info: expected stdout: test[/\\]1[/\\]stdout\.orig%
+%  info: stdout diff: test[/\\]1[/\\]stdout\.diff%
+%  info: stdin: test[/\\]1[/\\]stdin%
+EOE
+
 # No-newline tests.
 #
 : no-newline-str
@@ -157,24 +182,22 @@ $b
 :
 $c <'$* -i 1 <:"foo" >"foo"';
 $b 2>>~/EOE/ != 0
-/.{3}
+/testscript:1: error: .+driver(\.exe)? stdout doesn't match the expected output/
+/.{7}
 -foo
 +foo
 \ No newline at end of file
-/testscript:1: error: .+driver(\.exe)? stdout doesn't match the expected output/
-/.{3}
 EOE
 
 : no-newline-str-fail2
 :
 $c <'$* -i 1 <"foo" >:"foo"';
 $b 2>>~/EOE/ != 0
-/.{3}
+/testscript:1: error: .+driver(\.exe)? stdout doesn't match the expected output/
+/.{7}
 -foo
 \ No newline at end of file
 +foo
-/testscript:1: error: .+driver(\.exe)? stdout doesn't match the expected output/
-/.{3}
 EOE
 
 : no-newline-doc
@@ -198,12 +221,11 @@ foo
 EOO
 EOI
 $b 2>>~/EOE/ != 0
-/.{3}
+/testscript:1: error: .+driver(\.exe)? stdout doesn't match the expected output/
+/.{7}
 -foo
 +foo
 \ No newline at end of file
-/testscript:1: error: .+driver(\.exe)? stdout doesn't match the expected output/
-/.{3}
 EOE
 
 : no-newline-doc-fail2
@@ -216,12 +238,11 @@ foo
 EOO
 EOI
 $b 2>>~/EOE/ != 0
-/.{3}
+/testscript:1: error: .+driver(\.exe)? stdout doesn't match the expected output/
+/.{7}
 -foo
 \ No newline at end of file
 +foo
-/testscript:1: error: .+driver(\.exe)? stdout doesn't match the expected output/
-/.{3}
 EOE
 
 : no-newline-empty-str-doc
diff --git a/tests/test/script/runner/status.test b/tests/test/script/runner/status.test
index 6a2c06c..fc0edff 100644
--- a/tests/test/script/runner/status.test
+++ b/tests/test/script/runner/status.test
@@ -24,12 +24,21 @@ $b
 :
 $c <'$* -s 1 == 0';
 $b 2>>~%EOE% != 0
-%testscript:1: error: \.\.[/\\]\.\.[/\\]\.\.[/\\]driver(.exe)? exit status 1 != 0%
+%testscript:1: error: \.\.[/\\]\.\.[/\\]\.\.[/\\]driver(\.exe)? exit status 1 != 0%
 EOE
 
 : ne-false
 :
 $c <'$* -s 1 != 1';
 $b 2>>~%EOE% != 0
-%testscript:1: error: \.\.[/\\]\.\.[/\\]\.\.[/\\]driver(.exe)? exit status 1 == 1%
+%testscript:1: error: \.\.[/\\]\.\.[/\\]\.\.[/\\]driver(\.exe)? exit status 1 == 1%
+EOE
+
+: error
+:
+$c <'$* -s 1 -e "Error"';
+$b 2>>~%EOE% != 0
+%testscript:1: error: \.\.[/\\]\.\.[/\\]\.\.[/\\]driver(\.exe)? exit status 1 != 0%
+%  info: stderr: test[/\\]1[/\\]stderr%
+Error
 EOE