Skip to content

Commit

Permalink
Fix panic when started without a stdout (#549)
Browse files Browse the repository at this point in the history
  • Loading branch information
mjameswh authored May 14, 2024
1 parent 9471d7d commit 0160df0
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 2 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
- name: Setup Go
uses: actions/setup-go@v4
with:
go-version: '1.21'
go-version: "1.22"

- name: Test
run: go test ./...
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/temporalio/cli

go 1.21
go 1.22

require (
github.com/alitto/pond v1.8.3
Expand Down
40 changes: 40 additions & 0 deletions temporalcli/internal/printer/printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ package printer_test

import (
"bytes"
"os"
"os/exec"
"runtime"
"strings"
"testing"
"unicode"
Expand Down Expand Up @@ -140,3 +143,40 @@ func TestPrinter_JSONList(t *testing.T) {
p.EndList()
require.Equal(t, "", buf.String())
}

// Asserts the printer package don't panic if the CLI is run without a STDOUT.
// This is a tricky thing to validate, as it must be done in a subprocess and as
// `go test` has its own internal fix for improper STDOUT. This was fixed in
// Go 1.22, but keeping this here as a regression test.
// See https://github.com/temporalio/cli/issues/544.
func TestPrinter_NoPanicIfNoStdout(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Skipped on Windows")
return
}

goPath, err := exec.LookPath("go")
if err != nil {
t.Fatalf("Error finding go executable: %v", err)
}
// Don't use exec.Command here, as it silently replace nil file descriptors
// with /dev/null on the parent side. We specifically want to test what
// happens when stdout is nil.
p, err := os.StartProcess(
goPath,
[]string{"go", "run", "./test/main.go"},
&os.ProcAttr{
Files: []*os.File{os.Stdin, nil, os.Stderr},
},
)
if err != nil {
t.Fatalf("Error running command: %v", err)
}
state, err := p.Wait()
if err != nil {
t.Fatalf("Error running command: %v", err)
}
if state.ExitCode() != 0 {
t.Fatalf("Error running command; exit code = %d", state.ExitCode())
}
}
21 changes: 21 additions & 0 deletions temporalcli/internal/printer/test/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package main

import (
"os"

"github.com/temporalio/cli/temporalcli/internal/printer"
)

// This main function is used to test that the printer package don't panic if
// the CLI is run without a STDOUT. This is a tricky thing to validate, as it
// must be done in a subprocess and as `go test` has its own internal fix for
// improper STDOUT. This was fixed in Go 1.22, but keeping this here as a
// regression test. See https://github.com/temporalio/cli/issues/544.
func main() {
p := &printer.Printer{
Output: os.Stdout,
JSON: false,
}
p.Println("Test writing to stdout using Printer")
os.Exit(0)
}

0 comments on commit 0160df0

Please sign in to comment.