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

Jump to bottom

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
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@hansbogert
Copy link

hansbogert commented Dec 4, 2020 •
edited

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 commented Dec 4, 2020 •
edited

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 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
@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label 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
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
2 participants
@toothrot @hansbogert