diff --git a/internal/internal_integration/table_test.go b/internal/internal_integration/table_test.go index a16e8d596..a695ec3c4 100644 --- a/internal/internal_integration/table_test.go +++ b/internal/internal_integration/table_test.go @@ -21,11 +21,14 @@ var _ = Describe("Table driven tests", func() { Describe("constructing tables", func() { BeforeEach(func() { - entrySlice := []TableEntry{ - Entry("C", 1, 2), Entry("D", 1, 1), - } success, _ := RunFixture("table happy-path", func() { - DescribeTable("hello", bodyFunc, Entry("A", 1, 1), Entry("B", 1, 1), entrySlice) + DescribeTable("hello", bodyFunc, + Entry("A", 1, 1), + Entry("B", 1, 1), + []TableEntry{ + Entry("C", 1, 2), + Entry("D", 1, 1), + }) }) Ω(success).Should(BeFalse()) }) diff --git a/internal/internal_suite_test.go b/internal/internal_suite_test.go index 0244b99bc..d0f162144 100644 --- a/internal/internal_suite_test.go +++ b/internal/internal_suite_test.go @@ -85,7 +85,7 @@ func S(nodes ...Node) Spec { // convenience helper to quickly make code locations func CL(options ...interface{}) types.CodeLocation { - cl = types.NewCodeLocation(0) + cl = types.NewCodeLocation(1) for _, option := range options { if reflect.TypeOf(option).Kind() == reflect.String { cl.FileName = option.(string) diff --git a/internal/ordering.go b/internal/ordering.go index 161be820c..1e8047f2d 100644 --- a/internal/ordering.go +++ b/internal/ordering.go @@ -7,6 +7,50 @@ import ( "github.com/onsi/ginkgo/v2/types" ) +type SortableSpecs struct { + Specs Specs + Indexes []int +} + +func NewSortableSpecs(specs Specs) *SortableSpecs { + indexes := make([]int, len(specs)) + for i := range specs { + indexes[i] = i + } + return &SortableSpecs{ + Specs: specs, + Indexes: indexes, + } +} +func (s *SortableSpecs) Len() int { return len(s.Indexes) } +func (s *SortableSpecs) Swap(i, j int) { s.Indexes[i], s.Indexes[j] = s.Indexes[j], s.Indexes[i] } +func (s *SortableSpecs) Less(i, j int) bool { + a, b := s.Specs[s.Indexes[i]], s.Specs[s.Indexes[j]] + aCLs := a.Nodes.WithType(types.NodeTypesForContainerAndIt).CodeLocations() + bCLs := b.Nodes.WithType(types.NodeTypesForContainerAndIt).CodeLocations() + for i := 0; i < len(aCLs) && i < len(bCLs); i++ { + aCL, bCL := aCLs[i], bCLs[i] + if aCL.FileName < bCL.FileName { + return true + } else if aCL.FileName > bCL.FileName { + return false + } + if aCL.LineNumber < bCL.LineNumber { + return true + } else if aCL.LineNumber > bCL.LineNumber { + return false + } + } + // either everything is equal or we have different lengths of CLs + if len(aCLs) < len(bCLs) { + return true + } else if len(aCLs) > len(bCLs) { + return false + } + // ok, now we are sure everything was equal. so we use the spec text to break ties + return a.Text() < b.Text() +} + type GroupedSpecIndices []SpecIndices type SpecIndices []int @@ -28,12 +72,17 @@ func OrderSpecs(specs Specs, suiteConfig types.SuiteConfig) (GroupedSpecIndices, // Seed a new random source based on thee configured random seed. r := rand.New(rand.NewSource(suiteConfig.RandomSeed)) - // first break things into execution groups + // first, we sort the entire suite to ensure a deterministic order. the sort is performed by filename, then line number, and then spec text. this ensures every parallel process has the exact same spec order and is only necessary to cover the edge case where the user iterates over a map to generate specs. + sortableSpecs := NewSortableSpecs(specs) + sort.Sort(sortableSpecs) + + // then we break things into execution groups // a group represents a single unit of execution and is a collection of SpecIndices // usually a group is just a single spec, however ordered containers must be preserved as a single group executionGroupIDs := []uint{} executionGroups := map[uint]SpecIndices{} - for idx, spec := range specs { + for _, idx := range sortableSpecs.Indexes { + spec := specs[idx] groupNode := spec.Nodes.FirstNodeMarkedOrdered() if groupNode.IsZero() { groupNode = spec.Nodes.FirstNodeWithType(types.NodeTypeIt) diff --git a/internal/ordering_test.go b/internal/ordering_test.go index c127268d8..522b58313 100644 --- a/internal/ordering_test.go +++ b/internal/ordering_test.go @@ -187,18 +187,18 @@ var _ = Describe("OrderSpecs", func() { Context("when there are ordered specs and randomize-all is false and everything is in an enclosing container", func() { BeforeEach(func() { - con0 := N(ntCon) - con1 := N(ntCon, Ordered) - con2 := N(ntCon) + con0 := N(ntCon, CL(1)) + con1 := N(ntCon, Ordered, CL(4)) + con2 := N(ntCon, CL(10)) specs = Specs{ - S(con0, N("A", ntIt)), - S(con0, N("B", ntIt)), - S(con0, con1, N("C", ntIt)), - S(con0, con1, N("D", ntIt)), - S(con0, con1, N(ntCon), N("E", ntIt)), - S(con0, N("F", ntIt)), - S(con0, con2, N("G", ntIt)), - S(con0, con2, N("H", ntIt)), + S(con0, N("A", ntIt, CL(2))), + S(con0, N("B", ntIt, CL(3))), + S(con0, con1, N("C", ntIt, CL(5))), + S(con0, con1, N("D", ntIt, CL(6))), + S(con0, con1, N(ntCon, CL(7)), N("E", ntIt, CL(8))), + S(con0, N("F", ntIt, CL(9))), + S(con0, con2, N("G", ntIt, CL(11))), + S(con0, con2, N("H", ntIt, CL(12))), } conf.RandomizeAllSpecs = false @@ -275,5 +275,46 @@ var _ = Describe("OrderSpecs", func() { Ω(getTexts(specs, serialSpecIndices1)).ShouldNot(Equal(getTexts(specs, serialSpecIndices2))) }) }) + + Describe("presorting-specs", func() { + BeforeEach(func() { + conA0 := N(ntCon, CL("file-A", 1)) + conA1 := N(ntCon, CL("file-A", 4)) + conA2 := N(ntCon, CL("file-A", 10)) + conB0 := N(ntCon, CL("file-B", 1)) + conC0 := N(ntCon, CL("file-C", 1)) + specs = Specs{ + S(conA0, N("A", ntIt, CL("file-A", 2))), + S(conA0, N("B", ntIt, CL("file-A", 3))), + S(conA0, conA1, N("C", ntIt, CL("file-A", 5))), + S(conA0, conA1, N("D", ntIt, CL("file-A", 6))), + S(conA0, conA1, N(ntCon, CL("file-A", 7)), N("E", ntIt, CL("file-A", 8))), + S(conA0, N("F", ntIt, CL("file-A", 9))), + S(conA0, conA2, N("G", ntIt, CL("file-A", 11))), + S(conA0, conA2, N("H", ntIt, CL("file-A", 12))), + S(conB0, N("B-Z", ntIt, CL("file-B", 2))), + S(conB0, N("B-Y", ntIt, CL("file-B", 3))), + S(conB0, N("B-D", ntIt, CL("file-B", 4))), + S(conB0, N("B-C", ntIt, CL("file-B", 4))), + S(conB0, N("B-B", ntIt, CL("file-B", 4))), + S(conB0, N("B-A", ntIt, CL("file-B", 5))), + } + + for key := range map[string]bool{"C-A": true, "C-B": true, "C-C": true, "C-D": true, "C-E": true, "C-F": true} { + specs = append(specs, S(conC0, N(key, ntIt, CL("file-C", 2)))) // normally this would be totally non-deterministic + } + conf.RandomizeAllSpecs = false + }) + + It("ensures a deterministic order for specs that are defined at the same line without messing with the natural order of specs and containers", func() { + conf.RandomSeed = 1 // this happens to sort conA0 ahead of conB0 - other than that, though, we are actually testing SortableSpecs + groupedSpecIndices, serialSpecIndices := internal.OrderSpecs(specs, conf) + Ω(serialSpecIndices).Should(BeEmpty()) + + Ω(getTexts(specs, groupedSpecIndices).Join()).Should(Equal("ABCDEFGHB-ZB-YB-BB-CB-DB-AC-AC-BC-CC-DC-EC-F")) + + }) + }) + }) })