Allow capturing process stdout and stderr interleaved

src/tests/perf_tests/glmark2Benchmark.cpp

@@ -150,7 +150,7 @@ class GLMark2Benchmark : public testing::TestWithParam<GLMark2TestParams>
         }
         args.push_back(nullptr);
 
-        ProcessHandle process(args, true, false);
+        ProcessHandle process(args, ProcessOutputCapture::StdoutOnly);
         ASSERT_TRUE(process && process->started());
         ASSERT_TRUE(process->finish());
 

src/tests/test_utils/runner/TestSuite.cpp

@@ -1433,7 +1433,7 @@ bool TestSuite::launchChildTestProcess(uint32_t batchId,
     }
 
     // Launch child process and wait for completion.
-    processInfo.process = LaunchProcess(args, true, true);
+    processInfo.process = LaunchProcess(args, ProcessOutputCapture::StdoutAndStderrSeparately);
 
     if (!processInfo.process->started())
     {

src/tests/test_utils/runner/TestSuite_unittest.cpp

@@ -82,7 +82,7 @@ class TestSuiteTest : public testing::Test
             printf("\n");
         }
 
-        ProcessHandle process(args, true, true);
+        ProcessHandle process(args, ProcessOutputCapture::StdoutAndStderrSeparately);
         EXPECT_TRUE(process->started());
         EXPECT_TRUE(process->finish());
         EXPECT_TRUE(process->finished());

util/posix/test_utils_posix.cpp

