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

Spurious test failures on Windows #5706

Closed
calmh opened this issue May 10, 2019 · 4 comments

Comments

Projects
None yet
2 participants
@calmh
Copy link
Member

commented May 10, 2019

I don't always run tests on Windows, but when I do they fail.

C:\Users\Jakob Borg\source\repos\syncthing>go version
go version go1.12 windows/amd64

C:\Users\Jakob Borg\source\repos\syncthing>go run build.go test
ok      github.com/syncthing/syncthing/lib/api  6.613s
ok      github.com/syncthing/syncthing/lib/auto 0.334s
ok      github.com/syncthing/syncthing/lib/beacon       0.386s
ok      github.com/syncthing/syncthing/lib/build        0.253s
ok      github.com/syncthing/syncthing/lib/config       0.447s
ok      github.com/syncthing/syncthing/lib/connections  1.320s
ok      github.com/syncthing/syncthing/lib/db   1.355s
ok      github.com/syncthing/syncthing/lib/dialer       0.411s [no tests to run]
ok      github.com/syncthing/syncthing/lib/discover     0.702s
ok      github.com/syncthing/syncthing/lib/events       5.805s
--- FAIL: TestResolveWindows83 (0.00s)
    basicfs_windows_test.go:62: Resolving for 8.3 names of "\\?\C:\Users\JAKOBB~1\AppData\Local\Temp\476953126\LFDATA~1" resulted in "\\?\C:\Users\Jakob Borg\AppData\Local\Temp\476953126\LFDataTool", expected "\\?\C:\Users\JAKOBB~1\AppData\Local\Temp\476953126\LFDataTool"
    basicfs_windows_test.go:65: Resolving for 8.3 names of "\\?\C:\Users\JAKOBB~1\AppData\Local\Temp\476953126\foo\LFDATA~1" resulted in "\\?\C:\Users\JAKOBB~1\AppData\Local\Temp\476953126", expected "\\?\C:\Users\JAKOBB~1\AppData\Local\Temp\476953126\foo"
    basicfs_windows_test.go:68: Resolving for 8.3 names of "\\?\C:\Users\JAKOBB~1\AppData\Local\Temp\476953126\foo\bar\baz" resulted in "\\?\C:\Users\JAKOBB~1\AppData\Local\Temp\476953126", expected "\\?\C:\Users\JAKOBB~1\AppData\Local\Temp\476953126\foo\bar\baz"
--- FAIL: TestIsWindows83 (0.00s)
    basicfs_windows_test.go:83: "\\?\C:\Users\JAKOBB~1\AppData\Local\Temp\689632077\~syncthing~baz.tmp" is not a windows 8.3 path"
    basicfs_windows_test.go:83: "\\?\C:\Users\JAKOBB~1\AppData\Local\Temp\689632077\foo\bar\~syncthing~baz.tmp" is not a windows 8.3 path"
--- FAIL: TestCommonPrefix (0.00s)
    util_test.go:27: Expected \\?\C:\ got \\?\C:
    util_test.go:33: Expected \\?\c:\ got \\?\c:
FAIL
FAIL    github.com/syncthing/syncthing/lib/fs   0.604s
ok      github.com/syncthing/syncthing/lib/ignore       0.603s
?       github.com/syncthing/syncthing/lib/locations    [no test files]

It doesn't fail on the build server, apparently, but the tests seem fragile somehow.

The 8.3 test failures might be because my user name is longer than eight characters.

imsodin added a commit to imsodin/syncthing that referenced this issue May 10, 2019

@imsodin

This comment has been minimized.

Copy link
Member

commented May 10, 2019

It's weird that you get win83 names, but as you do the failures are expected. Can you check whether #5709 fixes them?

As for the prefix: Can't reproduce on win7. Is the failure consistent?

calmh added a commit that referenced this issue May 11, 2019

@calmh

This comment has been minimized.

Copy link
Member Author

commented May 11, 2019

The 83 stuff passes now with your latest fixes.

The prefix failure is consistent, yes. Windows 10. I'll take a look and see if something obvious pops up...

@calmh

This comment has been minimized.

Copy link
Member Author

commented May 11, 2019

So the problem is that in Go 1.12 (that I had installed on my Windows VM) filepath.Clean strips the trailing slash of UNC-style volume-only paths. That's wrong. In Go 1.12.5 on the build server (and, now, my VM) this is fixed. So it passes.

@calmh calmh closed this May 11, 2019

@calmh calmh added this to the v1.1.4 milestone May 11, 2019

@imsodin

This comment has been minimized.

Copy link
Member

commented May 11, 2019

Looking at upstreams bugs regarding this (golang/go#27791, golang/go#30307), there seems to be quite a bit of uncertainty about the behaviour of path/filepath with UNC paths, especially this comment: golang/go#30307 (comment)
Sounds like a good motivation for figuring out the need (or not need) of our UNC-ness: #5696

And good to know we now have test in place that will blow up if the behaviour changes again :)

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.