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

all: multiple packages fail go test when not in short mode #24464

Closed
alexd765 opened this issue Mar 20, 2018 · 28 comments
Closed

all: multiple packages fail go test when not in short mode #24464

alexd765 opened this issue Mar 20, 2018 · 28 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@alexd765
Copy link
Contributor

alexd765 commented Mar 20, 2018

What version of Go are you using (go version)?

go version devel +be371edd67 Tue Mar 20 19:38:06 2018 +0000 linux/amd64

What operating system and processor architecture are you using (go env)?

linux/amd64

What did you do?

Test all packages via all.bash (in short mode) and then via go test ./... (not in short mode)

~/gotip/src$ ./all.bash && go test ./...

Inspired by #24436

What did you expect to see?

All tests pass (in short mode)
All tests pass (not in short mode)

What did you see instead?

All tests pass (in short mode)
Multiple packages fail go test (not in short mode)

FAIL cmd/compile/internal/ssa (discussed here)
FAIL cmd/doc (fixed in cl 101836)
FAIL cmd/go (#24436)
FAIL cmd/gofmt (#24472)
FAIL go/types (fixed in cl 101815)
FAIL net

Edit: i have added links to the fixes and other discussions so far

@andybons andybons added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 20, 2018
@andybons andybons added this to the Unplanned milestone Mar 20, 2018
@andybons
Copy link
Member

@rsc

@gopherbot
Copy link

Change https://golang.org/cl/101815 mentions this issue: cmd/trace: remove unused variable in tests

gopherbot pushed a commit that referenced this issue Mar 21, 2018
Unused variables in closures are currently not diagnosed by the
compiler (this is Issue #3059), while go/types catches them.

One unused variable in the cmd/trace tests is causing the go/types
test that typechecks the whole standard library to fail:

  FAIL: TestStdlib (8.05s)
    stdlib_test.go:223: cmd/trace/annotations_test.go:241:6: gcTime
    declared but not used
  FAIL

Remove it.

Updates #24464

Change-Id: I0f1b9db6ae1f0130616ee649bdbfdc91e38d2184
Reviewed-on: https://go-review.googlesource.com/101815
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
@ALTree
Copy link
Member

ALTree commented Mar 21, 2018

go/types failure fixed in 041c5d8.

@gopherbot
Copy link

Change https://golang.org/cl/101835 mentions this issue: test: fmt fixedbugs/issue22662.go

@mvdan
Copy link
Member

mvdan commented Mar 21, 2018

The failure in cmd/doc is because it may fail depending on your GOPATH. For example:

$ go test
2018/03/21 12:26:36 no buildable Go source files in /home/mvdan/go/land/src/github.com/gopherjs/gopherjs/compiler/natives/src/crypto/rand
exit status 1
FAIL    cmd/doc 0.119s
$ GOPATH= go test
PASS
ok      cmd/doc 1.857s

The tests should be made more robust. Will give it a go, but there are many ways to fix this.

@gopherbot
Copy link

Change https://golang.org/cl/101836 mentions this issue: cmd/doc: use empty GOPATH when running the tests

@ALTree
Copy link
Member

ALTree commented Mar 21, 2018

The cmd/gofmt failure is caused by gofmt not being idempotent on certain programs. I opened #24472 about this, with a small reproducer.

gopherbot pushed a commit that referenced this issue Mar 21, 2018
Otherwise, a populated GOPATH might result in failures such as:

	$ go test
	[...] no buildable Go source files in [...]/gopherjs/compiler/natives/src/crypto/rand
	exit status 1

Move the initialization of the dirs walker out of the init func, so that
we can control its behavior in the tests.

Updates #24464.

Change-Id: I4b26a7d3d6809bdd8e9b6b0556d566e7855f80fe
Reviewed-on: https://go-review.googlesource.com/101836
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@randall77
Copy link
Contributor

I can't get cmd/compile/internal/ssa to fail.

$ git checkout be371edd677abe6de310c9ffc225b9e8b052d2b8
$ ./make.bash
Building Go cmd/dist using /Users/khr/go1.4.
Building Go toolchain1 using /Users/khr/go1.4.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for darwin/amd64.
---
Installed Go for darwin/amd64 in /Users/khr/sandbox/readonly
Installed commands in /Users/khr/sandbox/readonly/bin
$ ../bin/go test cmd/compile/internal/ssa/...
ok  	cmd/compile/internal/ssa	71.866s

(same thing on linux)

@ALTree
Copy link
Member

ALTree commented Mar 21, 2018

I can (linux/amd64), here's the log:

--- FAIL: TestNexting (7.94s)
    --- FAIL: TestNexting/gdb-dbg-hist (2.37s)
    	debug_test.go:234: step/next histories differ, diff=
    		--- testdata/hist.gdb-dbg.nexts	2018-03-19 11:37:08.131407088 +0100
    		+++ /tmp/debug_test231459976/test-hist.gdb-dbg.nexts	2018-03-21 17:54:58.587602381 +0100
    		@@ -9,7 +9,7 @@
    		 61:		sink = dx + dy            //gdb-opt=(dx,dy)
    		 63:		hist := make([]int, 7)                                //gdb-opt=(dx/O,dy/O) // TODO sink is missing if this code is in 'test' instead of 'main'
    		 64:		var reader io.Reader = strings.NewReader(cannedInput) //gdb-dbg=(hist/A) // TODO cannedInput/A is missing if this code is in 'test' instead of 'main'
    		-hist =  []int = {0, 0, 0, 0, 0, 0, 0}
    		+hist = {array = <A>, len = 7, cap = 7}
    		 65:		if len(os.Args) > 1 {
    		 73:		scanner := bufio.NewScanner(reader)
    		 74:		for scanner.Scan() { //gdb-opt=(scanner/A)
    --- FAIL: TestNexting/gdb-opt-hist (0.95s)
    	debug_test.go:234: step/next histories differ, diff=
    		--- testdata/hist.gdb-opt.nexts	2018-03-19 11:37:08.139407145 +0100
    		+++ /tmp/debug_test577668668/test-hist.gdb-opt.nexts	2018-03-21 17:55:03.655623768 +0100
    		@@ -25,7 +25,7 @@
    		 76:			i, err := strconv.ParseInt(s, 10, 64)
    		 77:			if err != nil { //gdb-dbg=(i) //gdb-opt=(err,hist,i)
    		 err = {tab = 0x0, data = 0x0}
    		-hist =  []int = {0, 0, 0, 0, 0, 0, 0}
    		+hist = {array = 0xc00003be98, len = 7, cap = 7}
    		 i = 1
    		 81:			hist = ensure(int(i), hist)
    		 82:			hist[int(i)]++
    		@@ -35,7 +35,7 @@
    		 76:			i, err := strconv.ParseInt(s, 10, 64)
    		 77:			if err != nil { //gdb-dbg=(i) //gdb-opt=(err,hist,i)
    		 err = {tab = 0x0, data = 0x0}
    		-hist =  []int = {0, 1, 0, 0, 0, 0, 0}
    		+hist = {array = 0xc00003be98, len = 7, cap = 7}
    		 i = 1
    		 81:			hist = ensure(int(i), hist)
    		 82:			hist[int(i)]++
    		@@ -45,7 +45,7 @@
    		 76:			i, err := strconv.ParseInt(s, 10, 64)
    		 77:			if err != nil { //gdb-dbg=(i) //gdb-opt=(err,hist,i)
    		 err = {tab = 0x0, data = 0x0}
    		-hist =  []int = {0, 2, 0, 0, 0, 0, 0}
    		+hist = {array = 0xc00003be98, len = 7, cap = 7}
    		 i = 1
    		 81:			hist = ensure(int(i), hist)
    		 82:			hist[int(i)]++
    		@@ -55,7 +55,7 @@
    		 76:			i, err := strconv.ParseInt(s, 10, 64)
    		 77:			if err != nil { //gdb-dbg=(i) //gdb-opt=(err,hist,i)
    		 err = {tab = 0x0, data = 0x0}
    		-hist =  []int = {0, 3, 0, 0, 0, 0, 0}
    		+hist = {array = 0xc00003be98, len = 7, cap = 7}
    		 i = 2
    		 81:			hist = ensure(int(i), hist)
    		 82:			hist[int(i)]++
    		@@ -65,7 +65,7 @@
    		 76:			i, err := strconv.ParseInt(s, 10, 64)
    		 77:			if err != nil { //gdb-dbg=(i) //gdb-opt=(err,hist,i)
    		 err = {tab = 0x0, data = 0x0}
    		-hist =  []int = {0, 3, 1, 0, 0, 0, 0}
    		+hist = {array = 0xc00003be98, len = 7, cap = 7}
    		 i = 2
    		 81:			hist = ensure(int(i), hist)
    		 82:			hist[int(i)]++
    		@@ -75,7 +75,7 @@
    		 76:			i, err := strconv.ParseInt(s, 10, 64)
    		 77:			if err != nil { //gdb-dbg=(i) //gdb-opt=(err,hist,i)
    		 err = {tab = 0x0, data = 0x0}
    		-hist =  []int = {0, 3, 2, 0, 0, 0, 0}
    		+hist = {array = 0xc00003be98, len = 7, cap = 7}
    		 i = 2
    		 81:			hist = ensure(int(i), hist)
    		 82:			hist[int(i)]++
    		@@ -85,7 +85,7 @@
    		 76:			i, err := strconv.ParseInt(s, 10, 64)
    		 77:			if err != nil { //gdb-dbg=(i) //gdb-opt=(err,hist,i)
    		 err = {tab = 0x0, data = 0x0}
    		-hist =  []int = {0, 3, 3, 0, 0, 0, 0}
    		+hist = {array = 0xc00003be98, len = 7, cap = 7}
    		 i = 4
    		 81:			hist = ensure(int(i), hist)
    		 82:			hist[int(i)]++
    		@@ -95,7 +95,7 @@
    		 76:			i, err := strconv.ParseInt(s, 10, 64)
    		 77:			if err != nil { //gdb-dbg=(i) //gdb-opt=(err,hist,i)
    		 err = {tab = 0x0, data = 0x0}
    		-hist =  []int = {0, 3, 3, 0, 1, 0, 0}
    		+hist = {array = 0xc00003be98, len = 7, cap = 7}
    		 i = 4
    		 81:			hist = ensure(int(i), hist)
    		 82:			hist[int(i)]++
    		@@ -105,7 +105,7 @@
    		 76:			i, err := strconv.ParseInt(s, 10, 64)
    		 77:			if err != nil { //gdb-dbg=(i) //gdb-opt=(err,hist,i)
    		 err = {tab = 0x0, data = 0x0}
    		-hist =  []int = {0, 3, 3, 0, 2, 0, 0}
    		+hist = {array = 0xc00003be98, len = 7, cap = 7}
    		 i = 5
    		 81:			hist = ensure(int(i), hist)
    		 82:			hist[int(i)]++
FAIL
exit status 1
FAIL	cmd/compile/internal/ssa	93.549s

@randall77
Copy link
Contributor

Hmm, might be a gdb version issue. I'm running 7.9.
@dr2chase

@mvdan
Copy link
Member

mvdan commented Mar 21, 2018

I get the same failure, gdb version 8.1.

@ALTree
Copy link
Member

ALTree commented Mar 21, 2018

My system comes with gdb 7.12

@dr2chase
Copy link
Contributor

Is that gdb version or presence/absence of Python scripts?
I'll try to figure this out, I could at least make it more picky about gdb version if that is the case.

@dr2chase
Copy link
Contributor

I have gdb versions 7.9 and 8.0.1, seems to work for me.
I will try to figure out how to install a version that triggers the wrong printing (which I have clearly seen in the past, because of the "/A" following hist).

I am not super-fluent in Python, but this from runtime-gdb.py seems semi-relevant:

class SliceTypePrinter:
	"Pretty print slices."

	pattern = re.compile(r'^struct \[\]')

	def __init__(self, val):
		self.val = val

	def display_hint(self):
		return 'array'

	def to_string(self):
		return str(self.val.type)[6:]  # skip 'struct '

	def children(self):
		sval = SliceValue(self.val)
		if sval.len > sval.cap:
			return
		for idx, item in enumerate(sval):
			yield ('[{0}]'.format(idx), item)

@alexd765
Copy link
Contributor Author

i have added links to the fixes and other discussions so far to the main post

@alexd765
Copy link
Contributor Author

cmd/compile/internal/ssa fails for me with gdb 8.0.1, so the gdb version doesn't seem to be the issue.

@josharian
Copy link
Contributor

The underlying problem here is #12508

@dr2chase
Copy link
Contributor

@alexd765 the SliceTypePrinter code is really old, so that's probably the same.
My Python versions are 2.7.14 and 2.7.13. Yours?

@mvdan
Copy link
Member

mvdan commented Mar 23, 2018

@dr2chase I can reproduce it and I only have versions 2.7.14 and 3.6.4 installed. I'm running Arch Linux, so I would assume that any somewhat recent Linux system should have this test failure.

I tried playing with that runtime-gdb.py file, but not even emptying the file changes the behaviour of the test.

@alexd765
Copy link
Contributor Author

I am on Ubuntu 17.10 with python 2.7.14

@dr2chase
Copy link
Contributor

Except that I am failing to reproduce the bug on Linux....
I am learning a little Python.

@alexd765
Copy link
Contributor Author

@dr2chase I also started to look into this a little bit:

First observation: if i force the test to use delve with ~/gotip/src/cmd/compile/internal/ssa$ go test -d -v it passes.

Then i saw that there is an option to regenerate the testdata with ~/gotip/src/cmd/compile/internal/ssa$ go test -u. It changed some things in 2 testdata files and after that the tests are passing.

I can send the changes as a cl if that helps

@dr2chase
Copy link
Contributor

Mucking with the testdata files w/o understanding why we have this difference isn't going to help -- you'll break it for me, for example.

Also, I have several CLs stacked up that will mess with the testdata in a big way, alleged positive. (If you actually look at what's recorded there, there's plenty of room for improvement.)

@dr2chase
Copy link
Contributor

Perhaps it is not autoloading runtime-gdb.py?
Are there any warnings at the beginning about auto-load and safe-path?

I definitely need to modify the test to be better about this.

@gopherbot
Copy link

Change https://golang.org/cl/102598 mentions this issue: cmd/compile: invoke gdb more carefully in ssa/debug_test.go

gopherbot pushed a commit that referenced this issue Mar 26, 2018
Gdb can be sensitive to contents of .gdbinit, and to run
this test properly needs to have runtime/runtime-gdb.py
on the auto load safe path.  Therefore, turn off .gdbinit
loading and explicitly add $GOROOT/runtime to the safe
load path.

This should make ssa/debug_test.go run more consistently.

Updates #24464.

Change-Id: I63ed17c032cb3773048713ce51fca3a3f86e79b6
Reviewed-on: https://go-review.googlesource.com/102598
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@mvdan
Copy link
Member

mvdan commented Mar 27, 2018

@dr2chase all the TestNexting subtests are passing on my machine, so we can fairly safely assume that it's fixed for @alexd765 too :)

Out of the initial 5 issues, 3 have been fixed and 2 have been split into separate issues. So it seems like there's nothing else to do here. Thanks again @alexd765 for reporting them.

@mvdan mvdan closed this as completed Mar 27, 2018
@dr2chase
Copy link
Contributor

Thanks for verifying. I wasn't 100% sure, but I was hopeful.

@alexd765
Copy link
Contributor Author

yeah i can confirm that cmd/compile/internal/ssa is fixed for me as well

@golang golang locked and limited conversation to collaborators Mar 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

8 participants