From d44dedf3e3dd9dedc4d8dc177a65cfc5a13a3f8c Mon Sep 17 00:00:00 2001 From: Onsi Fakhouri Date: Wed, 17 Jun 2020 21:13:22 -0600 Subject: [PATCH] Defer running top-level container nodes until RunSpecs is called A common lifecycle gotcha is to try to dynamically generate tests based on configuration data. This fix defers evaluation of the top-level container blocks (Describe, Context, etc.) until RunSpecs. This allows the user to load any configuration information in the TestX hook just before RunSpecs is called and leverage that configuration information to inform how the testing tree is generated. [Fixes #693] --- extensions/table/table_entry.go | 6 ---- integration/fail_test.go | 2 -- internal/suite/suite.go | 54 ++++++++++++++++++++++++------ internal/suite/suite_suite_test.go | 16 +++++++++ 4 files changed, 60 insertions(+), 18 deletions(-) diff --git a/extensions/table/table_entry.go b/extensions/table/table_entry.go index 783e7964a..4d9c237ad 100644 --- a/extensions/table/table_entry.go +++ b/extensions/table/table_entry.go @@ -21,12 +21,6 @@ type TableEntry struct { } func (t TableEntry) generateIt(itBody reflect.Value) { - if t.codeLocation == (types.CodeLocation{}) { - // The user created the TableEntry struct directly instead of having used the (F/P/X)Entry constructors. - // Therefore default to the code location of the surrounding DescribeTable. - t.codeLocation = codelocation.New(5) - } - var description string descriptionValue := reflect.ValueOf(t.Description) switch descriptionValue.Kind() { diff --git a/integration/fail_test.go b/integration/fail_test.go index 873a4ad13..9ec764b07 100644 --- a/integration/fail_test.go +++ b/integration/fail_test.go @@ -56,8 +56,6 @@ var _ = Describe("Failing Specs", func() { Ω(output).ShouldNot(ContainSubstring("table_entry.go")) Ω(output).Should(MatchRegexp(`a TableEntry constructed by Entry \[It\]\n.*fail_fixture_test\.go:110`), "the output of a failing Entry should include its file path and line number") - Ω(output).Should(MatchRegexp(`a directly constructed TableEntry \[It\]\n.*fail_fixture_test\.go:106`), - "the output of a failing TableEntry should include the surrounding DescribeTable's file path and line number") Ω(output).Should(ContainSubstring("0 Passed | 19 Failed")) }) diff --git a/internal/suite/suite.go b/internal/suite/suite.go index 34f639ee4..e75da1f89 100644 --- a/internal/suite/suite.go +++ b/internal/suite/suite.go @@ -22,25 +22,37 @@ type ginkgoTestingT interface { Fail() } +type deferredContainerNode struct { + text string + body func() + flag types.FlagType + codeLocation types.CodeLocation +} + type Suite struct { topLevelContainer *containernode.ContainerNode currentContainer *containernode.ContainerNode - containerIndex int - beforeSuiteNode leafnodes.SuiteNode - afterSuiteNode leafnodes.SuiteNode - runner *specrunner.SpecRunner - failer *failer.Failer - running bool + + deferredContainerNodes []deferredContainerNode + + containerIndex int + beforeSuiteNode leafnodes.SuiteNode + afterSuiteNode leafnodes.SuiteNode + runner *specrunner.SpecRunner + failer *failer.Failer + running bool + expandTopLevelNodes bool } func New(failer *failer.Failer) *Suite { topLevelContainer := containernode.New("[Top Level]", types.FlagTypeNone, types.CodeLocation{}) return &Suite{ - topLevelContainer: topLevelContainer, - currentContainer: topLevelContainer, - failer: failer, - containerIndex: 1, + topLevelContainer: topLevelContainer, + currentContainer: topLevelContainer, + failer: failer, + containerIndex: 1, + deferredContainerNodes: []deferredContainerNode{}, } } @@ -53,6 +65,11 @@ func (suite *Suite) Run(t ginkgoTestingT, description string, reporters []report panic("ginkgo.parallel.node is one-indexed and must be <= ginkgo.parallel.total") } + suite.expandTopLevelNodes = true + for _, deferredNode := range suite.deferredContainerNodes { + suite.PushContainerNode(deferredNode.text, deferredNode.body, deferredNode.flag, deferredNode.codeLocation) + } + r := rand.New(rand.NewSource(config.RandomSeed)) suite.topLevelContainer.Shuffle(r) iterator, hasProgrammaticFocus := suite.generateSpecsIterator(description, config) @@ -137,6 +154,23 @@ func (suite *Suite) SetSynchronizedAfterSuiteNode(bodyA interface{}, bodyB inter } func (suite *Suite) PushContainerNode(text string, body func(), flag types.FlagType, codeLocation types.CodeLocation) { + /* + We defer walking the container nodes (which immediately evaluates the `body` function) + until `RunSpecs` is called. We do this by storing off the deferred container nodes. Then, when + `RunSpecs` is called we actually go through and add the container nodes to the test structure. + + This allows us to defer calling all the `body` functions until _after_ the top level functions + have been walked, _after_ func init()s have been called, and _after_ `go test` has called `flag.Parse()`. + + This allows users to load up configuration information in the `TestX` go test hook just before `RunSpecs` + is invoked and solves issues like #693 and makes the lifecycle easier to reason about. + + */ + if !suite.expandTopLevelNodes { + suite.deferredContainerNodes = append(suite.deferredContainerNodes, deferredContainerNode{text, body, flag, codeLocation}) + return + } + container := containernode.New(text, flag, codeLocation) suite.currentContainer.PushContainerNode(container) diff --git a/internal/suite/suite_suite_test.go b/internal/suite/suite_suite_test.go index 06fe1d12a..ff69741f0 100644 --- a/internal/suite/suite_suite_test.go +++ b/internal/suite/suite_suite_test.go @@ -1,19 +1,25 @@ package suite_test import ( + "fmt" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "testing" ) +var dynamicallyGeneratedTests = []string{} + func Test(t *testing.T) { RegisterFailHandler(Fail) + dynamicallyGeneratedTests = []string{"Test A", "Test B", "Test C"} RunSpecs(t, "Suite") } var numBeforeSuiteRuns = 0 var numAfterSuiteRuns = 0 +var numDynamicallyGeneratedTests = 0 var _ = BeforeSuite(func() { numBeforeSuiteRuns++ @@ -23,6 +29,16 @@ var _ = AfterSuite(func() { numAfterSuiteRuns++ Ω(numBeforeSuiteRuns).Should(Equal(1)) Ω(numAfterSuiteRuns).Should(Equal(1)) + Ω(numDynamicallyGeneratedTests).Should(Equal(3), "Expected three test to be dynamically generated") +}) + +var _ = Describe("Top-level cotnainer node lifecycle", func() { + for _, test := range dynamicallyGeneratedTests { + numDynamicallyGeneratedTests += 1 + It(fmt.Sprintf("runs dynamically generated test: %s", test), func() { + Ω(true).Should(BeTrue()) + }) + } }) //Fakes