@@ -106,16 +106,22 @@ class PosixProcess : public Process
 {
   public:
     PosixProcess(const std::vector<const char *> &commandLineArgs,
-                 bool captureStdOut,
-                 bool captureStdErr)
+                 ProcessOutputCapture captureOutput)
     {
         if (commandLineArgs.empty())
         {
             return;
         }
 
+        const bool captureStdout = captureOutput != ProcessOutputCapture::Nothing;
+        const bool captureStderr =
+            captureOutput == ProcessOutputCapture::StdoutAndStderrInterleaved ||
+            captureOutput == ProcessOutputCapture::StdoutAndStderrSeparately;
+        const bool pipeStderrToStdout =
+            captureOutput == ProcessOutputCapture::StdoutAndStderrInterleaved;
+
         // Create pipes for stdout and stderr.
-        if (captureStdOut)
+        if (captureStdout)
         {
             if (pipe(mStdoutPipe.fds) != 0)
             {
@@ -128,7 +134,7 @@ class PosixProcess : public Process
                 return;
             }
         }
-        if (captureStdErr)
+        if (captureStderr && !pipeStderrToStdout)
         {
             if (pipe(mStderrPipe.fds) != 0)
             {
@@ -156,7 +162,7 @@ class PosixProcess : public Process
             // Child.  Execute the application.
 
             // Redirect stdout and stderr to the pipe fds.
-            if (captureStdOut)
+            if (captureStdout)
             {
                 if (dup2(mStdoutPipe.fds[1], STDOUT_FILENO) < 0)
                 {
@@ -164,7 +170,14 @@ class PosixProcess : public Process
                 }
                 mStdoutPipe.closeEndPoint(1);
             }
-            if (captureStdErr)
+            if (pipeStderrToStdout)
+            {
+                if (dup2(STDOUT_FILENO, STDERR_FILENO) < 0)
+                {
+                    _exit(errno);
+                }
+            }
+            else if (captureStderr)
             {
                 if (dup2(mStderrPipe.fds[1], STDERR_FILENO) < 0)
                 {
@@ -418,11 +431,9 @@ bool DeleteFile(const char *path)
     return unlink(path) == 0;
 }
 
-Process *LaunchProcess(const std::vector<const char *> &args,
-                       bool captureStdout,
-                       bool captureStderr)
+Process *LaunchProcess(const std::vector<const char *> &args, ProcessOutputCapture captureOutput)
 {
-    return new PosixProcess(args, captureStdout, captureStderr);
+    return new PosixProcess(args, captureOutput);
 }
 
 int NumberOfProcessors()

util/test_utils.cpp

@@ -65,9 +65,8 @@ ProcessHandle::ProcessHandle() : mProcess(nullptr) {}
 ProcessHandle::ProcessHandle(Process *process) : mProcess(process) {}
 
 ProcessHandle::ProcessHandle(const std::vector<const char *> &args,
-                             bool captureStdout,
-                             bool captureStderr)
-    : mProcess(LaunchProcess(args, captureStdout, captureStderr))
+                             ProcessOutputCapture captureOutput)
+    : mProcess(LaunchProcess(args, captureOutput))
 {}
 
 ProcessHandle::~ProcessHandle()

util/test_utils.h

@@ -85,12 +85,23 @@ class Process : angle::NonCopyable
     std::string mStderr;
 };
 
+enum class ProcessOutputCapture
+{
+    Nothing,
+    // Capture stdout only
+    StdoutOnly,
+    // Capture stdout, and pipe stderr to stdout
+    StdoutAndStderrInterleaved,
+    // Capture stdout and stderr separately
+    StdoutAndStderrSeparately,
+};
+
 class ProcessHandle final : angle::NonCopyable
 {
   public:
     ProcessHandle();
     ProcessHandle(Process *process);
-    ProcessHandle(const std::vector<const char *> &args, bool captureStdout, bool captureStderr);
+    ProcessHandle(const std::vector<const char *> &args, ProcessOutputCapture captureOutput);
     ~ProcessHandle();
     ProcessHandle(ProcessHandle &&other);
     ProcessHandle &operator=(ProcessHandle &&rhs);
@@ -114,9 +125,7 @@ class ProcessHandle final : angle::NonCopyable
 //
 // On success, returns a Process pointer with started() == true.
 // On failure, returns a Process pointer with started() == false.
-Process *LaunchProcess(const std::vector<const char *> &args,
-                       bool captureStdout,
-                       bool captureStderr);
+Process *LaunchProcess(const std::vector<const char *> &args, ProcessOutputCapture captureOutput);
 
 int NumberOfProcessors();
 

util/test_utils_unittest.cpp

@@ -130,13 +130,16 @@ TEST(TestUtils, MAYBE_CreateAndDeleteFileInTempDir)
 #if defined(ANGLE_PLATFORM_ANDROID)
 #    define MAYBE_RunApp DISABLED_RunApp
 #    define MAYBE_RunAppAsync DISABLED_RunAppAsync
+#    define MAYBE_RunAppAsyncRedirectStderrToStdout DISABLED_RunAppAsyncRedirectStderrToStdout
 // TODO: fuchsia support. http://anglebug.com/3161
 #elif defined(ANGLE_PLATFORM_FUCHSIA)
 #    define MAYBE_RunApp DISABLED_RunApp
 #    define MAYBE_RunAppAsync DISABLED_RunAppAsync
+#    define MAYBE_RunAppAsyncRedirectStderrToStdout DISABLED_RunAppAsyncRedirectStderrToStdout
 #else
 #    define MAYBE_RunApp RunApp
 #    define MAYBE_RunAppAsync RunAppAsync
+#    define MAYBE_RunAppAsyncRedirectStderrToStdout RunAppAsyncRedirectStderrToStdout
 #endif  // defined(ANGLE_PLATFORM_ANDROID)
 
 // Test running an external application and receiving its output
@@ -151,7 +154,7 @@ TEST(TestUtils, MAYBE_RunApp)
 
     // Test that the application can be executed.
     {
-        ProcessHandle process(args, true, true);
+        ProcessHandle process(args, ProcessOutputCapture::StdoutAndStderrSeparately);
         EXPECT_TRUE(process->started());
         EXPECT_TRUE(process->finish());
         EXPECT_TRUE(process->finished());
@@ -167,7 +170,7 @@ TEST(TestUtils, MAYBE_RunApp)
         bool setEnvDone = SetEnvironmentVar(kRunAppTestEnvVarName, kRunAppTestEnvVarValue);
         EXPECT_TRUE(setEnvDone);
 
-        ProcessHandle process(LaunchProcess(args, true, true));
+        ProcessHandle process(LaunchProcess(args, ProcessOutputCapture::StdoutAndStderrSeparately));
         EXPECT_TRUE(process->started());
         EXPECT_TRUE(process->finish());
 
@@ -191,9 +194,9 @@ TEST(TestUtils, MAYBE_RunAppAsync)
 
     std::vector<const char *> args = {executablePath.c_str(), kRunAppTestArg1, kRunAppTestArg2};
 
-    // Test that the application can be executed.
+    // Test that the application can be executed and the output is captured correctly.
     {
-        ProcessHandle process(args, true, true);
+        ProcessHandle process(args, ProcessOutputCapture::StdoutAndStderrSeparately);
         EXPECT_TRUE(process->started());
 
         constexpr double kTimeout = 3.0;
@@ -213,6 +216,39 @@ TEST(TestUtils, MAYBE_RunAppAsync)
     }
 }
 
+// Test running an external application and receiving its stdout and stderr output interleaved.
+TEST(TestUtils, MAYBE_RunAppAsyncRedirectStderrToStdout)
+{
+    std::string executablePath = GetExecutableDirectory();
+    EXPECT_NE(executablePath, "");
+    executablePath += "/";
+    executablePath += kRunAppHelperExecutable;
+
+    std::vector<const char *> args = {executablePath.c_str(), kRunAppTestArg1, kRunAppTestArg2};
+
+    // Test that the application can be executed and the output is captured correctly.
+    {
+        ProcessHandle process(args, ProcessOutputCapture::StdoutAndStderrInterleaved);
+        EXPECT_TRUE(process->started());
+
+        constexpr double kTimeout = 3.0;
+
+        Timer timer;
+        timer.start();
+        while (!process->finished() && timer.getElapsedTime() < kTimeout)
+        {
+            angle::Sleep(1);
+        }
+
+        EXPECT_TRUE(process->finished());
+        EXPECT_GT(process->getElapsedTimeSeconds(), 0.0);
+        EXPECT_EQ(std::string(kRunAppTestStdout) + kRunAppTestStderr,
+                  NormalizeNewLines(process->getStdout()));
+        EXPECT_EQ("", process->getStderr());
+        EXPECT_EQ(EXIT_SUCCESS, process->getExitCode());
+    }
+}
+
 // Verify that NumberOfProcessors returns something reasonable.
 TEST(TestUtils, NumberOfProcessors)
 {

util/test_utils_unittest_helper.cpp

@@ -44,6 +44,7 @@ int main(int argc, char **argv)
     if (env == "")
     {
         printf("%s", kRunAppTestStdout);
+        fflush(stdout);
         fprintf(stderr, "%s", kRunAppTestStderr);
     }
     else

util/windows/test_utils_win.cpp

@@ -134,8 +134,7 @@ class WindowsProcess : public Process
 {
   public:
     WindowsProcess(const std::vector<const char *> &commandLineArgs,
-                   bool captureStdOut,
-                   bool captureStdErr)
+                   ProcessOutputCapture captureOutput)
     {
         mProcessInfo.hProcess = INVALID_HANDLE_VALUE;
         mProcessInfo.hThread  = INVALID_HANDLE_VALUE;
@@ -162,10 +161,17 @@ class WindowsProcess : public Process
 
         STARTUPINFOA startInfo = {};
 
+        const bool captureStdout = captureOutput != ProcessOutputCapture::Nothing;
+        const bool captureStderr =
+            captureOutput == ProcessOutputCapture::StdoutAndStderrInterleaved ||
+            captureOutput == ProcessOutputCapture::StdoutAndStderrSeparately;
+        const bool pipeStderrToStdout =
+            captureOutput == ProcessOutputCapture::StdoutAndStderrInterleaved;
+
         // Create pipes for stdout and stderr.
         startInfo.cb        = sizeof(STARTUPINFOA);
         startInfo.hStdInput = ::GetStdHandle(STD_INPUT_HANDLE);
-        if (captureStdOut)
+        if (captureStdout)
         {
             if (!mStdoutPipe.initPipe(&securityAttribs))
             {
@@ -178,7 +184,11 @@ class WindowsProcess : public Process
             startInfo.hStdOutput = ::GetStdHandle(STD_OUTPUT_HANDLE);
         }
 
-        if (captureStdErr)
+        if (pipeStderrToStdout)
+        {
+            startInfo.hStdError = startInfo.hStdOutput;
+        }
+        else if (captureStderr)
         {
             if (!mStderrPipe.initPipe(&securityAttribs))
             {
@@ -192,7 +202,7 @@ class WindowsProcess : public Process
         }
 
 #if !defined(ANGLE_ENABLE_WINDOWS_UWP)
-        if (captureStdOut || captureStdErr)
+        if (captureStdout || captureStderr)
         {
             startInfo.dwFlags |= STARTF_USESTDHANDLES;
         }
@@ -393,11 +403,9 @@ void WriteDebugMessage(const char *format, ...)
     OutputDebugStringA(buffer.data());
 }
 
-Process *LaunchProcess(const std::vector<const char *> &args,
-                       bool captureStdout,
-                       bool captureStderr)
+Process *LaunchProcess(const std::vector<const char *> &args, ProcessOutputCapture captureOutput)
 {
-    return new WindowsProcess(args, captureStdout, captureStderr);
+    return new WindowsProcess(args, captureOutput);
 }
 
 bool GetTempDir(char *tempDirOut, uint32_t maxDirNameLen)