diff --git a/internal/internal_integration/progress_report_test.go b/internal/internal_integration/progress_report_test.go index 21ffa6500..742227bcf 100644 --- a/internal/internal_integration/progress_report_test.go +++ b/internal/internal_integration/progress_report_test.go @@ -4,6 +4,7 @@ import ( "time" . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/ginkgo/v2/internal/test_helpers" "github.com/onsi/ginkgo/v2/types" . "github.com/onsi/gomega" ) @@ -478,4 +479,26 @@ var _ = Describe("Progress Reporting", func() { Ω(pr.AdditionalReports).Should(Equal([]string{"Some More (Never Cancelled) Global Information"})) }) }) + + Context("when a global progress reporter fails", func() { + BeforeEach(func() { + success, _ := RunFixture("emitting spec progress", func() { + Describe("a container", func() { + It("A", func(ctx SpecContext) { + AttachProgressReporter(func() string { + F("bam") + return "Some Global Information" + }) + triggerProgressSignal() + }) + }) + }) + Ω(success).Should(BeFalse()) + }) + + It("marks the spec as failed", func() { + Ω(reporter.Did.Find("A")).Should(HaveFailed("bam")) + }) + }) + }) diff --git a/internal/progress_reporter_manager.go b/internal/progress_reporter_manager.go index bea4771d1..a39e1f587 100644 --- a/internal/progress_reporter_manager.go +++ b/internal/progress_reporter_manager.go @@ -4,6 +4,8 @@ import ( "context" "sort" "sync" + + "github.com/onsi/ginkgo/v2/types" ) type ProgressReporterManager struct { @@ -33,7 +35,7 @@ func (prm *ProgressReporterManager) AttachProgressReporter(reporter func() strin } } -func (prm *ProgressReporterManager) QueryProgressReporters(ctx context.Context) []string { +func (prm *ProgressReporterManager) QueryProgressReporters(ctx context.Context, failer *Failer) []string { prm.lock.Lock() keys := []int{} for key := range prm.progressReporters { @@ -53,6 +55,13 @@ func (prm *ProgressReporterManager) QueryProgressReporters(ctx context.Context) for _, reporter := range reporters { reportC := make(chan string, 1) go func() { + defer func() { + e := recover() + if e != nil { + failer.Panic(types.NewCodeLocationWithStackTrace(1), e) + reportC <- "failed to query attached progress reporter" + } + }() reportC <- reporter() }() var report string diff --git a/internal/progress_reporter_manager_test.go b/internal/progress_reporter_manager_test.go index e635ac2c2..06535cb46 100644 --- a/internal/progress_reporter_manager_test.go +++ b/internal/progress_reporter_manager_test.go @@ -5,6 +5,7 @@ import ( "time" . "github.com/onsi/ginkgo/v2" + "github.com/onsi/ginkgo/v2/types" . "github.com/onsi/gomega" "github.com/onsi/gomega/gleak" @@ -19,19 +20,19 @@ var _ = Describe("ProgressReporterManager", func() { }) It("can attach and detach progress reporters", func() { - Ω(manager.QueryProgressReporters(context.Background())).Should(BeEmpty()) + Ω(manager.QueryProgressReporters(context.Background(), nil)).Should(BeEmpty()) cancelA := manager.AttachProgressReporter(func() string { return "A" }) - Ω(manager.QueryProgressReporters(context.Background())).Should(Equal([]string{"A"})) + Ω(manager.QueryProgressReporters(context.Background(), nil)).Should(Equal([]string{"A"})) cancelB := manager.AttachProgressReporter(func() string { return "B" }) - Ω(manager.QueryProgressReporters(context.Background())).Should(Equal([]string{"A", "B"})) + Ω(manager.QueryProgressReporters(context.Background(), nil)).Should(Equal([]string{"A", "B"})) cancelC := manager.AttachProgressReporter(func() string { return "C" }) - Ω(manager.QueryProgressReporters(context.Background())).Should(Equal([]string{"A", "B", "C"})) + Ω(manager.QueryProgressReporters(context.Background(), nil)).Should(Equal([]string{"A", "B", "C"})) cancelB() - Ω(manager.QueryProgressReporters(context.Background())).Should(Equal([]string{"A", "C"})) + Ω(manager.QueryProgressReporters(context.Background(), nil)).Should(Equal([]string{"A", "C"})) cancelA() - Ω(manager.QueryProgressReporters(context.Background())).Should(Equal([]string{"C"})) + Ω(manager.QueryProgressReporters(context.Background(), nil)).Should(Equal([]string{"C"})) cancelC() - Ω(manager.QueryProgressReporters(context.Background())).Should(BeEmpty()) + Ω(manager.QueryProgressReporters(context.Background(), nil)).Should(BeEmpty()) }) It("bails if a progress reporter takes longer than the passed-in context's deadline", func() { @@ -45,11 +46,25 @@ var _ = Describe("ProgressReporterManager", func() { }) manager.AttachProgressReporter(func() string { return "D" }) context, cancel := context.WithTimeout(context.Background(), time.Millisecond*100) - result := manager.QueryProgressReporters(context) + result := manager.QueryProgressReporters(context, nil) Ω(result).Should(Equal([]string{"A", "B"})) cancel() close(c) Eventually(gleak.Goroutines).ShouldNot(gleak.HaveLeaked(startingGoroutines)) }) + + It("catches panics and reports them as failures", func() { + manager.AttachProgressReporter(func() string { + panic("bam") + }) + manager.AttachProgressReporter(func() string { return "B" }) + failer := internal.NewFailer() + result := manager.QueryProgressReporters(context.Background(), failer) + Ω(result).Should(Equal([]string{"failed to query attached progress reporter", "B"})) + state, failure := failer.Drain() + Ω(state).Should(Equal(types.SpecStatePanicked)) + Ω(failure.Message).Should(Equal("Test Panicked")) + Ω(failure.ForwardedPanic).Should(Equal("bam")) + }) }) diff --git a/internal/spec_context_test.go b/internal/spec_context_test.go index 9dddb4fb1..b4e0225da 100644 --- a/internal/spec_context_test.go +++ b/internal/spec_context_test.go @@ -4,6 +4,7 @@ import ( "context" . "github.com/onsi/ginkgo/v2" + "github.com/onsi/ginkgo/v2/internal" . "github.com/onsi/gomega" ) @@ -12,38 +13,38 @@ var _ = Describe("SpecContext", func() { Ω(c.SpecReport().LeafNodeText).Should(Equal("allows access to the current spec report")) }) - It("can be wrapped and still retreived", func(c SpecContext) { + It("can be wrapped and still retrieved", func(c SpecContext) { Ω(c.Value("GINKGO_SPEC_CONTEXT")).Should(Equal(c)) wrappedC := context.WithValue(c, "foo", "bar") _, ok := wrappedC.(SpecContext) Ω(ok).Should(BeFalse()) - Ω(wrappedC.Value("GINKGO_SPEC_CONTEXT").(SpecContext).SpecReport().LeafNodeText).Should(Equal("can be wrapped and still retreived")) + Ω(wrappedC.Value("GINKGO_SPEC_CONTEXT").(SpecContext).SpecReport().LeafNodeText).Should(Equal("can be wrapped and still retrieved")) }) It("can attach and detach progress reporters", func(c SpecContext) { type CompleteSpecContext interface { AttachProgressReporter(func() string) func() - QueryProgressReporters(ctx context.Context) []string + QueryProgressReporters(ctx context.Context, failer *internal.Failer) []string } wrappedC := context.WithValue(c, "foo", "bar") ctx := wrappedC.Value("GINKGO_SPEC_CONTEXT").(CompleteSpecContext) - Ω(ctx.QueryProgressReporters(context.Background())).Should(BeEmpty()) + Ω(ctx.QueryProgressReporters(context.Background(), nil)).Should(BeEmpty()) cancelA := ctx.AttachProgressReporter(func() string { return "A" }) - Ω(ctx.QueryProgressReporters(context.Background())).Should(Equal([]string{"A"})) + Ω(ctx.QueryProgressReporters(context.Background(), nil)).Should(Equal([]string{"A"})) cancelB := ctx.AttachProgressReporter(func() string { return "B" }) - Ω(ctx.QueryProgressReporters(context.Background())).Should(Equal([]string{"A", "B"})) + Ω(ctx.QueryProgressReporters(context.Background(), nil)).Should(Equal([]string{"A", "B"})) cancelC := ctx.AttachProgressReporter(func() string { return "C" }) - Ω(ctx.QueryProgressReporters(context.Background())).Should(Equal([]string{"A", "B", "C"})) + Ω(ctx.QueryProgressReporters(context.Background(), nil)).Should(Equal([]string{"A", "B", "C"})) cancelB() - Ω(ctx.QueryProgressReporters(context.Background())).Should(Equal([]string{"A", "C"})) + Ω(ctx.QueryProgressReporters(context.Background(), nil)).Should(Equal([]string{"A", "C"})) cancelA() - Ω(ctx.QueryProgressReporters(context.Background())).Should(Equal([]string{"C"})) + Ω(ctx.QueryProgressReporters(context.Background(), nil)).Should(Equal([]string{"C"})) cancelC() - Ω(ctx.QueryProgressReporters(context.Background())).Should(BeEmpty()) + Ω(ctx.QueryProgressReporters(context.Background(), nil)).Should(BeEmpty()) }) }) diff --git a/internal/suite.go b/internal/suite.go index 8f0283ca8..a1dbd4c62 100644 --- a/internal/suite.go +++ b/internal/suite.go @@ -348,9 +348,9 @@ func (suite *Suite) generateProgressReport(fullReport bool) types.ProgressReport defer cancel() var additionalReports []string if suite.currentSpecContext != nil { - additionalReports = append(additionalReports, suite.currentSpecContext.QueryProgressReporters(deadline)...) + additionalReports = append(additionalReports, suite.currentSpecContext.QueryProgressReporters(deadline, suite.failer)...) } - additionalReports = append(additionalReports, suite.QueryProgressReporters(deadline)...) + additionalReports = append(additionalReports, suite.QueryProgressReporters(deadline, suite.failer)...) gwOutput := suite.currentSpecReport.CapturedGinkgoWriterOutput + string(suite.writer.Bytes()) pr, err := NewProgressReport(suite.isRunningInParallel(), suite.currentSpecReport, suite.currentNode, suite.currentNodeStartTime, suite.currentByStep, gwOutput, timelineLocation, additionalReports, suite.config.SourceRoots, fullReport)