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
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dpinela
Copy link
Contributor

dpinela 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
@ysmolski
Copy link
Member

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.

@ysmolski ysmolski added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label May 13, 2018
@agnivade agnivade added this to the Go1.11 milestone May 13, 2018
@gopherbot
Copy link

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

@ysmolski ysmolski added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels May 15, 2018
@ysmolski
Copy link
Member

ysmolski commented May 15, 2018

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

@ysmolski ysmolski modified the milestones: Go1.11, Go1.12 May 29, 2018
@andybons
Copy link
Member

andybons 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 subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants