Skip to content
Permalink
Browse files

interp: test tty detection properly

This includes two table-driven tests to detect whether FDs 0-3 are
terminals. One uses the Go API with StdIO, without spawning a separate
process, and another uses os/exec to run the interpreter in a separate
process via TestMain.

The cases are very similar, except that with pure Go we can fake a
buffer to be a terminal via the IsTerm method. The exec method also has
real FDs 0/1/2, versus the native version's, whose global FDs 0/1/2
belong go 'go test'.

We remove the IsTerm interface too, as it was initially added mainly for
testing, and it's no longer strictly needed. We can re-add it in the
future as part of the API, if we think it's useful.

We also add github.com/creack/pty, so that we can test a pseudoterminal
in both cases. This isn't supported on Windows, so don't run the tests
there.

While at it, switch from x/crypto to x/term for IsTerminal. It's the new
home for this API, and the module is far smaller.

Finally, a few changes to the 'test -t' logic needed to be made to
properly support all of the combinations above. The mistakes in the old
code were correctly spotted by Larry Clapp.

Thanks to Larry once again for helping get the pty tests working on Mac.

Fixes #508, again.
  • Loading branch information
mvdan committed Feb 19, 2020
1 parent b86cb96 commit cc3ea662c91b9cdcc621f40b773a398d155bdb94
Showing with 149 additions and 47 deletions.
  1. +2 −2 cmd/gosh/main.go
  2. +2 −2 cmd/shfmt/main.go
  3. +2 −1 go.mod
  4. +5 −7 go.sum
  5. +0 −24 interp/interp_test.go
  6. +133 −0 interp/terminal_test.go
  7. +5 −11 interp/test.go
