Skip to content

check: fix data race on C.status #44

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

Merged
niemeyer merged 1 commit into from
Jun 26, 2015
Merged

check: fix data race on C.status #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 30 additions & 21 deletions check.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"strconv"
"strings"
"sync"
"sync/atomic"
"time"
)

Expand All @@ -43,7 +44,7 @@ const (
missedSt
)

type funcStatus int
type funcStatus uint32

// A method value can't reach its own Method structure.
type methodType struct {
Expand Down Expand Up @@ -81,7 +82,7 @@ type C struct {
method *methodType
kind funcKind
testName string
status funcStatus
_status funcStatus
logb *logger
logw io.Writer
done chan *C
Expand All @@ -93,6 +94,14 @@ type C struct {
timer
}

func (c *C) status() funcStatus {
return funcStatus(atomic.LoadUint32((*uint32)(&c._status)))
}

func (c *C) setStatus(s funcStatus) {
atomic.StoreUint32((*uint32)(&c._status), uint32(s))
}

func (c *C) stopNow() {
runtime.Goexit()
}
Expand Down Expand Up @@ -455,7 +464,7 @@ func (tracker *resultTracker) _loopRoutine() {
tracker._waiting += 1
case c = <-tracker._doneChan:
tracker._waiting -= 1
switch c.status {
switch c.status() {
case succeededSt:
if c.kind == testKd {
if c.mustFail {
Expand Down Expand Up @@ -601,15 +610,15 @@ func (runner *suiteRunner) run() *Result {
runner.tracker.start()
if runner.checkFixtureArgs() {
c := runner.runFixture(runner.setUpSuite, "", nil)
if c == nil || c.status == succeededSt {
if c == nil || c.status() == succeededSt {
for i := 0; i != len(runner.tests); i++ {
c := runner.runTest(runner.tests[i])
if c.status == fixturePanickedSt {
if c.status() == fixturePanickedSt {
runner.skipTests(missedSt, runner.tests[i+1:])
break
}
}
} else if c != nil && c.status == skippedSt {
} else if c != nil && c.status() == skippedSt {
runner.skipTests(skippedSt, runner.tests)
} else {
runner.skipTests(missedSt, runner.tests)
Expand Down Expand Up @@ -674,22 +683,22 @@ func (runner *suiteRunner) callDone(c *C) {
switch v := value.(type) {
case *fixturePanic:
if v.status == skippedSt {
c.status = skippedSt
c.setStatus(skippedSt)
} else {
c.logSoftPanic("Fixture has panicked (see related PANIC)")
c.status = fixturePanickedSt
c.setStatus(fixturePanickedSt)
}
default:
c.logPanic(1, value)
c.status = panickedSt
c.setStatus(panickedSt)
}
}
if c.mustFail {
switch c.status {
switch c.status() {
case failedSt:
c.status = succeededSt
c.setStatus(succeededSt)
case succeededSt:
c.status = failedSt
c.setStatus(failedSt)
c.logString("Error: Test succeeded, but was expected to fail")
c.logString("Reason: " + c.reason)
}
Expand Down Expand Up @@ -724,11 +733,11 @@ func (runner *suiteRunner) runFixtureWithPanic(method *methodType, testName stri
return nil
}
c := runner.runFixture(method, testName, logb)
if c != nil && c.status != succeededSt {
if c != nil && c.status() != succeededSt {
if skipped != nil {
*skipped = c.status == skippedSt
*skipped = c.status() == skippedSt
}
panic(&fixturePanic{c.status, method})
panic(&fixturePanic{c.status(), method})
}
return c
}
Expand All @@ -753,7 +762,7 @@ func (runner *suiteRunner) forkTest(method *methodType) *C {
if mt.NumIn() != 1 || mt.In(0) != reflect.TypeOf(c) {
// Rather than a plain panic, provide a more helpful message when
// the argument type is incorrect.
c.status = panickedSt
c.setStatus(panickedSt)
c.logArgPanic(c.method, "*check.C")
return
}
Expand All @@ -773,7 +782,7 @@ func (runner *suiteRunner) forkTest(method *methodType) *C {
c.StartTimer()
c.method.Call([]reflect.Value{reflect.ValueOf(c)})
c.StopTimer()
if c.status != succeededSt || c.duration >= c.benchTime || benchN >= 1e9 {
if c.status() != succeededSt || c.duration >= c.benchTime || benchN >= 1e9 {
return
}
perOpN := int(1e9)
Expand Down Expand Up @@ -808,7 +817,7 @@ func (runner *suiteRunner) runTest(method *methodType) *C {
func (runner *suiteRunner) skipTests(status funcStatus, methods []*methodType) {
for _, method := range methods {
runner.runFunc(method, testKd, "", nil, func(c *C) {
c.status = status
c.setStatus(status)
})
}
}
Expand All @@ -825,7 +834,7 @@ func (runner *suiteRunner) checkFixtureArgs() bool {
succeeded = false
runner.runFunc(method, fixtureKd, "", nil, func(c *C) {
c.logArgPanic(method, "*check.C")
c.status = panickedSt
c.setStatus(panickedSt)
})
}
}
Expand All @@ -839,7 +848,7 @@ func (runner *suiteRunner) reportCallStarted(c *C) {

func (runner *suiteRunner) reportCallDone(c *C) {
runner.tracker.callDone(c)
switch c.status {
switch c.status() {
case succeededSt:
if c.mustFail {
runner.output.WriteCallSuccess("FAIL EXPECTED", c)
Expand Down Expand Up @@ -917,7 +926,7 @@ func (ow *outputWriter) WriteCallSuccess(label string, c *C) {
if c.reason != "" {
suffix = " (" + c.reason + ")"
}
if c.status == succeededSt {
if c.status() == succeededSt {
suffix += "\t" + c.timerString()
}
suffix += "\n"
Expand Down
8 changes: 4 additions & 4 deletions helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func (c *C) TestName() string {

// Failed returns whether the currently running test has already failed.
func (c *C) Failed() bool {
return c.status == failedSt
return c.status() == failedSt
}

// Fail marks the currently running test as failed.
Expand All @@ -25,7 +25,7 @@ func (c *C) Failed() bool {
// what went wrong. The higher level helper functions will fail the test
// and do the logging properly.
func (c *C) Fail() {
c.status = failedSt
c.setStatus(failedSt)
}

// FailNow marks the currently running test as failed and stops running it.
Expand All @@ -40,7 +40,7 @@ func (c *C) FailNow() {
// Succeed marks the currently running test as succeeded, undoing any
// previous failures.
func (c *C) Succeed() {
c.status = succeededSt
c.setStatus(succeededSt)
}

// SucceedNow marks the currently running test as succeeded, undoing any
Expand Down Expand Up @@ -72,7 +72,7 @@ func (c *C) Skip(reason string) {
panic("Missing reason why the test is being skipped")
}
c.reason = reason
c.status = skippedSt
c.setStatus(skippedSt)
c.stopNow()
}

Expand Down