Skip to content

Commit

Permalink
testament: don't close Process before reading exit code (#1130)
Browse files Browse the repository at this point in the history
## Summary

On Windows Testament now correctly captures the exit code from commands
(compile, test run, etc).

## Details

On platforms like Windows closing a process before reading the exit code
invalidates the exit code, returning 0, which would falsely signify a
success.

Instead, use  `defer`  to make sure that the object is kept alive until
the end of scope.

---------

Co-authored-by: Saem Ghani <[email protected]>
  • Loading branch information
alaviss and saem authored Jan 23, 2024
1 parent 33ed520 commit 99a0ca7
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion testament/testament.nim
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,8 @@ proc callNimCompiler(cmd: string): CompilerOutput =
verboseCmd(cmd)
var p = startProcess(command = cmd,
options = {poStdErrToStdOut, poUsePath, poEvalCommand})
# windows requires reading the the exit code (below) prior to closing
defer: close(p)
let outp = p.outputStream
var foundSuccessMsg = false
var foundErrorMsg = false
Expand All @@ -418,7 +420,6 @@ proc callNimCompiler(cmd: string): CompilerOutput =
foundSuccessMsg = true
elif not running(p):
break
close(p)
result.msg = ""
result.file = ""
result.output = ""
Expand Down

0 comments on commit 99a0ca7

Please sign in to comment.