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

"go tool cover -func=cover.out" reports wrong percentage value on Windows? #42452

Closed
hartwork opened this issue Nov 8, 2020 · 5 comments
Closed

"go tool cover -func=cover.out" reports wrong percentage value on Windows? #42452

hartwork opened this issue Nov 8, 2020 · 5 comments
Labels
FrozenDueToAge

Comments

@hartwork
Copy link

hartwork commented Nov 8, 2020

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

$ go version  # inside GitHub Actions CI
go version go1.15.4 windows/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
  set GO111MODULE=
  set GOARCH=amd64
  set GOBIN=
  set GOCACHE=C:\Users\runneradmin\AppData\Local\go-build
  set GOENV=C:\Users\runneradmin\AppData\Roaming\go\env
  set GOEXE=.exe
  set GOFLAGS=
  set GOHOSTARCH=amd64
  set GOHOSTOS=windows
  set GOINSECURE=
  set GOMODCACHE=C:\Users\runneradmin\go\pkg\mod
  set GONOPROXY=
  set GONOSUMDB=
  set GOOS=windows
  set GOPATH=C:\Users\runneradmin\go
  set GOPRIVATE=
  set GOPROXY=https://proxy.golang.org,direct
  set GOROOT=C:\hostedtoolcache\windows\go\1.15.4\x64
  set GOSUMDB=sum.golang.org
  set GOTMPDIR=
  set GOTOOLDIR=C:\hostedtoolcache\windows\go\1.15.4\x64\pkg\tool\windows_amd64
  set GCCGO=gccgo
  set AR=ar
  set CC=gcc
  set CXX=g++
  set CGO_ENABLED=1
  set GOMOD=D:\a\go-wait-for-it\go-wait-for-it\go.mod
  set CGO_CFLAGS=-g -O2
  set CGO_CPPFLAGS=
  set CGO_CXXFLAGS=-g -O2
  set CGO_FFLAGS=-g -O2
  set CGO_LDFLAGS=-g -O2
  set PKG_CONFIG=pkg-config
  set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\RUNNER~1\AppData\Local\Temp\go-build677325866=/tmp/go-build -gno-record-gcc-switches

What did you do?

This is my first ever bug report to the Go community. Please bare with me if anything in here is not optimal, yet. Thank you! 🙏

I have publicly available code that is not platform specific and has 100.0% code coverage when run on Linux or macOS. Now I added Windows to the CI mix, and one function (waitForAddress) seems to be mis-reported as 91.7% by go tool cover -func=cover.out on Windows only, for no apparent reason. In detail (this is Bash on Windows in this very GitHub Actions CI):

$ git clone --branch make-ci-cover-windows https://github.com/hartwork/go-wait-for-it
$ go-wait-for-it
$ go build .
$ go test -v -coverprofile=cover.out ./...
$ go tool cover -func=cover.out
[..]
github.com/hartwork/go-wait-for-it/internal/network/wait.go:22:		waitForAddress				91.7%
[..]

What did you expect to see?

I expect the coverage to be 100.0% just like on macOS and Linux, because

  • there is nothing platform specific here
  • I have compare the output of diff -u internal/network/wait.go <(go tool cover -mode=set internal/network/wait.go)
    across all three platforms (and that very output is included in the CI log): all three are identical.

So I'm suspecting a Windows-specific bug in go tool cover here. What do you think?

What did you see instead?

A misreport of 91.7% for function waitForAddress on Windows, only. Linux und macOS show 100.0%.
@antong
Copy link
Contributor

antong commented Nov 8, 2020

In wait.go you have:

func waitForAddress(address syntax.Address) <-chan bool {
	available := make(chan bool, 1)
	go func() {
		for {
			c, err := net.Dial("tcp", address.String())
			if err == nil {
				available <- true
				c.Close()
				break
			}
			time.Sleep(500 * time.Millisecond)
		}
	}()
	return available
}

Depending on how you set up the test and on timing, net.Dial() may succeed the first time and never hit the time.Sleep() line. Check out the html output and you'll see what happens.

I wouldn't worry too much about 100% test coverage. Depending on how you are using the function in question, I'd perhaps worry a bit more about the unstoppable go routine it spawns. For example, if you give it an unreachable address, the go routine will continue to dial it forever, with no way to stop it.

@hartwork
Copy link
Author

hartwork commented Nov 8, 2020

Hi @antong, thanks for your reply!
The output from go tool cover -mode=set internal/network/wait.go shows that the line with time.Sleep was already hit on Windows and the tool's output is identical across platforms. 🤔
On the other hand, I just added another test spinning the loop for 3 seconds and that brought coverage up to 100% on Windows. These two findings contract themselves, any ideas what is going on? 🤔
PS: waitForAddress is intended to loop forever and be used with an external timeout. I'll rename the function to waitForAddressForever to make that more clear in a second. Getting to 100% coverage was intended because I wanted to see what it would take to get there (e.g. compared to Python).

@antong
Copy link
Contributor

antong commented Nov 8, 2020

I think go tool cover -mode=set just rewrites the source code to make it possible to produce the cover profile. go test -cover uses that internally to produce the actual coverage report. So the source code would be identical between runs and between different platforms, but if there are nondeterministic things like the timing in your test, then the coverage report itself can be different between platforms and between run. Don't look at the -mode=set output, but on the coverage profile that go test produces (go test -coverprofile=c.out). Do check the html output. It is really cool!

@hartwork
Copy link
Author

hartwork commented Nov 8, 2020 •
edited

You're right, here is proof based on new CI output:

--- /tmp/linux.txt       2020-11-08 23:42:51.919840484 +0100
+++ /tmp/windows.txt     2020-11-08 23:42:52.860847559 +0100
@@ -23,7 +23,7 @@
 github.com/hartwork/go-wait-for-it/internal/network/wait.go:24.12,25.7 1 1
 github.com/hartwork/go-wait-for-it/internal/network/wait.go:25.7,27.18 2 1
 github.com/hartwork/go-wait-for-it/internal/network/wait.go:27.18,30.10 3 1
-github.com/hartwork/go-wait-for-it/internal/network/wait.go:32.4,32.38 1 1
+github.com/hartwork/go-wait-for-it/internal/network/wait.go:32.4,32.38 1 0
 github.com/hartwork/go-wait-for-it/internal/network/wait.go:35.2,35.18 1 1
 github.com/hartwork/go-wait-for-it/internal/network/wait.go:38.130,42.17 3 1
 github.com/hartwork/go-wait-for-it/internal/network/wait.go:42.17,44.3 1 1

So I was using the tool wrong. Thanks for your help 🙏 , I'll close the issues as invalid then.

@hartwork hartwork closed this as completed Nov 8, 2020
@antong
Copy link
Contributor

antong commented Nov 9, 2020

@hartwork , glad I could help and happy that you got it working!

@golang golang locked and limited conversation to collaborators Nov 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@antong @hartwork @gopherbot