Resolved race conditions, solidified/consolidated story convention te…

…sts. (fixes #70 - finally!)

convey/context.go

@@ -1,23 +1,28 @@
 package convey
 
 import (
-	"fmt"
 	"runtime"
-	"strings"
 	"sync"
 
 	"github.com/smartystreets/goconvey/execution"
 	"github.com/smartystreets/goconvey/reporting"
 )
 
+// SuiteContext magically handles all coordination of reporter, runners as they handle calls
+// to Convey, So, and the like. It does this via runtime call stack inspection, making sure
+// that each test function has its own runner and reporter, and routes all live registrations
+// to the appropriate runner/reporter.
 type SuiteContext struct {
 	runners   map[string]execution.Runner
 	reporters map[string]reporting.Reporter
 	lock      sync.Mutex
 }
 
-func (self *SuiteContext) Assign() execution.Runner {
-	key := resolveExternalCallerWithTestName()
+func (self *SuiteContext) Run(entry *execution.Registration) {
+	key := resolveTestPackageAndFunctionName()
+	if self.currentRunner() != nil {
+		panic(execution.ExtraGoTest)
+	}
 	reporter := buildReporter()
 	runner := execution.NewRunner()
 	runner.UpgradeReporter(reporter)
@@ -27,21 +32,34 @@ func (self *SuiteContext) Assign() execution.Runner {
 	self.reporters[key] = reporter
 	self.lock.Unlock()
 
-	return runner
+	runner.Begin(entry)
+	runner.Run()
+
+	self.lock.Lock()
+	delete(self.runners, key)
+	delete(self.reporters, key)
+	self.lock.Unlock()
 }
 
 func (self *SuiteContext) CurrentRunner() execution.Runner {
-	key := resolveExternalCallerWithTestName()
+	runner := self.currentRunner()
+
+	if runner == nil {
+		panic(execution.MissingGoTest)
+	}
+
+	return runner
+}
+func (self *SuiteContext) currentRunner() execution.Runner {
 	self.lock.Lock()
 	defer self.lock.Unlock()
-	return self.runners[key]
+	return self.runners[resolveTestPackageAndFunctionName()]
 }
 
 func (self *SuiteContext) CurrentReporter() reporting.Reporter {
-	key := resolveExternalCallerWithTestName()
 	self.lock.Lock()
 	defer self.lock.Unlock()
-	return self.reporters[key]
+	return self.reporters[resolveTestPackageAndFunctionName()]
 }
 
 func NewSuiteContext() *SuiteContext {
@@ -51,41 +69,26 @@ func NewSuiteContext() *SuiteContext {
 	return self
 }
 
-func resolveExternalCallerWithTestName() string {
-	// TODO: It turns out the more robust solution is to manually parse the debug.Stack()
-	//       because we can then filter out non-test methods that start with "Test".
-
-	var (
-		caller_id uintptr
-		testName  string
-		file      string
-	)
+// resolveTestPackageAndFunctionName traverses the call stack in reverse, looking for
+// the go testing harnass call ("testing.tRunner") and then grabs the very next entry,
+// which represents the package under test and the test function name. Voila!
+func resolveTestPackageAndFunctionName() string {
+	var callerId uintptr
 	callers := runtime.Callers(0, callStack)
 
-	var x int
-	for ; x < callers; x++ {
-		caller_id, file, _, _ = runtime.Caller(x)
-		if strings.HasSuffix(file, "test.go") {
-			break
-		}
-	}
-
-	for ; x < callers; x++ {
-		caller_id, _, _, _ = runtime.Caller(x)
-		packageAndTestName := runtime.FuncForPC(caller_id).Name()
-		parts := strings.Split(packageAndTestName, ".")
-		testName = parts[len(parts)-1]
-		if strings.HasPrefix(testName, "Test") {
-			break
+	for y := callers; y > 0; y-- {
+		callerId, _, _, _ = runtime.Caller(y)
+		packageAndTestFunctionName := runtime.FuncForPC(callerId).Name()
+		if packageAndTestFunctionName == goTestHarness {
+			callerId, _, _, _ = runtime.Caller(y - 1)
+			name := runtime.FuncForPC(callerId).Name()
+			return name
 		}
 	}
-
-	if testName == "" {
-		testName = "<unkown test method name>" // panic?
-	}
-	return fmt.Sprintf("%s---%s", testName, file)
+	panic("Can't resolve test method name! Are you calling Convey() from a `*_test.go` file and a `Test*` method (because you should be)?")
 }
 
-const maxStackDepth = 100 // This had better be enough...
+const maxStackDepth = 100               // This had better be enough...
+const goTestHarness = "testing.tRunner" // I hope this doesn't change...
 
 var callStack []uintptr = make([]uintptr, maxStackDepth, maxStackDepth)

convey/doc.go

@@ -41,9 +41,7 @@ func SkipConvey(items ...interface{}) {
 
 func register(entry *execution.Registration) {
 	if entry.IsTopLevel() {
-		runner := suites.Assign()
-		runner.Begin(entry)
-		runner.Run()
+		suites.Run(entry)
 	} else {
 		suites.CurrentRunner().Register(entry)
 	}

convey/reporting_hooks_test.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/smartystreets/goconvey/gotest"
 	"github.com/smartystreets/goconvey/reporting"
 )
 
@@ -229,3 +230,12 @@ func (self *fakeReporter) EndStory() {
 func (self *fakeReporter) wholeStory() string {
 	return strings.Join(self.calls, "|")
 }
+
+////////////////////////////////
+
+type fakeGoTest struct{}
+
+func (self *fakeGoTest) Fail()                                     {}
+func (self *fakeGoTest) Fatalf(format string, args ...interface{}) {}
+
+var test gotest.T = &fakeGoTest{}

convey/story_conventions_test.go

@@ -1,63 +1,113 @@
 package convey
 
-// TODO: get these working again:
-
-// func TestMissingTopLevelGoTestReferenceCausesPanic(t *testing.T) {
-// 	output := map[string]bool{}
-
-// 	defer expectEqual(t, false, output["good"])
-// 	defer requireGoTestReference(t)
-
-// 	Convey("Hi", func() {
-// 		output["bad"] = true // this shouldn't happen
-// 	})
-// }
-
-// func requireGoTestReference(t *testing.T) {
-// 	err := recover()
-// 	if err == nil {
-// 		t.Error("We should have recovered a panic here (because of a missing *testing.T reference)!")
-// 	} else {
-// 		expectEqual(t, execution.MissingGoTest, err)
-// 	}
-// }
-
-// func TestMissingTopLevelGoTestReferenceAfterGoodExample(t *testing.T) {
-// 	output := map[string]bool{}
-
-// 	defer func() {
-// 		expectEqual(t, true, output["good"])
-// 		expectEqual(t, false, output["bad"])
-// 	}()
-// 	defer requireGoTestReference(t)
-
-// 	Convey("Good example", t, func() {
-// 		output["good"] = true
-// 	})
-
-// 	Convey("Bad example", func() {
-// 		output["bad"] = true // shouldn't happen
-// 	})
-// }
-
-// func TestExtraReferencePanics(t *testing.T) {
-// 	output := map[string]bool{}
-
-// 	defer func() {
-// 		err := recover()
-// 		if err == nil {
-// 			t.Error("We should have recovered a panic here (because of an extra *testing.T reference)!")
-// 		} else if !strings.HasPrefix(fmt.Sprintf("%v", err), execution.ExtraGoTest) {
-// 			t.Error("Should have panicked with the 'extra go test' error!")
-// 		}
-// 		if output["bad"] {
-// 			t.Error("We should NOT have run the bad example!")
-// 		}
-// 	}()
-
-// 	Convey("Good example", t, func() {
-// 		Convey("Bad example - passing in *testing.T a second time!", t, func() {
-// 			output["bad"] = true // shouldn't happen
-// 		})
-// 	})
-// }
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/smartystreets/goconvey/execution"
+)
+
+func TestMissingTopLevelGoTestReferenceCausesPanic(t *testing.T) {
+	output := map[string]bool{}
+
+	defer expectEqual(t, false, output["good"])
+	defer requireGoTestReference(t)
+
+	Convey("Hi", func() {
+		output["bad"] = true // this shouldn't happen
+	})
+}
+
+func requireGoTestReference(t *testing.T) {
+	err := recover()
+	if err == nil {
+		t.Error("We should have recovered a panic here (because of a missing *testing.T reference)!")
+	} else {
+		expectEqual(t, execution.MissingGoTest, err)
+	}
+}
+
+func TestMissingTopLevelGoTestReferenceAfterGoodExample(t *testing.T) {
+	output := map[string]bool{}
+
+	defer func() {
+		expectEqual(t, true, output["good"])
+		expectEqual(t, false, output["bad"])
+	}()
+	defer requireGoTestReference(t)
+
+	Convey("Good example", t, func() {
+		output["good"] = true
+	})
+
+	Convey("Bad example", func() {
+		output["bad"] = true // shouldn't happen
+	})
+}
+
+func TestExtraReferencePanics(t *testing.T) {
+	output := map[string]bool{}
+
+	defer func() {
+		err := recover()
+		if err == nil {
+			t.Error("We should have recovered a panic here (because of an extra *testing.T reference)!")
+		} else if !strings.HasPrefix(fmt.Sprintf("%v", err), execution.ExtraGoTest) {
+			t.Error("Should have panicked with the 'extra go test' error!")
+		}
+		if output["bad"] {
+			t.Error("We should NOT have run the bad example!")
+		}
+	}()
+
+	Convey("Good example", t, func() {
+		Convey("Bad example - passing in *testing.T a second time!", t, func() {
+			output["bad"] = true // shouldn't happen
+		})
+	})
+}
+
+func TestParseRegistrationMissingRequiredElements(t *testing.T) {
+	defer func() {
+		if r := recover(); r != nil {
+			if r != "You must provide a name (string), then a *testing.T (if in outermost scope), and then an action (func())." {
+				t.Errorf("Incorrect panic message.")
+			}
+		}
+	}()
+
+	Convey()
+
+	t.Errorf("goTest should have panicked in Convey(...) and then recovered in the defer func().")
+}
+
+func TestParseRegistration_MissingNameString(t *testing.T) {
+	defer func() {
+		if r := recover(); r != nil {
+			if r != parseError {
+				t.Errorf("Incorrect panic message.")
+			}
+		}
+	}()
+
+	action := func() {}
+
+	Convey(action)
+
+	t.Errorf("goTest should have panicked in Convey(...) and then recovered in the defer func().")
+}
+
+func TestParseRegistration_MissingActionFunc(t *testing.T) {
+	defer func() {
+		if r := recover(); r != nil {
+			if r != parseError {
+				t.Errorf("Incorrect panic message: '%s'", r)
+			}
+		}
+	}()
+
+	Convey("Hi there", 12345)
+
+	t.Errorf("goTest should have panicked in Convey(...) and then recovered in the defer func().")
+}