cmd/go: when testing, pass Stdout to test child process directly

[akyoto]

What did you do?

Run go test in a project that uses colored terminal output in the test files (e.g. scarlet).

What did you expect to see?

Colored output like in go 1.7. It worked without problems using https://github.com/fatih/color.

What did you see instead?

Colors don't work. Monochrome output.

Does this issue reproduce with the latest release (go1.7.3)?

No it doesn't. Colored output works with 1.7.3 in go test.

System details

go version go1.8beta1 linux/amd64
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/eduard/workspace"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
TERM="dumb"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build997840737=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
GOROOT/bin/go version: go version go1.8beta1 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.8beta1 X:framepointer
uname -sr: Linux 4.4.0-47-generic
Distributor ID:	Ubuntu
Description:	Ubuntu 16.04.1 LTS
Release:	16.04
Codename:	xenial
/lib/x86_64-linux-gnu/libc.so.6: GNU C Library (Ubuntu GLIBC 2.23-0ubuntu4) stable release version 2.23, by Roland McGrath et al.
gdb --version: GNU gdb (Ubuntu 7.11.1-0ubuntu1~16.04) 7.11.1

[bradfitz]

We've never explicitly supported colors or animation or anything like that.

I recommend you file a bug against https://github.com/fatih/color and then let @fatih file a concrete bug here about what we changed, if anything. It might be a bug in the color package.

[akyoto]

@bradfitz fatih/color#48

[mattn]

do you set color.NoColor = false ?

[akyoto]

@mattn: Setting it explicitly in the test files fixes the issue.

// NoColor defines if the output is colorized or not. It's dynamically set to
// false or true based on the stdout's file descriptor referring to a terminal
// or not. This is a global option and affects all colors. For more control
// over each color block use the methods DisableColor() individually.
var NoColor = !isatty.IsTerminal(os.Stdout.Fd())

Seems like the problem lies here: https://github.com/mattn/go-isatty/blob/master/isatty_linux.go

[bradfitz]

Did Go start setting TERM=dumb this cycle? There was another bug about "go bug" reporting TERM=dumb.

[mattn]

go test handle output stream from testers. So NoColor should be true on tests.

[minux]

Closing, I think there is nothing to do.

[akyoto]

Excuse me, I don't see how IsTerminal returning false on a terminal is considered correct behaviour.
There were no issues in 1.7.3.

Would you like me to file a bug report on https://github.com/mattn/go-isatty instead?

[mattn]

Please ask on golang-nuts.

[minux]

It's likely because go test buffers the test output and thus the direct output file descriptor is no longer a real tty device.

[fatih]

It's likely because go test buffers the test output and thus
the direct output file descriptor is no longer a real tty device.

I've tested this with both go1.7.4 and go1.8.beta1. In 1.7.4 the file descriptor inside a test file is a terminal, however in go1.8beta1 it's no longer the case. This is the code piece I've used to test it:

package main

import (
	"fmt"
	"os"
	"testing"

	isatty "github.com/mattn/go-isatty"
)

func TestTerm(t *testing.T) {
	isTerminal := isatty.IsTerminal(os.Stdout.Fd())
	fmt.Printf("isTerminal = %+v\n", isTerminal)
	fmt.Printf("TERM=%q\n", os.Getenv("TERM"))
}

On 1.7.4 it's:

$ go test
isTerminal = true
TERM="xterm-256color"
PASS
ok  	_/Users/fatih/test	0.006s

on 1.8beta1 it's:

$ go test
isTerminal = false
TERM="xterm-256color"
PASS
ok  	_/Users/fatih/test	0.006s

I've also printed the TERM value to see if that changes somehow during the execution, which is not the case (it's printing my own custom value I've set)

I've checked https://beta.golang.org/doc/go1.8 but couldn't find any information about this change. I've tried to search for any changes about this in the codereview mail list and https://github.com/golang/go/blob/master/doc/go1.8.txt but there was no issues that might affect this. Can anyone point me if there is any information I can check why go test buffers or the respective change that triggers this?

[mikioh]

@fatih,

Try your test with GOGC=off. When it works, the root cause probably may exist around careless system call invocation in the external package and the fix would be to call runtime.KeepAlive.

[fatih]

The same function, if executed inside a main package prints the following values:

On 1.7.4:

$ go run main.go
isTerminal = true
TERM="xterm-256color"

On 1.8beta1:

$ go run main.go
isTerminal = true
TERM="xterm-256color"

So it's only different when executed with go test

@mikioh I've set GOGC=off but it still prints false on 1.8beta1

[minux]

The problem is that cmd/go in Go 1.8 caches the output of tests, so stdout really is not a terminal anymore: $ cat a_test.go package main import ( "fmt" "os" "testing" ) func TestTerm(t *testing.T) { fmt.Println(os.Readlink("/proc/self/fd/1")) } $ go test a_test.go -v # go master === RUN TestTerm pipe:[16263977] <nil> --- PASS: TestTerm (0.00s) PASS ok command-line-arguments 0.002s $ go17 test a_test.go -v # go 1.7 === RUN TestTerm /dev/pts/17 <nil> --- PASS: TestTerm (0.00s) PASS ok command-line-arguments 0.001s $ I'm not sure we need to call out such implementation details in release notes though.

[minux]

The change that changes this is https://golang.org/cl/22341 to fix #15211.

[fatih]

@minux I think this should be a part of the release document or somehow documented. There might be projects outside that assumes stdout is a terminal. Thanks for the CL link, at least it makes sense now.

[gopherbot]

CL https://golang.org/cl/33857 mentions this issue.

[bradfitz]

We're going to revert part of the change so the test process's output is os.Stdout.

[gopherbot]

CL https://golang.org/cl/34010 mentions this issue.

[gopherbot]

CL https://golang.org/cl/35957 mentions this issue.

[gopherbot]

Change https://golang.org/cl/200106 mentions this issue: Revert "cmd/go: fail if a test binary exits with no output"

[mvdan]

The decision taken here is making it hard to make certain improvements to go test, such as #29062. In #29062 (comment) I argue why I think we should undo this decision and consistently run tests without a real terminal. I might turn that into a proposal if needed.

[mvdan]

My counter-argument above is now a proposal, in case anyone has thoughts: #34877