ensure deterministic order even if specs are generated by iterating o…

…ver a map

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())
 		})

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)

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)

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"))
+
+			})
+		})
+
 	})
 })