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.

Sign up for GitHub

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

Jump to bottom

x/tools/imports: imports_test is flaky #18142

Closed
ianlancetaylor opened this issue Dec 1, 2016 · 6 comments
Closed

x/tools/imports: imports_test is flaky #18142

ianlancetaylor opened this issue Dec 1, 2016 · 6 comments
Labels
FrozenDueToAge Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Unreleased

Comments

@ianlancetaylor
Copy link
Contributor

Building on tip on amd64 GNU/Linux.

cd $GOPATH/src/golang.org/x/imports
go test -c

Run ./imports.test 500 times. It failed once, with the following error:

--- FAIL: TestImportSymlinks (0.01s)
	fix_test.go:889: results differ
		GOT:
		package p
		
		import (
			"fmt"
			"x/apkg/mypkg"
		)
		
		var (
			_ = fmt.Print
			_ = mypkg.Foo
		)
		
		WANT:
		package p
		
		import (
			"fmt"
			"x/mypkg"
		)
		
		var (
			_ = fmt.Print
			_ = mypkg.Foo
		)
		
FAIL

CC @bradfitz
@ianlancetaylor ianlancetaylor added this to the Unreleased milestone Dec 1, 2016
@bradfitz bradfitz added the Testing An issue that has been verified to require only test changes, not just a test failure. label Dec 1, 2016
@bradfitz bradfitz self-assigned this Dec 1, 2016
@pwaller
Copy link
Contributor

pwaller commented Jan 2, 2017

FWIW, I am unable to reproduce this with 5,000 runs on GNU/Linux amd64, both on go1.8beta2 and git checkout $(git rev-list -n1 --before 2016-12-01 origin/master). Can you provide more information for reproduction?

go version devel +85bcf7f Thu Dec 1 15:58:59 2016 +0000 linux/amd64

I tested this version of goimports: golang/tools@908849c

@NightTsarina
Copy link

I've just received a bug report in Debian because of this very issue. It seems this test fails sometimes, but I was not able to reproduce it either..

https://buildd.debian.org/status/fetch.php?pkg=golang-golang-x-tools&arch=armhf&ver=1%3A0.0%7Egit20161028.0.b814a3b%2Bds-3%2Bb1&stamp=1488869926&raw=0

@bradfitz
Copy link
Contributor

bradfitz commented Mar 7, 2017

Sorry, I dropped the ball on this. I actually spent a fair bit of time debugging it but didn't make it far. IIRC, it seemed filesystem-specific. @TheTincho, do you know which filesystem the Debian auto-builders run on?

@NightTsarina
Copy link

@bradfitz Uhm, no.. But I'd bet it is ext4

@bradfitz
Copy link
Contributor

bradfitz commented Mar 8, 2017

The problem is that fastWalk uses multiple goroutines to walk the filesystem (one of the reasons that it's fast). fastWalk documents that it doesn't follow symlinks unless the caller requests it (per symlink). goimports does request following symlinks and tries to stop infinite walks from symlink loops, but depending on the order of goroutine scheduling, different paths can be explored through the test's created filesystem layout (which includes a symlink loop), and an unlucky rare ordering can cause the test to fail, finding the unexpected name for a path.

Probably the callback's mechanism to stop infinite symlink loop walking should be changed.

Some debug output of a successful run (first) vs a failure (second):

bradfitzlaptop imports$ go test -test.run=TestImportSymlinks -test.count=1 -test.v
=== RUN   TestImportSymlinks
--- PASS: TestImportSymlinks (0.06s)
        fix_test.go:897: findImportGoPath(pkgName="mypkg", symbols=map[Foo:true]) ...
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src", typ=d---------
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src/x", typ=d---------
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src/x/apkg", typ=L---------
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src/x/mypkg", typ=L---------
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src/x/apkg/apkg", typ=L---------
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src/x/mypkg/f.go", typ=----------
        fix_test.go:897: already visited symlink /private/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src/x/apkg
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src/x/apkg/mypkg", typ=L---------
        fix_test.go:897: already visited symlink /private/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src/x/mypkg
        fix_test.go:897: candidate[/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src/x/mypkg] = {dir:/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src/x/mypkg importPath:x/mypkg importPathShort:x/mypkg}
        fix_test.go:897: mypkg candidate 1/1: x/mypkg
        fix_test.go:897: findImportGoPath(pkgName="mypkg", symbols=map[Foo:true]) ...
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src", typ=d---------
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src/.goimportsignore", typ=----------
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src/x", typ=d---------
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src/x/apkg", typ=L---------
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src/x/mypkg", typ=L---------
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src/x/apkg/apkg", typ=L---------
        fix_test.go:897: already visited symlink /private/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src/x/apkg
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest294175778/src/x/apkg/mypkg", typ=L---------
PASS
ok      golang.org/x/tools/imports      0.092s
bradfitzlaptop imports$
bradfitzlaptop imports$ go test -test.run=TestImportSymlinks -test.count=500
--- FAIL: TestImportSymlinks (0.02s)
        fix_test.go:897: findImportGoPath(pkgName="mypkg", symbols=map[Foo:true]) ...
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest480273255/src", typ=d---------
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest480273255/src/x", typ=d---------
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest480273255/src/x/apkg", typ=L---------
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest480273255/src/x/mypkg", typ=L---------
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest480273255/src/x/apkg/apkg", typ=L---------
        fix_test.go:897: already visited symlink /private/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest480273255/src/x/apkg
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest480273255/src/x/apkg/mypkg", typ=L---------
        fix_test.go:897: walk path "/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest480273255/src/x/apkg/mypkg/f.go", typ=----------
        fix_test.go:897: already visited symlink /private/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest480273255/src/x/mypkg
        fix_test.go:897: candidate[/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest480273255/src/x/apkg/mypkg] = {dir:/var/folders/n7/gw0f1vrj0m5ffgwgyzbm2fwm001w6m/T/symlinktest480273255/src/x/apkg/mypkg importPath:x/apkg/mypkg importPathShort:x/apkg/mypkg}
        fix_test.go:897: mypkg candidate 1/1: x/apkg/mypkg
        fix_test.go:924: results differ
                GOT:
                package p

                import (
                        "fmt"
                        "x/apkg/mypkg"
                )

                var (   
                        _ = fmt.Print
                        _ = mypkg.Foo
                )

                WANT:
                package p

                import (
                        "fmt"
                        "x/mypkg"
                )

                var (   
                        _ = fmt.Print
                        _ = mypkg.Foo
                )

FAIL

@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators Mar 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@bradfitz @pwaller @ianlancetaylor @NightTsarina @gopherbot