Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testing: test output indentation looks inconsistent when tab width != 4 #25369

Closed
dpinela opened this issue May 13, 2018 · 4 comments

Comments

Projects
None yet
5 participants
@dpinela
Copy link
Contributor

commented May 13, 2018

What did you do?

Run the following program:
https://play.golang.org/p/IAMobZmXMqw

What did you expect to see?

=== RUN   TestX
=== RUN   TestX/Y
=== RUN   TestX/Y/Z
--- FAIL: TestX (0.00s)
    main.go:6: 1
    --- FAIL: TestX/Y (0.00s)
        main.go:8: 2
        --- FAIL: TestX/Y/Z (0.00s)
            main.go:10: 3
    main.go:13: 4
FAIL

This is what you see if tabs are 4 spaces wide. Note that each subtest's output is indented with 4 leading spaces, and each log message has a tab after those.

What did you see instead?

=== RUN   TestX
=== RUN   TestX/Y
=== RUN   TestX/Y/Z
--- FAIL: TestX (0.00s)
        main.go:6: 1
    --- FAIL: TestX/Y (0.00s)
        main.go:8: 2
        --- FAIL: TestX/Y/Z (0.00s)
                main.go:10: 3
        main.go:13: 4
FAIL

This is what you see if tabs are 8 spaces wide; any tab width other than 4 will cause similar inconsistencies.

System details

go version go1.10.2 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/dpinela/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/dpinela/dev/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.10.2/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.10.2/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/41/2xpv_r1j5n5bnflwb7s1hv580000gp/T/go-build030076165=/tmp/go-build -gno-record-gcc-switches -fno-common"
GOROOT/bin/go version: go version go1.10.2 darwin/amd64
GOROOT/bin/go tool compile -V: compile version go1.10.2
uname -v: Darwin Kernel Version 17.5.0: Fri Apr 13 19:32:32 PDT 2018; root:xnu-4570.51.2~1/RELEASE_X86_64
ProductName:	Mac OS X
ProductVersion:	10.13.4
BuildVersion:	17E202
lldb --version: lldb-902.0.79.2
  Swift-4.1

@dpinela dpinela changed the title testing: test output indentation looks inconsistent testing: test output indentation looks inconsistent when tab width != 8 May 13, 2018

@dpinela dpinela changed the title testing: test output indentation looks inconsistent when tab width != 8 testing: test output indentation looks inconsistent when tab width != 4 May 13, 2018

@ysmolsky

This comment has been minimized.

Copy link
Member

commented May 13, 2018

Indeed, there is a mix of tabs and 4 spaces indentations in testing package. I am not sure what was the purpose of tabs in indentation of every line since we have 4 spaces here: https://github.com/golang/go/blob/master/src/testing/testing.go#L429

A quick fix comes to mind:

diff --git a/src/testing/testing.go b/src/testing/testing.go
index 573ef05fdc..ee3bebb7ba 100644
--- a/src/testing/testing.go
+++ b/src/testing/testing.go
@@ -381,7 +381,7 @@ func (c *common) decorate(s string) string {
        }
        buf := new(strings.Builder)
        // Every line is indented at least one tab.
-       buf.WriteByte('\t')
+       buf.WriteString("    ")
        fmt.Fprintf(buf, "%s:%d: ", file, line)
        lines := strings.Split(s, "\n")
        if l := len(lines); l > 1 && lines[l-1] == "" {

But it implies fixing tests as well.

@agnivade agnivade added this to the Go1.11 milestone May 13, 2018

@gopherbot

This comment has been minimized.

Copy link

commented May 15, 2018

Change https://golang.org/cl/113177 mentions this issue: testing: make indentation consistent in sub-tests

@ysmolsky ysmolsky added NeedsFix and removed NeedsDecision labels May 15, 2018

@ysmolsky

This comment has been minimized.

Copy link
Member

commented May 15, 2018

I wonder if this change can break others programs or some Golang promises.
ping @andybons @bradfitz

@ysmolsky ysmolsky modified the milestones: Go1.11, Go1.12 May 29, 2018

@gopherbot gopherbot closed this in fffb3a5 May 31, 2018

@andybons

This comment has been minimized.

Copy link
Member

commented Jun 1, 2018

I think it’s fine. If someone is relying on the textual output of tests instead of using the JSON output, then this will be a good reason for them to switch :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.