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: don't overwrite race detector exit code #43001

Open
hansbogert opened this issue Dec 4, 2020 · 2 comments
Open

testing: don't overwrite race detector exit code #43001

hansbogert opened this issue Dec 4, 2020 · 2 comments

Comments

@hansbogert
Copy link

@hansbogert hansbogert commented Dec 4, 2020

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

$ go version
go version go1.15 linux/amd64

Does this issue reproduce with the latest release?

(Also) Tested against golang:latest from the docker hub

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

go env Output
$ go env # Non docker environment
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/hvdb/.cache/go-build"
GOENV="/home/hvdb/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/hvdb/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/hvdb/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go-1.15"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.15/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build016896563=/tmp/go-build -gno-record-gcc-switches"

What did you do?

File main.go

import "fmt"

func main() {
	c := make(chan bool)
	m := make(map[string]string)
	go func() {
		m["1"] = "a" // First conflicting access.
		c <- true
	}()
	m["2"] = "b" // Second conflicting access.
	<-c
	for k, v := range m {
		fmt.Println(k, v)
	}
}

File main_test.go

package main

import (
	"os"
	"testing"
)

func TestMain(m *testing.M) {
	main()
	os.Exit(m.Run())
}
$ go test
2 b
1 a
testing: warning: no tests to run
PASS
ok  	_/home/hvdb/Sync/bug/golang-race-exitcode	0.001s


$ go test  -race
==================
WARNING: DATA RACE
[clipped]
==================
1 a
2 b
testing: warning: no tests to run
FAIL
exit status 1
FAIL	_/home/hvdb/Sync/bug/golang-race-exitcode	0.008s


$ GORACE=exitcode=0 go test -race 
==================
WARNING: DATA RACE
[clipped]
==================
2 b
1 a
testing: warning: no tests to run
FAIL
exit status 1
FAIL	_/home/hvdb/Sync/bug/golang-race-exitcode	0.009s

What did you expect to see?

$ go test  -race
[same as before]
exit status 66
FAIL	_/home/hvdb/Sync/bug/golang-race-exitcode	0.008s

$ GORACE=exitcode=0 go test -race 
[same as before]
exit status 0
FAIL	_/home/hvdb/Sync/bug/golang-race-exitcode	0.009s

What did you see instead?

See what did you do

@hansbogert
Copy link
Author

@hansbogert hansbogert commented Dec 4, 2020

For context:

I understand that this is a bit of a corner case, however some frameworks, like godog, really rely on just being run in TestMain, it'd be great if the exitcode semantics would be honored.

In case of godog, it works almost completely fine when NOT calling m.Run() from TestMain. The correct exitcodes are used when encountering dataraces. However, coverage-profile data is only written once you call m.Run(), however we get the above inconsistent behaviour.

@toothrot
Copy link
Contributor

@toothrot toothrot commented Dec 8, 2020

I think TestMain is a red herring here. I believe the failed test exit code supersedes the race detector exit code, regardless of whether TestMain is defined by the programmer.

/cc @mpvl

@toothrot toothrot added this to the Backlog milestone Dec 8, 2020
@seankhliao seankhliao changed the title test with TestMain fails on datarace, but not with exitcode 66 testing: don't overwrite race detector exit code Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants