From 6e926cd7440b819d764b5c339058429586119eec Mon Sep 17 00:00:00 2001
From: Jake Ehrlich <96448973+JakeSiFive@users.noreply.github.com>
Date: Fri, 25 Aug 2023 14:46:32 -0500
Subject: [PATCH] Make getJobStd{err,out} check job status before returning
 (#1403)

This change makes it so that getJobStdout and getJobStderr first check the job status before returning a Pass, and return a Fail if its non-zero.

To match the old behavoir getJobFailedStdout and getJobFailedStderr were added.
---
 share/wake/lib/system/job.wake | 49 ++++++++++++++++++++++++++++++++--
 tests/tests.wake               |  8 +++---
 2 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/share/wake/lib/system/job.wake b/share/wake/lib/system/job.wake
index c195a8bc9..addbfc8af 100644
--- a/share/wake/lib/system/job.wake
+++ b/share/wake/lib/system/job.wake
@@ -494,8 +494,8 @@ export def mkJobCacheRunner (hashFn: Result RunnerInput Error => Result String E
 
                 JArray (map mkVisJson readPaths)
 
-            require Pass stdout = getJobStdout job
-            require Pass stderr = getJobStderr job
+            require Pass stdout = getJobFailedStdout job
+            require Pass stderr = getJobFailedStderr job
 
             def jobCacheAddJson =
                 prettyJSON
@@ -762,21 +762,66 @@ def mapPath = match _
     Pass l = findFailFn treeOk l
 
 export def getJobStdoutRaw (job: Job): Result String Error =
+    require Exited 0 = getJobStatus job
+    else failWithError "job terminated with non-zero exit code"
+
     stdio job 1
 
 export def getJobStderrRaw (job: Job): Result String Error =
+    require Exited 0 = getJobStatus job
+    else failWithError "job terminated with non-zero exit code"
+
+    stdio job 2
+
+# Gives the full stdout of a job as a string, without any manipulation.
+# Returns the result successfully as long as the job was successfully launched
+# and closed its stdout handle at some point during its execution. This
+# generally occurs by the process simply terminating. The only case where
+# this would return a failure is if the job did not successfully launch.
+export def getJobFailedStdoutRaw (job: Job): Result String Error =
+    stdio job 1
+
+# Gives the full stderr of a job as a string, without any manipulation.
+# Returns the result successfully as long as the job was successfully launched
+# and closed its stderr handle at some point during its execution. This
+# generally occurs by the process simply terminating. The only case
+# in which this would return a failure is if the job did not successfully
+# launch.
+export def getJobFailedStderrRaw (job: Job): Result String Error =
     stdio job 2
 
+# Gives the job's stdout if the job exited with an exit
+# code of zero. The output will be manipulated to not contain
+# ANSI escape codes.
 export def getJobStdout (job: Job): Result String Error =
     require Pass stdout = getJobStdoutRaw job
 
     Pass (stdout | filterTerminalCodes)
 
+# Gives the job's stderr if the job exited with an exit
+# code of zero. The output will be manipulated to not contain
+# ANSI escape codes.
 export def getJobStderr (job: Job): Result String Error =
     require Pass stderr = getJobStderrRaw job
 
     Pass (stderr | filterTerminalCodes)
 
+# Gives the job's stdout if the job was launched successfully
+# and closed its stdout at some point. The output will be
+# manipulated to not contain ANSI escape codes.
+export def getJobFailedStdout (job: Job): Result String Error =
+    require Pass stdout = getJobFailedStdoutRaw job
+
+    Pass (stdout | filterTerminalCodes)
+
+# Gives the job's stdout if the job was launched successfully
+# and closed its stdout at some point. The output will be
+# manipulated to not contain ANSI escape codes.
+export def getJobFailedStderr (job: Job): Result String Error =
+    require Pass stderr = getJobFailedStderrRaw job
+
+    Pass (stderr | filterTerminalCodes)
+
 export def getJobInputs (job: Job): Result (List Path) Error =
     tree job 1
     | guardPath job
diff --git a/tests/tests.wake b/tests/tests.wake
index 9c7b88831..9c3a7b21b 100644
--- a/tests/tests.wake
+++ b/tests/tests.wake
@@ -155,11 +155,11 @@ export def runUnitTests _: Result Unit Error =
     def removeCarriageReturns = replace `\r` ""
 
     require Pass jobStdout =
-        testJob.getJobStdout
+        testJob.getJobFailedStdout
         | rmap removeCarriageReturns
 
     require Pass jobStderr =
-        testJob.getJobStderr
+        testJob.getJobFailedStderr
         | rmap removeCarriageReturns
 
     require Pass _ = match testJob.isJobOk
@@ -236,11 +236,11 @@ def runTest (testScript: Path): Result Unit Error =
         | runJobWith localRunner # On OS/X you cannot mount fuse within fuse
 
     require Pass jobStdout =
-        testJob.getJobStdout
+        testJob.getJobFailedStdout
         | rmap (replace `\r` "")
 
     require Pass jobStderr =
-        testJob.getJobStderr
+        testJob.getJobFailedStderr
         | rmap (replace `\r` "")
 
     require Pass _ = match shouldPass testJob.isJobOk