@@ -11,7 +11,7 @@ import (
"os"
"strings"

"golang.org/x/crypto/ssh/terminal"
"golang.org/x/term"

"mvdan.cc/sh/v3/interp"
"mvdan.cc/sh/v3/syntax"
@@ -41,7 +41,7 @@ func runAll() error {
return run(r, strings.NewReader(*command), "")
}
if flag.NArg() == 0 {
if terminal.IsTerminal(int(os.Stdin.Fd())) {
if term.IsTerminal(int(os.Stdin.Fd())) {
return runInteractive(r, os.Stdin, os.Stdout, os.Stderr)
}
return run(r, os.Stdin, "")
@@ -15,7 +15,7 @@ import (
"regexp"

"github.com/pkg/diff"
"golang.org/x/crypto/ssh/terminal"
"golang.org/x/term"
"mvdan.cc/editorconfig"

"mvdan.cc/sh/v3/fileutil"
@@ -155,7 +155,7 @@ Utilities:
color = true
} else if os.Getenv("TERM") == "dumb" {
// Equivalent to forcing color to be turned off.
} else if f, ok := out.(*os.File); ok && terminal.IsTerminal(int(f.Fd())) {
} else if f, ok := out.(*os.File); ok && term.IsTerminal(int(f.Fd())) {
color = true
}
if flag.NArg() == 0 || (flag.NArg() == 1 && flag.Arg(0) == "-") {
3 go.mod
@@ -3,14 +3,15 @@ module mvdan.cc/sh/v3
go 1.11

require (
github.com/creack/pty v1.1.9
github.com/kr/pretty v0.2.0
github.com/kr/text v0.2.0 // indirect
github.com/pkg/diff v0.0.0-20190930165518-531926345625
github.com/rogpeppe/go-internal v1.5.2
github.com/stretchr/testify v1.4.0 // indirect
golang.org/x/crypto v0.0.0-20200214034016-1d94cc7ab1c6
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e
golang.org/x/sys v0.0.0-20200217220822-9197077df867
golang.org/x/term v0.0.0-20191110171634-ad39bd3f0407
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
mvdan.cc/editorconfig v0.1.1-0.20200121172147-e40951bde157
12 go.sum
@@ -1,10 +1,12 @@
github.com/creack/pty v1.1.9 h1:uDmaGzcdjhF4i/plgjmEsriH11Y0o7RKapEf/LDaM3w=
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
github.com/kr/pretty v0.2.0 h1:s5hAObm+yFO5uHYt5dYjxi2rXrsnmRpJx4OYvIWUaQs=
github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
github.com/kr/pty v1.1.1 h1:VkoXIwSboBpnk99O/KFauAEILuNHv5DVFKZMBN/gUgw=
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
@@ -22,17 +24,13 @@ github.com/stretchr/objx v0.1.0 h1:4G4v2dO3VZwixGIRoQ5Lfboy6nUhCyYzaqnIAPPhYs4=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20200214034016-1d94cc7ab1c6 h1:Sy5bstxEqwwbYs6n0/pBuxKENqOeZUgD45Gp3Q3pqLg=
golang.org/x/crypto v0.0.0-20200214034016-1d94cc7ab1c6/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY=
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200217220822-9197077df867 h1:JoRuNIf+rpHl+VhScRQQvzbHed86tKkqwPMV34T8myw=
golang.org/x/sys v0.0.0-20200217220822-9197077df867/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/term v0.0.0-20191110171634-ad39bd3f0407 h1:5zh5atpUEdIc478E/ebrIaHLKcfVvG6dL/fGv7BcMoM=
golang.org/x/term v0.0.0-20191110171634-ad39bd3f0407/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
@@ -3345,27 +3345,3 @@ func TestMalformedPathOnWindows(t *testing.T) {
t.Fatalf("wrong output:\nwant: %q\ngot: %q", want, got)
}
}

type termBuffer struct {
bytes.Buffer
}

func (*termBuffer) IsTerm() bool { return true }

func TestRunnerTerminalStdIO(t *testing.T) {
t.Parallel()
file := parse(t, nil, `
for n in 0 1 2 3; do if [[ -t $n ]]; then echo $n; fi; done
`)

var b termBuffer
r, _ := New(StdIO(&b, &b, nil))
if err := r.Run(context.Background(), file); err != nil {
t.Fatal(err)
}

want := "0\n1\n"
if got := b.String(); got != want {
t.Fatalf("\nwant: %q\ngot: %q", want, got)
}
}
@@ -0,0 +1,133 @@
// Copyright (c) 2019, Daniel Martí <mvdan@mvdan.cc>
// See LICENSE for licensing information

// +build !windows

package interp

import (
"bufio"
"context"
"io"
"os"
"os/exec"
"strings"
"testing"

"github.com/creack/pty"
)

func TestRunnerTerminalStdIO(t *testing.T) {
t.Parallel()

tests := []struct {
name string
files func(*testing.T) (slave io.Writer, master io.Reader)
want string
}{
{"Nil", func(t *testing.T) (io.Writer, io.Reader) {
return nil, strings.NewReader("\n")
}, "\n"},
{"Pipe", func(t *testing.T) (io.Writer, io.Reader) {
pr, pw := io.Pipe()
return pw, pr
}, "end\n"},
{"Pseudo", func(t *testing.T) (io.Writer, io.Reader) {
pty, tty, err := pty.Open()
if err != nil {
t.Fatal(err)
}
return tty, pty
}, "012end\r\n"},
}
file := parse(t, nil, `
for n in 0 1 2 3; do if [[ -t $n ]]; then echo -n $n; fi; done; echo end
`)
for _, test := range tests {
test := test
t.Run(test.name, func(t *testing.T) {
t.Parallel()

slave, master := test.files(t)
// some slaves can be used as stdin too, like a tty
slaveReader, _ := slave.(io.Reader)

r, _ := New(StdIO(slaveReader, slave, slave))
go func() {
// To mimic os/exec.Cmd.Start, use a goroutine.
if err := r.Run(context.Background(), file); err != nil {
t.Error(err)
}
}()

got, err := bufio.NewReader(master).ReadString('\n')
if err != nil {
t.Fatal(err)
}
if got != test.want {
t.Fatalf("\nwant: %q\ngot: %q", test.want, got)
}
if closer, ok := slave.(io.Closer); ok {
closer.Close()
}
if closer, ok := master.(io.Closer); ok {
closer.Close()
}
})
}
}

func TestRunnerTerminalExec(t *testing.T) {
t.Parallel()
tests := []struct {
name string
start func(*testing.T, *exec.Cmd) io.Reader
want string
}{
{"Nil", func(t *testing.T, cmd *exec.Cmd) io.Reader {
if err := cmd.Start(); err != nil {
t.Fatal(err)
}
return strings.NewReader("\n")
}, "\n"},
{"Pipe", func(t *testing.T, cmd *exec.Cmd) io.Reader {
out, err := cmd.StdoutPipe()
if err != nil {
t.Fatal(err)
}
cmd.Stderr = cmd.Stdout
if err := cmd.Start(); err != nil {
t.Fatal(err)
}
return out
}, "end\n"},
{"Pseudo", func(t *testing.T, cmd *exec.Cmd) io.Reader {
pty_, err := pty.Start(cmd)
if err != nil {
t.Fatal(err)
}
return pty_
}, "012end\r\n"},
}
for _, test := range tests {
test := test
t.Run(test.name, func(t *testing.T) {
t.Parallel()

cmd := exec.Command(os.Getenv("GOSH_PROG"),
"for n in 0 1 2 3; do if [[ -t $n ]]; then echo -n $n; fi; done; echo end")
out := test.start(t, cmd)

got, err := bufio.NewReader(out).ReadString('\n')
if err != nil {
t.Fatal(err)
}
if got != test.want {
t.Fatalf("\nwant: %q\ngot: %q", test.want, string(got))
}
if err := cmd.Wait(); err != nil {
t.Fatal(err)
}
})
}
}
@@ -10,7 +10,7 @@ import (
"os/exec"
"regexp"

"golang.org/x/crypto/ssh/terminal"
"golang.org/x/term"

"mvdan.cc/sh/v3/expand"
"mvdan.cc/sh/v3/syntax"
@@ -173,19 +173,13 @@ func (r *Runner) unTest(ctx context.Context, op syntax.UnTestOperator, x string)
case 2:
f = r.stderr
}
if f, ok := f.(interface{ IsTerm() bool }); ok {
// Allow the user to set their own logic entirely.
return f.IsTerm()
}
if f, ok := f.(interface{ Fd() uintptr }); ok {
// Support Fd methods such as the one on *os.File.
fd = int(f.Fd())
}
if fd < 3 {
// Don't use the process's 0/1/2 file descriptor.
return false
return term.IsTerminal(int(f.Fd()))
}
return terminal.IsTerminal(fd)
// TODO: allow term.IsTerminal here too if running in the
// "single process" mode.
return false
case syntax.TsEmpStr:
return x == ""
case syntax.TsNempStr:

0 comments on commit cc3ea66

Please sign in to comment.
You can’t perform that action at this time.