Ensure CurrentSpecReport and AddReportEntry are thread-safe

Author: onsi

internal/internal_integration/report_entries_test.go

@@ -1,35 +1,92 @@
 package internal_integration_test
 
 import (
+	"fmt"
+	"time"
+
 	. "github.com/onsi/ginkgo/v2"
 	"github.com/onsi/ginkgo/v2/types"
 	. "github.com/onsi/gomega"
 )
 
 var _ = Describe("ReportEntries", func() {
-	BeforeEach(func() {
-		success, _ := RunFixture("Report Entries", func() {
-			BeforeSuite(func() {
-				AddReportEntry("bridge", "engaged")
-			})
+	Context("happy path", func() {
+		BeforeEach(func() {
+			success, _ := RunFixture("Report Entries", func() {
+				BeforeSuite(func() {
+					AddReportEntry("bridge", "engaged")
+				})
+
+				It("adds-entries", func() {
+					AddReportEntry("medical", "healthy")
+					AddReportEntry("engineering", "on fire")
+				})
 
-			It("adds-entries", func() {
-				AddReportEntry("medical", "healthy")
-				AddReportEntry("engineering", "on fire")
+				It("adds-no-entries", func() {})
 			})
+			Ω(success).Should(BeTrue())
+		})
 
-			It("adds-no-entries", func() {})
+		It("attaches entries to the report", func() {
+			Ω(reporter.Did.Find("adds-entries").ReportEntries[0].Name).Should(Equal("medical"))
+			Ω(reporter.Did.Find("adds-entries").ReportEntries[0].Value.String()).Should(Equal("healthy"))
+			Ω(reporter.Did.Find("adds-entries").ReportEntries[1].Name).Should(Equal("engineering"))
+			Ω(reporter.Did.Find("adds-entries").ReportEntries[1].Value.String()).Should(Equal("on fire"))
+			Ω(reporter.Did.Find("adds-no-entries").ReportEntries).Should(BeEmpty())
+			Ω(reporter.Did.FindByLeafNodeType(types.NodeTypeBeforeSuite).ReportEntries[0].Name).Should(Equal("bridge"))
+			Ω(reporter.Did.FindByLeafNodeType(types.NodeTypeBeforeSuite).ReportEntries[0].Value.String()).Should(Equal("engaged"))
 		})
-		Ω(success).Should(BeTrue())
 	})
 
-	It("attaches entries to the report", func() {
-		Ω(reporter.Did.Find("adds-entries").ReportEntries[0].Name).Should(Equal("medical"))
-		Ω(reporter.Did.Find("adds-entries").ReportEntries[0].Value.String()).Should(Equal("healthy"))
-		Ω(reporter.Did.Find("adds-entries").ReportEntries[1].Name).Should(Equal("engineering"))
-		Ω(reporter.Did.Find("adds-entries").ReportEntries[1].Value.String()).Should(Equal("on fire"))
-		Ω(reporter.Did.Find("adds-no-entries").ReportEntries).Should(BeEmpty())
-		Ω(reporter.Did.FindByLeafNodeType(types.NodeTypeBeforeSuite).ReportEntries[0].Name).Should(Equal("bridge"))
-		Ω(reporter.Did.FindByLeafNodeType(types.NodeTypeBeforeSuite).ReportEntries[0].Value.String()).Should(Equal("engaged"))
+	Context("avoiding races", func() {
+		BeforeEach(func() {
+			success, _ := RunFixture("Report Entries - but no races", func() {
+				BeforeEach(func() {
+					stop := make(chan interface{})
+					done := make(chan interface{})
+					ticker := time.NewTicker(10 * time.Millisecond)
+					i := 0
+					go func() {
+						for {
+							select {
+							case <-ticker.C:
+								AddReportEntry(fmt.Sprintf("report-%d", i))
+								i++
+							case <-stop:
+								ticker.Stop()
+								close(done)
+								return
+							}
+						}
+					}()
+					DeferCleanup(func() {
+						close(stop)
+						<-done
+					})
+				})
+
+				It("reporter", func() {
+					for i := 0; i < 5; i++ {
+						time.Sleep(20 * time.Millisecond)
+						AddReportEntry(fmt.Sprintf("waiting... %d", i))
+						Ω(len(CurrentSpecReport().ReportEntries)).Should(BeNumerically("<", (i+1)*10))
+					}
+				})
+
+				ReportAfterEach(func(report SpecReport) {
+					//no races here, either
+					Ω(len(report.ReportEntries)).Should(BeNumerically(">", 5))
+				})
+
+			})
+			Ω(success).Should(BeTrue())
+		})
+
+		It("attaches entries without racing", func() {
+			Ω(reporter.Did.Find("reporter").ReportEntries).Should(ContainElement(HaveField("Name", "report-0")))
+			Ω(reporter.Did.Find("reporter").ReportEntries).Should(ContainElement(HaveField("Name", "report-2")))
+			Ω(reporter.Did.Find("reporter").ReportEntries).Should(ContainElement(HaveField("Name", "waiting... 1")))
+			Ω(reporter.Did.Find("reporter").ReportEntries).Should(ContainElement(HaveField("Name", "waiting... 3")))
+		})
 	})
 })

internal/suite.go

@@ -2,6 +2,7 @@ package internal
 
 import (
 	"fmt"
+	"sync"
 	"time"
 
 	"github.com/onsi/ginkgo/v2/formatter"
@@ -35,18 +36,20 @@ type Suite struct {
 	interruptHandler  interrupt_handler.InterruptHandlerInterface
 	config            types.SuiteConfig
 
-	skipAll           bool
-	report            types.Report
-	currentSpecReport types.SpecReport
-	currentNode       Node
+	skipAll                         bool
+	report                          types.Report
+	currentSpecReport               types.SpecReport
+	currentSpecReportUserAccessLock *sync.Mutex
+	currentNode                     Node
 
 	client parallel_support.Client
 }
 
 func NewSuite() *Suite {
 	return &Suite{
-		tree:  &TreeNode{},
-		phase: PhaseBuildTopLevel,
+		tree:                            &TreeNode{},
+		phase:                           PhaseBuildTopLevel,
+		currentSpecReportUserAccessLock: &sync.Mutex{},
 	}
 }
 
@@ -212,14 +215,20 @@ func (suite *Suite) pushCleanupNode(node Node) error {
   Spec Running methods - used during PhaseRun
 */
 func (suite *Suite) CurrentSpecReport() types.SpecReport {
+	suite.currentSpecReportUserAccessLock.Lock()
+	defer suite.currentSpecReportUserAccessLock.Unlock()
 	report := suite.currentSpecReport
 	if suite.writer != nil {
 		report.CapturedGinkgoWriterOutput = string(suite.writer.Bytes())
 	}
+	report.ReportEntries = make([]ReportEntry, len(report.ReportEntries))
+	copy(report.ReportEntries, suite.currentSpecReport.ReportEntries)
 	return report
 }
 
 func (suite *Suite) AddReportEntry(entry ReportEntry) error {
+	suite.currentSpecReportUserAccessLock.Lock()
+	defer suite.currentSpecReportUserAccessLock.Unlock()
 	if suite.phase != PhaseRun {
 		return types.GinkgoErrors.AddReportEntryNotDuringRunPhase(entry.Location)
 	}