From dfef62add141dd9de7858aef46e0dcbee47d1915 Mon Sep 17 00:00:00 2001 From: Onsi Fakhouri Date: Tue, 30 Aug 2022 10:39:38 -0600 Subject: [PATCH] Add SuppressProgressReporting decorator to turn off --progress announcements for a given node Fixes #1023 --- decorator_dsl.go | 6 + docs/index.md | 15 ++- dsl/decorators/decorators_dsl.go | 1 + .../config_progress_test.go | 20 ++- internal/node.go | 59 ++++----- internal/node_test.go | 120 ++++++++++++------ internal/suite.go | 2 +- internal/suite_test.go | 4 +- reporting_dsl.go | 14 +- 9 files changed, 164 insertions(+), 77 deletions(-) diff --git a/decorator_dsl.go b/decorator_dsl.go index f23e526f3..6ef4b7598 100644 --- a/decorator_dsl.go +++ b/decorator_dsl.go @@ -80,3 +80,9 @@ Labels are the type for spec Label decorators. Use Label(...) to construct Labe You can learn more here: https://onsi.github.io/ginkgo/#spec-labels */ type Labels = internal.Labels + +/* +SuppressProgressReporting is a decorator that allows you to disable progress reporting of a particular node. This is useful if `ginkgo -v -progress` is generating too much noise; particularly +if you have a `ReportAfterEach` node that is running for every skipped spec and is generating lots of progress reports. +*/ +const SuppressProgressReporting = internal.SuppressProgressReporting diff --git a/docs/index.md b/docs/index.md index 55be4ebfd..0bb49dff0 100644 --- a/docs/index.md +++ b/docs/index.md @@ -2615,7 +2615,7 @@ There are a couple more flags that are verbosity-related but can be controlled i First, you can tell Ginkgo to always emit the `GinkgoWriter` output of every spec with `--always-emit-ginkgo-writer`. This will emit `GinkgoWriter` output for both failed _and_ passing specs, regardless of verbosity setting. -Second, you can tell Ginkgo to emit progress of a spec as Ginkgo runs each of its node closures. You do this with `ginkgo --progress -v` (or `-vv`). `--progress` will emit a message to the `GinkgoWriter` just before a node starts running. By running with `-v` or `-vv` you can then stream the output to the `GinkgoWriter` immediately. `--progress` was initially introduced to help debug specs that are stuck/hanging. It is not longer necessary as Ginkgo's behavior during an interrupt has matured and now generally has enough information to help you identify where a spec is stuck. +Second, you can tell Ginkgo to emit progress of a spec as Ginkgo runs each of its node closures. You do this with `ginkgo --progress -v` (or `-vv`). `--progress` will emit a message to the `GinkgoWriter` just before a node starts running. By running with `-v` or `-vv` you can then stream the output to the `GinkgoWriter` immediately. `--progress` was initially introduced to help debug specs that are stuck/hanging but is no longer strictly necessary as Ginkgo's behavior during an interrupt has matured and now generally has enough information to help you identify where a spec is stuck. If you, nonetheless, want to run with `--progress` but want to suppress output of individual nodes (e.g. a top-level `ReportAfterEach` that always runs even if a spec is skipped) you can pass the `SuppressProgressOuput` decorator to the node in question. #### Other Settings Here are a grab bag of other settings: @@ -4369,6 +4369,19 @@ With this setup, `"is flaky"` and `"is also flaky"` will run up to 3 times. `"i If `ginkgo --flake-attempts=N` is set the value passed in by the CLI will override all the decorated values. Every test will now run up to `N` times. + +#### The SuppressProgressOutput Decorator + +When running with `ginkgo -v -progress` Ginkgo will emit information about each node just before it runs. This information goes to the `GinkgoWriter` and straight to the console if using `-v`. There are contexts when this can be overly noisy. In particular, `ReportBeforeEach` and `ReportAfterEach` nodes always run, even when a spec is skipped. This can make Ginkgo's output noise when running with `-v -progress` as each `Report*Each` node will be announced, even for skipped specs. + +The `SuppressProgressOutput` decorator allows you to disable progress reporting for a given node: + +```go +ReportAfterEach(func(report SpecReport) { + //... +}, SuppressProgressReporting) +``` + ## Ginkgo CLI Overview This chapter provides a quick overview and tour of the Ginkgo CLI. For comprehensive details about all of the Ginkgo CLI's flags, run `ginkgo help`. To get information about Ginkgo's implicit `run` command (i.e. what you get when you just run `ginkgo`) run `ginkgo help run`. diff --git a/dsl/decorators/decorators_dsl.go b/dsl/decorators/decorators_dsl.go index c41a49c65..50f2b7f97 100644 --- a/dsl/decorators/decorators_dsl.go +++ b/dsl/decorators/decorators_dsl.go @@ -25,5 +25,6 @@ const Pending = ginkgo.Pending const Serial = ginkgo.Serial const Ordered = ginkgo.Ordered const OncePerOrdered = ginkgo.OncePerOrdered +const SuppressProgressReporting = ginkgo.SuppressProgressReporting var Label = ginkgo.Label diff --git a/internal/internal_integration/config_progress_test.go b/internal/internal_integration/config_progress_test.go index 3037e535b..5cc3b4121 100644 --- a/internal/internal_integration/config_progress_test.go +++ b/internal/internal_integration/config_progress_test.go @@ -15,9 +15,9 @@ var _ = Describe("when config.EmitSpecProgress is enabled", func() { conf.EmitSpecProgress = true }) - It("emits progress to the writer as it goes", func() { + It("emits progress to the writer as it goes, but skips nodes marked SuppressProgressOutput", func() { l := types.NewCodeLocation(0) - RunFixture("emitting spec progress", func() { + success, _ := RunFixture("emitting spec progress", func() { BeforeSuite(func() { Ω(buffer).Should(gbytes.Say(`\[BeforeSuite\] TOP-LEVEL`)) Ω(buffer).Should(gbytes.Say(`%s:%d`, l.FileName, l.LineNumber+2)) @@ -35,16 +35,30 @@ var _ = Describe("when config.EmitSpecProgress is enabled", func() { Ω(buffer).Should(gbytes.Say(`\[It\] B`)) Ω(buffer).Should(gbytes.Say(`%s:\d+`, l.FileName)) }) + It("C", func() { + Ω(buffer).ShouldNot(gbytes.Say(`\[It\] C`)) + }, SuppressProgressReporting) AfterEach(func() { Ω(buffer).Should(gbytes.Say(`\[AfterEach\] a container`)) Ω(buffer).Should(gbytes.Say(`%s:\d+`, l.FileName)) }) + ReportAfterEach(func(_ SpecReport) { + Ω(buffer).Should(gbytes.Say(`\[ReportAfterEach\] a container`)) + Ω(buffer).Should(gbytes.Say(`%s:\d+`, l.FileName)) + }) }) AfterSuite(func() { Ω(buffer).Should(gbytes.Say(`\[AfterSuite\] TOP-LEVEL`)) Ω(buffer).Should(gbytes.Say(`%s:\d+`, l.FileName)) }) + ReportAfterEach(func(_ SpecReport) { + Ω(buffer).Should(gbytes.Say(`\[ReportAfterEach\] TOP-LEVEL`)) + Ω(buffer).Should(gbytes.Say(`%s:\d+`, l.FileName)) + }) + ReportAfterEach(func(_ SpecReport) { + Ω(buffer).ShouldNot(gbytes.Say(`\[ReportAfterEach\] TOP-LEVEL`)) + }, SuppressProgressReporting) }) - + Ω(success).Should(BeTrue()) }) }) diff --git a/internal/node.go b/internal/node.go index 289e4dde2..dc11129e6 100644 --- a/internal/node.go +++ b/internal/node.go @@ -40,13 +40,14 @@ type Node struct { ReportEachBody func(types.SpecReport) ReportAfterSuiteBody func(types.Report) - MarkedFocus bool - MarkedPending bool - MarkedSerial bool - MarkedOrdered bool - MarkedOncePerOrdered bool - FlakeAttempts int - Labels Labels + MarkedFocus bool + MarkedPending bool + MarkedSerial bool + MarkedOrdered bool + MarkedOncePerOrdered bool + MarkedSuppressProgressReporting bool + FlakeAttempts int + Labels Labels NodeIDWhereCleanupWasGenerated uint } @@ -57,12 +58,14 @@ type pendingType bool type serialType bool type orderedType bool type honorsOrderedType bool +type suppressProgressReporting bool const Focus = focusType(true) const Pending = pendingType(true) const Serial = serialType(true) const Ordered = orderedType(true) const OncePerOrdered = honorsOrderedType(true) +const SuppressProgressReporting = suppressProgressReporting(true) type FlakeAttempts uint type Offset uint @@ -114,6 +117,8 @@ func isDecoration(arg interface{}) bool { return true case t == reflect.TypeOf(OncePerOrdered): return true + case t == reflect.TypeOf(SuppressProgressReporting): + return true case t == reflect.TypeOf(FlakeAttempts(0)): return true case t == reflect.TypeOf(Labels{}): @@ -176,7 +181,6 @@ func NewNode(deprecationTracker *types.DeprecationTracker, nodeType types.NodeTy remainingArgs = []interface{}{} //now process the rest of the args for _, arg := range args { - switch t := reflect.TypeOf(arg); { case t == reflect.TypeOf(float64(0)): break //ignore deprecated timeouts @@ -205,6 +209,11 @@ func NewNode(deprecationTracker *types.DeprecationTracker, nodeType types.NodeTy if !nodeType.Is(types.NodeTypeBeforeEach | types.NodeTypeJustBeforeEach | types.NodeTypeAfterEach | types.NodeTypeJustAfterEach) { appendError(types.GinkgoErrors.InvalidDecoratorForNodeType(node.CodeLocation, nodeType, "OncePerOrdered")) } + case t == reflect.TypeOf(SuppressProgressReporting): + node.MarkedSuppressProgressReporting = bool(arg.(suppressProgressReporting)) + if nodeType.Is(types.NodeTypeContainer) { + appendError(types.GinkgoErrors.InvalidDecoratorForNodeType(node.CodeLocation, nodeType, "SuppressProgressReporting")) + } case t == reflect.TypeOf(FlakeAttempts(0)): node.FlakeAttempts = int(arg.(FlakeAttempts)) if !nodeType.Is(types.NodeTypesForContainerAndIt) { @@ -223,6 +232,18 @@ func NewNode(deprecationTracker *types.DeprecationTracker, nodeType types.NodeTy } } case t.Kind() == reflect.Func: + if nodeType.Is(types.NodeTypeReportBeforeEach | types.NodeTypeReportAfterEach) { + if node.ReportEachBody != nil { + appendError(types.GinkgoErrors.MultipleBodyFunctions(node.CodeLocation, nodeType)) + trackedFunctionError = true + break + } + + //we can trust that the function is valid because the compiler has our back here + node.ReportEachBody = arg.(func(types.SpecReport)) + break + } + if node.Body != nil { appendError(types.GinkgoErrors.MultipleBodyFunctions(node.CodeLocation, nodeType)) trackedFunctionError = true @@ -251,7 +272,7 @@ func NewNode(deprecationTracker *types.DeprecationTracker, nodeType types.NodeTy appendError(types.GinkgoErrors.InvalidDeclarationOfFocusedAndPending(node.CodeLocation, nodeType)) } - if node.Body == nil && !node.MarkedPending && !trackedFunctionError { + if node.Body == nil && node.ReportEachBody == nil && !node.MarkedPending && !trackedFunctionError { appendError(types.GinkgoErrors.MissingBodyFunction(node.CodeLocation, nodeType)) } for _, arg := range remainingArgs { @@ -285,26 +306,6 @@ func NewSynchronizedAfterSuiteNode(allProcsBody func(), proc1Body func(), codeLo }, nil } -func NewReportBeforeEachNode(body func(types.SpecReport), codeLocation types.CodeLocation) (Node, []error) { - return Node{ - ID: UniqueNodeID(), - NodeType: types.NodeTypeReportBeforeEach, - ReportEachBody: body, - CodeLocation: codeLocation, - NestingLevel: -1, - }, nil -} - -func NewReportAfterEachNode(body func(types.SpecReport), codeLocation types.CodeLocation) (Node, []error) { - return Node{ - ID: UniqueNodeID(), - NodeType: types.NodeTypeReportAfterEach, - ReportEachBody: body, - CodeLocation: codeLocation, - NestingLevel: -1, - }, nil -} - func NewReportAfterSuiteNode(text string, body func(types.Report), codeLocation types.CodeLocation) (Node, []error) { return Node{ ID: UniqueNodeID(), diff --git a/internal/node_test.go b/internal/node_test.go index 515e5fd8a..19bf6f831 100644 --- a/internal/node_test.go +++ b/internal/node_test.go @@ -35,6 +35,7 @@ var _ = Describe("Partitioning Decorations", func() { Pending, Serial, Ordered, + SuppressProgressReporting, nil, 1, []interface{}{Focus, Pending, []interface{}{Offset(2), Serial, FlakeAttempts(2)}, Ordered, Label("a", "b", "c")}, @@ -54,6 +55,7 @@ var _ = Describe("Partitioning Decorations", func() { Pending, Serial, Ordered, + SuppressProgressReporting, []interface{}{Focus, Pending, []interface{}{Offset(2), Serial, FlakeAttempts(2)}, Ordered, Label("a", "b", "c")}, Label("A", "B", "C"), Label("D"), @@ -97,7 +99,7 @@ var _ = Describe("Constructing nodes", func() { Describe("happy path", func() { It("creates a node with a non-zero id", func() { - node, errors := internal.NewNode(dt, ntIt, "text", body, cl, Focus, Label("A", "B", "C")) + node, errors := internal.NewNode(dt, ntIt, "text", body, cl, Focus, Label("A", "B", "C"), SuppressProgressReporting) Ω(node.ID).Should(BeNumerically(">", 0)) Ω(node.NodeType).Should(Equal(ntIt)) Ω(node.Text).Should(Equal("text")) @@ -107,11 +109,52 @@ var _ = Describe("Constructing nodes", func() { Ω(node.MarkedFocus).Should(BeTrue()) Ω(node.MarkedPending).Should(BeFalse()) Ω(node.NestingLevel).Should(Equal(-1)) + Ω(node.MarkedSuppressProgressReporting).Should(BeTrue()) Ω(node.Labels).Should(Equal(Labels{"A", "B", "C"})) ExpectAllWell(errors) }) }) + Describe("Building ReportBeforeEach nodes", func() { + It("returns a correctly configured node", func() { + var didRun bool + body := func(types.SpecReport) { didRun = true } + + node, errors := internal.NewNode(dt, types.NodeTypeReportBeforeEach, "", body, cl) + Ω(errors).Should(BeEmpty()) + Ω(node.ID).Should(BeNumerically(">", 0)) + Ω(node.NodeType).Should(Equal(types.NodeTypeReportBeforeEach)) + + node.ReportEachBody(types.SpecReport{}) + Ω(didRun).Should(BeTrue()) + + Ω(node.Body).Should(BeNil()) + + Ω(node.CodeLocation).Should(Equal(cl)) + Ω(node.NestingLevel).Should(Equal(-1)) + }) + }) + + Describe("Building ReportAfterEach nodes", func() { + It("returns a correctly configured node", func() { + var didRun bool + body := func(types.SpecReport) { didRun = true } + + node, errors := internal.NewNode(dt, types.NodeTypeReportAfterEach, "", body, cl) + Ω(errors).Should(BeEmpty()) + Ω(node.ID).Should(BeNumerically(">", 0)) + Ω(node.NodeType).Should(Equal(types.NodeTypeReportAfterEach)) + + node.ReportEachBody(types.SpecReport{}) + Ω(didRun).Should(BeTrue()) + + Ω(node.Body).Should(BeNil()) + + Ω(node.CodeLocation).Should(Equal(cl)) + Ω(node.NestingLevel).Should(Equal(-1)) + }) + }) + Describe("Assigning CodeLocation", func() { Context("with nothing explicitly specified ", func() { It("assumes a base-offset of 2", func() { @@ -273,6 +316,37 @@ var _ = Describe("Constructing nodes", func() { }) }) + Describe("the SuppressProgressReporting decoration", func() { + It("the node does not SuppressProgressReporting by default", func() { + node, errors := internal.NewNode(dt, ntIt, "text", body) + Ω(node.MarkedSuppressProgressReporting).Should(BeFalse()) + ExpectAllWell(errors) + }) + It("marks the node as SuppressProgressReporting", func() { + node, errors := internal.NewNode(dt, ntIt, "", body, SuppressProgressReporting) + Ω(node.MarkedSuppressProgressReporting).Should(BeTrue()) + ExpectAllWell(errors) + }) + It("does not allow container nodes to be marked", func() { + node, errors := internal.NewNode(dt, ntCon, "", body, cl, SuppressProgressReporting) + Ω(node).Should(BeZero()) + Ω(errors).Should(ConsistOf(types.GinkgoErrors.InvalidDecoratorForNodeType(cl, ntCon, "SuppressProgressReporting"))) + Ω(dt.DidTrackDeprecations()).Should(BeFalse()) + + node, errors = internal.NewNode(dt, ntIt, "text", body, cl, SuppressProgressReporting) + Ω(node.MarkedSuppressProgressReporting).Should(BeTrue()) + ExpectAllWell(errors) + + node, errors = internal.NewNode(dt, ntBef, "", body, cl, SuppressProgressReporting) + Ω(node.MarkedSuppressProgressReporting).Should(BeTrue()) + ExpectAllWell(errors) + + node, errors = internal.NewNode(dt, types.NodeTypeReportAfterEach, "", func(_ SpecReport) {}, cl, SuppressProgressReporting) + Ω(node.MarkedSuppressProgressReporting).Should(BeTrue()) + ExpectAllWell(errors) + }) + }) + Describe("The Label decoration", func() { It("has no labels by default", func() { node, errors := internal.NewNode(dt, ntIt, "text", body) @@ -341,6 +415,14 @@ var _ = Describe("Constructing nodes", func() { Ω(dt.DidTrackDeprecations()).Should(BeFalse()) }) + It("errors if more than one function is provided for a ReportBeforeEach/ReportAFterEach node", func() { + reportBody := func(types.SpecReport) {} + node, errors := internal.NewNode(dt, types.NodeTypeReportAfterEach, "", reportBody, body, cl) + Ω(node).Should(BeZero()) + Ω(errors).Should(ConsistOf(types.GinkgoErrors.MultipleBodyFunctions(cl, types.NodeTypeReportAfterEach))) + Ω(dt.DidTrackDeprecations()).Should(BeFalse()) + }) + It("errors if the function has a return value", func() { f := func() string { return "" } node, errors := internal.NewNode(dt, ntIt, "text", f, cl) @@ -449,42 +531,6 @@ var _ = Describe("Node", func() { }) }) - Describe("NewReportBeforeEachNode", func() { - It("returns a correctly configured node", func() { - var didRun bool - body := func(types.SpecReport) { didRun = true } - - node, errors := internal.NewReportBeforeEachNode(body, cl) - Ω(errors).Should(BeEmpty()) - Ω(node.ID).Should(BeNumerically(">", 0)) - Ω(node.NodeType).Should(Equal(types.NodeTypeReportBeforeEach)) - - node.ReportEachBody(types.SpecReport{}) - Ω(didRun).Should(BeTrue()) - - Ω(node.CodeLocation).Should(Equal(cl)) - Ω(node.NestingLevel).Should(Equal(-1)) - }) - }) - - Describe("NewReportAfterEachNode", func() { - It("returns a correctly configured node", func() { - var didRun bool - body := func(types.SpecReport) { didRun = true } - - node, errors := internal.NewReportAfterEachNode(body, cl) - Ω(errors).Should(BeEmpty()) - Ω(node.ID).Should(BeNumerically(">", 0)) - Ω(node.NodeType).Should(Equal(types.NodeTypeReportAfterEach)) - - node.ReportEachBody(types.SpecReport{}) - Ω(didRun).Should(BeTrue()) - - Ω(node.CodeLocation).Should(Equal(cl)) - Ω(node.NestingLevel).Should(Equal(-1)) - }) - }) - Describe("NewReportAfterSuiteNode", func() { It("returns a correctly configured node", func() { var didRun bool diff --git a/internal/suite.go b/internal/suite.go index 930556553..e27a29ddb 100644 --- a/internal/suite.go +++ b/internal/suite.go @@ -569,7 +569,7 @@ func (suite *Suite) runNode(node Node, interruptChannel chan interface{}, text s suite.currentNode = Node{} }() - if suite.config.EmitSpecProgress { + if suite.config.EmitSpecProgress && !node.MarkedSuppressProgressReporting { if text == "" { text = "TOP-LEVEL" } diff --git a/internal/suite_test.go b/internal/suite_test.go index 18a7bed63..2d715b551 100644 --- a/internal/suite_test.go +++ b/internal/suite_test.go @@ -289,7 +289,7 @@ var _ = Describe("Suite", func() { Context("when pushing a cleanup node in a ReportBeforeEach node", func() { It("errors", func() { var errors = make([]error, 4) - reportBeforeEachNode, _ := internal.NewReportBeforeEachNode(func(_ types.SpecReport) { + reportBeforeEachNode := N(types.NodeTypeReportBeforeEach, func(_ types.SpecReport) { errors[3] = suite.PushNode(N(types.NodeTypeCleanupInvalid, cl)) }, types.NewCodeLocation(0)) @@ -311,7 +311,7 @@ var _ = Describe("Suite", func() { Context("when pushing a cleanup node in a ReportAfterEach node", func() { It("errors", func() { var errors = make([]error, 4) - reportAfterEachNode, _ := internal.NewReportAfterEachNode(func(_ types.SpecReport) { + reportAfterEachNode := N(types.NodeTypeReportAfterEach, func(_ types.SpecReport) { errors[3] = suite.PushNode(N(types.NodeTypeCleanupInvalid, cl)) }, types.NewCodeLocation(0)) diff --git a/reporting_dsl.go b/reporting_dsl.go index 07b4c6122..8750236d9 100644 --- a/reporting_dsl.go +++ b/reporting_dsl.go @@ -79,8 +79,11 @@ receives a SpecReport. They are called before the spec starts. You cannot nest any other Ginkgo nodes within a ReportBeforeEach node's closure. You can learn more about ReportBeforeEach here: https://onsi.github.io/ginkgo/#generating-reports-programmatically */ -func ReportBeforeEach(body func(SpecReport)) bool { - return pushNode(internal.NewReportBeforeEachNode(body, types.NewCodeLocation(1))) +func ReportBeforeEach(body func(SpecReport), args ...interface{}) bool { + combinedArgs := []interface{}{body} + combinedArgs = append(combinedArgs, args...) + + return pushNode(internal.NewNode(deprecationTracker, types.NodeTypeReportBeforeEach, "", combinedArgs...)) } /* @@ -90,8 +93,11 @@ receives a SpecReport. They are called after the spec has completed and receive You cannot nest any other Ginkgo nodes within a ReportAfterEach node's closure. You can learn more about ReportAfterEach here: https://onsi.github.io/ginkgo/#generating-reports-programmatically */ -func ReportAfterEach(body func(SpecReport)) bool { - return pushNode(internal.NewReportAfterEachNode(body, types.NewCodeLocation(1))) +func ReportAfterEach(body func(SpecReport), args ...interface{}) bool { + combinedArgs := []interface{}{body} + combinedArgs = append(combinedArgs, args...) + + return pushNode(internal.NewNode(deprecationTracker, types.NodeTypeReportAfterEach, "", combinedArgs...)) } /*