Programmatic focus is no longer overwrriten by CLI filters

Author: onsi

docs/index.md

@@ -2597,8 +2597,8 @@ These mechanisms can all be used in concert.  They combine with the following ru
 
 - `Pending` specs are always pending and can never be coerced to run by another filtering mechanism.
 - Specs that invoke `Skip()` will always be skipped regardless of other filtering mechanisms.
-- The CLI based filters (`--label-filter`, `--focus-file/--skip-file`, `--focus/--skip`) **always** override any programmatic focus.
-- When multiple CLI filters are provided they are all ANDed together.  The spec must satisfy the label filter query **and** any location-based filters **and** any description based filters.
+- Programmatic filters always apply and result in a non-zero exit code.  Any additional CLI filters only apply to the subset of specs selected by the programmatic filters.
+- When multiple CLI filters (`--label-filter`, `--focus-file/--skip-file`, `--focus/--skip`) are provided they are all ANDed together.  The spec must satisfy the label filter query **and** any location-based filters **and** any description based filters.
 
 ### Repeating Spec Runs and Managing Flaky Specs
 

integration/_fixtures/filter_fixture/sprocket_c_test.go

@@ -4,7 +4,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 )
 
-var _ = FDescribe("SprocketC", func() {
+var _ = Describe("SprocketC", func() {
 	It("cat", func() {
 
 	})

integration/_fixtures/flags_fixture/flags_test.go

@@ -18,66 +18,56 @@ func init() {
 }
 
 var _ = Describe("Testing various flags", func() {
-	FDescribe("the focused set", func() {
-		It("should honor -cover", func() {
-			Ω(Tested()).Should(Equal("tested"))
-		})
-
-		It("should allow gcflags", func() {
-			fmt.Printf("NaN returns %T\n", remapped.NaN())
-		})
+	It("should honor -cover", func() {
+		Ω(Tested()).Should(Equal("tested"))
+	})
 
-		PIt("should honor -failOnPending and -noisyPendings")
+	It("should allow gcflags", func() {
+		fmt.Printf("NaN returns %T\n", remapped.NaN())
+	})
 
-		Describe("smores", func() {
-			It("should honor -skip: marshmallow", func() {
-				println("marshmallow")
-			})
+	PIt("should honor -failOnPending and -noisyPendings")
 
-			It("should honor -focus: chocolate", func() {
-				println("chocolate")
-			})
+	Describe("smores", func() {
+		It("should honor -skip: marshmallow", func() {
+			println("marshmallow")
 		})
 
-		It("should detect races", func() {
-			var a string
-			c := make(chan interface{}, 0)
-			go func() {
-				a = "now you don't"
-				close(c)
-			}()
-			a = "now you see me"
-			println(a)
-			Eventually(c).Should(BeClosed())
+		It("should honor -focus: chocolate", func() {
+			println("chocolate")
 		})
+	})
 
-		It("should randomize A", func() {
-			println("RANDOM_A")
-		})
+	It("should detect races", func() {
+		var a string
+		c := make(chan interface{}, 0)
+		go func() {
+			a = "now you don't"
+			close(c)
+		}()
+		a = "now you see me"
+		println(a)
+		Eventually(c).Should(BeClosed())
+	})
 
-		It("should randomize B", func() {
-			println("RANDOM_B")
-		})
+	It("should randomize A", func() {
+		println("RANDOM_A")
+	})
 
-		It("should randomize C", func() {
-			println("RANDOM_C")
-		})
+	It("should randomize B", func() {
+		println("RANDOM_B")
+	})
 
-		It("should pass in additional arguments after '--' directly to the test process", func() {
-			fmt.Printf("CUSTOM_FLAG: %s", customFlag)
-		})
+	It("should randomize C", func() {
+		println("RANDOM_C")
 	})
 
-	Describe("more smores", func() {
-		It("should not run these unless -focus is set", func() {
-			println("smores")
-		})
+	It("should pass in additional arguments after '--' directly to the test process", func() {
+		fmt.Printf("CUSTOM_FLAG: %s", customFlag)
 	})
 
-	Describe("a failing test", func() {
-		It("should fail", func() {
-			Ω(true).Should(Equal(false))
-		})
+	It("should fail", func() {
+		Ω(true).Should(Equal(false))
 	})
 
 	Describe("a flaky test", func() {

integration/filter_test.go

@@ -1,15 +1,12 @@
 package integration_test
 
 import (
-	"strings"
-
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/gbytes"
 	"github.com/onsi/gomega/gexec"
 
 	. "github.com/onsi/ginkgo/v2/internal/test_helpers"
-	"github.com/onsi/ginkgo/v2/types"
 )
 
 var _ = Describe("Filter", func() {
@@ -76,14 +73,10 @@ var _ = Describe("Filter", func() {
 			"--focus=", "--skip=",
 			"--json-report=report.json",
 		)
-		Eventually(session).Should(gexec.Exit(types.GINKGO_FOCUS_EXIT_CODE))
+		Eventually(session).Should(gexec.Exit(0))
 		specs := Reports(fm.LoadJSONReports("filter", "report.json")[0].SpecReports)
 		for _, spec := range specs {
-			if strings.HasPrefix(spec.FullText(), "SprocketC") {
-				Ω(spec).Should(HavePassed())
-			} else {
-				Ω(spec).Should(Or(HaveBeenSkipped(), BePending()))
-			}
+			Ω(spec).Should(SatisfyAny(HavePassed(), BePending()))
 		}
 	})
 

integration/flags_test.go

@@ -4,7 +4,6 @@ import (
 	"strings"
 
 	. "github.com/onsi/ginkgo/v2"
-	"github.com/onsi/ginkgo/v2/types"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/gexec"
 )
@@ -20,21 +19,20 @@ var _ = Describe("Flags Specs", func() {
 
 	It("normally passes, prints out noisy pendings, does not randomize tests, and honors the programmatic focus", func() {
 		session := startGinkgo(fm.PathTo("flags"), "--no-color")
-		Eventually(session).Should(gexec.Exit(types.GINKGO_FOCUS_EXIT_CODE))
+		Eventually(session).Should(gexec.Exit(1))
 		output := string(session.Out.Contents())
 
 		Ω(output).Should(ContainSubstring("9 Passed"))
-		Ω(output).Should(ContainSubstring("0 Failed"))
+		Ω(output).Should(ContainSubstring("2 Failed"))
 		Ω(output).Should(ContainSubstring("1 Pending"))
-		Ω(output).Should(ContainSubstring("3 Skipped"))
+		Ω(output).Should(ContainSubstring("0 Skipped"))
 		Ω(output).Should(ContainSubstring("[PENDING]"))
 		Ω(output).Should(ContainSubstring("marshmallow"))
 		Ω(output).Should(ContainSubstring("chocolate"))
 		Ω(output).Should(ContainSubstring("CUSTOM_FLAG: default"))
-		Ω(output).Should(ContainSubstring("Detected Programmatic Focus - setting exit status to %d", types.GINKGO_FOCUS_EXIT_CODE))
-		Ω(output).ShouldNot(ContainSubstring("smores"))
 		Ω(output).ShouldNot(ContainSubstring("SLOW TEST"))
 		Ω(output).ShouldNot(ContainSubstring("should honor -slow-spec-threshold"))
+		Ω(output).ShouldNot(ContainSubstring("Full Stack Trace"))
 
 		orders := getRandomOrders(output)
 		Ω(orders[0]).Should(BeNumerically("<", orders[1]))
@@ -53,15 +51,15 @@ var _ = Describe("Flags Specs", func() {
 			Skip("race detection is not supported")
 		}
 		session := startGinkgo(fm.PathTo("flags"), "--no-color", "--race")
-		Eventually(session).Should(gexec.Exit(types.GINKGO_FOCUS_EXIT_CODE))
+		Eventually(session).Should(gexec.Exit(1))
 		output := string(session.Out.Contents())
 
 		Ω(output).Should(ContainSubstring("WARNING: DATA RACE"))
 	})
 
 	It("should randomize tests when told to", func() {
-		session := startGinkgo(fm.PathTo("flags"), "--no-color", "--randomize-all", "--seed=40")
-		Eventually(session).Should(gexec.Exit(types.GINKGO_FOCUS_EXIT_CODE))
+		session := startGinkgo(fm.PathTo("flags"), "--no-color", "--randomize-all", "--seed=120")
+		Eventually(session).Should(gexec.Exit(1))
 		output := string(session.Out.Contents())
 
 		orders := getRandomOrders(output)
@@ -70,14 +68,14 @@ var _ = Describe("Flags Specs", func() {
 
 	It("should pass additional arguments in", func() {
 		session := startGinkgo(fm.PathTo("flags"), "--", "--customFlag=madagascar")
-		Eventually(session).Should(gexec.Exit(types.GINKGO_FOCUS_EXIT_CODE))
+		Eventually(session).Should(gexec.Exit(1))
 		output := string(session.Out.Contents())
 
 		Ω(output).Should(ContainSubstring("CUSTOM_FLAG: madagascar"))
 	})
 
 	It("should print out full stack traces for failures when told to", func() {
-		session := startGinkgo(fm.PathTo("flags"), "--focus=a failing test", "--trace")
+		session := startGinkgo(fm.PathTo("flags"), "--trace")
 		Eventually(session).Should(gexec.Exit(1))
 		output := string(session.Out.Contents())
 
@@ -148,7 +146,7 @@ var _ = Describe("Flags Specs", func() {
 
 	It("should emit node start/end events when running with --show-node-events", func() {
 		session := startGinkgo(fm.PathTo("flags"), "--no-color", "-v", "--show-node-events")
-		Eventually(session).Should(gexec.Exit(types.GINKGO_FOCUS_EXIT_CODE))
+		Eventually(session).Should(gexec.Exit(1))
 		output := string(session.Out.Contents())
 
 		Eventually(output).Should(ContainSubstring("> Enter [It] should honor -cover"))

internal/focus.go

@@ -8,22 +8,22 @@ import (
 )
 
 /*
-	If a container marked as focus has a descendant that is also marked as focus, Ginkgo's policy is to
-	unmark the container's focus.  This gives developers a more intuitive experience when debugging specs.
-	It is common to focus a container to just run a subset of specs, then identify the specific specs within the container to focus -
-	this policy allows the developer to simply focus those specific specs and not need to go back and turn the focus off of the container:
-
-	As a common example, consider:
-
-		FDescribe("something to debug", function() {
-			It("works", function() {...})
-			It("works", function() {...})
-			FIt("doesn't work", function() {...})
-			It("works", function() {...})
-		})
-
-	here the developer's intent is to focus in on the `"doesn't work"` spec and not to run the adjacent specs in the focused `"something to debug"` container.
-	The nested policy applied by this function enables this behavior.
+If a container marked as focus has a descendant that is also marked as focus, Ginkgo's policy is to
+unmark the container's focus.  This gives developers a more intuitive experience when debugging specs.
+It is common to focus a container to just run a subset of specs, then identify the specific specs within the container to focus -
+this policy allows the developer to simply focus those specific specs and not need to go back and turn the focus off of the container:
+
+As a common example, consider:
+
+	FDescribe("something to debug", function() {
+		It("works", function() {...})
+		It("works", function() {...})
+		FIt("doesn't work", function() {...})
+		It("works", function() {...})
+	})
+
+here the developer's intent is to focus in on the `"doesn't work"` spec and not to run the adjacent specs in the focused `"something to debug"` container.
+The nested policy applied by this function enables this behavior.
 */
 func ApplyNestedFocusPolicyToTree(tree *TreeNode) {
 	var walkTree func(tree *TreeNode) bool
@@ -44,46 +44,43 @@ func ApplyNestedFocusPolicyToTree(tree *TreeNode) {
 }
 
 /*
-	Ginkgo supports focussing specs using `FIt`, `FDescribe`, etc. - this is called "programmatic focus"
-	It also supports focussing specs using regular expressions on the command line (`-focus=`, `-skip=`) that match against spec text
-	and file filters (`-focus-files=`, `-skip-files=`) that match against code locations for nodes in specs.
+Ginkgo supports focussing specs using `FIt`, `FDescribe`, etc. - this is called "programmatic focus"
+It also supports focussing specs using regular expressions on the command line (`-focus=`, `-skip=`) that match against spec text and file filters (`-focus-files=`, `-skip-files=`) that match against code locations for nodes in specs.
 
-	If any of the CLI flags are provided they take precedence.  The file filters run first followed by the regex filters.
+When both programmatic and file filters are provided their results are ANDed together.  If multiple kinds of filters are provided, the file filters run first followed by the regex filters.
 
-	This function sets the `Skip` property on specs by applying Ginkgo's focus policy:
-	- If there are no CLI arguments and no programmatic focus, do nothing.
-	- If there are no CLI arguments but a spec somewhere has programmatic focus, skip any specs that have no programmatic focus.
-	- If there are CLI arguments parse them and skip any specs that either don't match the focus filters or do match the skip filters.
+This function sets the `Skip` property on specs by applying Ginkgo's focus policy:
+- If there are no CLI arguments and no programmatic focus, do nothing.
+- If a spec somewhere has programmatic focus skip any specs that have no programmatic focus.
+- If there are CLI arguments parse them and skip any specs that either don't match the focus filters or do match the skip filters.
 
-	*Note:* specs with pending nodes are Skipped when created by NewSpec.
+*Note:* specs with pending nodes are Skipped when created by NewSpec.
 */
 func ApplyFocusToSpecs(specs Specs, description string, suiteLabels Labels, suiteConfig types.SuiteConfig) (Specs, bool) {
 	focusString := strings.Join(suiteConfig.FocusStrings, "|")
 	skipString := strings.Join(suiteConfig.SkipStrings, "|")
 
-	hasFocusCLIFlags := focusString != "" || skipString != "" || len(suiteConfig.SkipFiles) > 0 || len(suiteConfig.FocusFiles) > 0 || suiteConfig.LabelFilter != ""
-
 	type SkipCheck func(spec Spec) bool
 
 	// by default, skip any specs marked pending
 	skipChecks := []SkipCheck{func(spec Spec) bool { return spec.Nodes.HasNodeMarkedPending() }}
 	hasProgrammaticFocus := false
 
-	if !hasFocusCLIFlags {
-		// check for programmatic focus
-		for _, spec := range specs {
-			if spec.Nodes.HasNodeMarkedFocus() && !spec.Nodes.HasNodeMarkedPending() {
-				skipChecks = append(skipChecks, func(spec Spec) bool { return !spec.Nodes.HasNodeMarkedFocus() })
-				hasProgrammaticFocus = true
-				break
-			}
+	for _, spec := range specs {
+		if spec.Nodes.HasNodeMarkedFocus() && !spec.Nodes.HasNodeMarkedPending() {
+			hasProgrammaticFocus = true
+			break
 		}
 	}
 
+	if hasProgrammaticFocus {
+		skipChecks = append(skipChecks, func(spec Spec) bool { return !spec.Nodes.HasNodeMarkedFocus() })
+	}
+
 	if suiteConfig.LabelFilter != "" {
 		labelFilter, _ := types.ParseLabelFilter(suiteConfig.LabelFilter)
-		skipChecks = append(skipChecks, func(spec Spec) bool { 
-			return !labelFilter(UnionOfLabels(suiteLabels, spec.Nodes.UnionOfLabels())) 
+		skipChecks = append(skipChecks, func(spec Spec) bool {
+			return !labelFilter(UnionOfLabels(suiteLabels, spec.Nodes.UnionOfLabels()))
 		})
 	}
 

internal/focus_test.go

@@ -155,7 +155,7 @@ var _ = Describe("Focus", func() {
 					S(N("green dragon"), N()),
 					S(N(Pending), N("blue Dragon")),
 					S(N("yellow dragon")),
-					S(N(Focus, "yellow dragon")),
+					S(N("yellow dragon")),
 				}
 			})
 
@@ -218,7 +218,7 @@ var _ = Describe("Focus", func() {
 					S(N(CL("file_b", 3, "file_b", 15))), //include because "file_:15-21" is in FocusFiles
 					S(N(CL("file_b", 17))),              //skip because "_b:17" is in SkipFiles
 					S(N(CL("file_b", 20), Pending)),     //skip because spec is flagged pending
-					S(N(CL("c", 3), Focus)),             //skip because "c" is not in FocusFiles - override programmatic focus
+					S(N(CL("c", 3))),                    //skip because "c" is not in FocusFiles
 					S(N(CL("d", 17))),                   //include because "d " is in FocusFiles
 				}
 
@@ -240,7 +240,7 @@ var _ = Describe("Focus", func() {
 					S(N(ntCon, Label("cat", "dog")), N(ntIt, "A", Label("fish"))),  //skip because fish
 					S(N(ntCon, Label("cat", "dog")), N(ntIt, "B", Label("apple"))), //include because has cat and not fish
 					S(N(ntCon, Label("dog")), N(ntIt, "C", Label("apple"))),        //skip because no cat or cow
-					S(N(ntCon, Label("cow")), N(ntIt, "D", Label("fish"), Focus)),  //skip because fish, override focus
+					S(N(ntCon, Label("cow")), N(ntIt, "D", Label("fish"))),         //skip because fish
 					S(N(ntCon, Label("cow")), N(ntIt, "E")),                        //include because cow and no fish
 					S(N(ntCon, Label("cow")), N(ntIt, "F", Pending)),               //skip because pending
 				}
@@ -277,7 +277,7 @@ var _ = Describe("Focus", func() {
 					S(N("dog cat", CL("file_b", 3, "file_b", 15), Label("brown"))), //skip because "file_:15-21" is in FocusFiles but "cat" is in SkipStirngs
 					S(N("fish", CL("file_b", 17), Label("brown"))),                 //skip because "_b:17" is in SkipFiles, even though "fish" is in FocusStrings
 					S(N("biscuit", CL("file_b", 20), Pending, Label("brown"))),     //skip because spec is flagged pending
-					S(N("pony", CL("c", 3), Focus, Label("brown"))),                //skip because "c" is not in FocusFiles or FocusStrings - override programmatic focus
+					S(N("pony", CL("c", 3), Label("brown"))),                       //skip because "c" is not in FocusFiles or FocusStrings
 					S(N("goat", CL("d", 17), Label("brown"))),                      //skip because "goat" is in FocusStrings but "d" is not in FocusFiles
 				}
 
@@ -294,5 +294,32 @@ var _ = Describe("Focus", func() {
 				Ω(hasProgrammaticFocus).Should(BeFalse())
 			})
 		})
+
+		Context("when configured with focus/skip files, focus/skip strings, and label filters and there is a programmatic focus", func() {
+			BeforeEach(func() {
+				specs = Specs{
+					S(N("dog", CL("file_a", 1), Label("brown"))),                   //skip because "file_:1" is in FocusFiles and "dog" is in FocusStrings and has "brown" label but a different spec has a programmatic focus
+					S(N("dog", CL("file_a", 17), Label("brown"), Focus)),           //include because "file_:15-21" is in FocusFiles and "dog" is in FocusStrings and has "brown" label
+					S(N("dog", CL("file_a", 1), Label("white"), Focus)),            //skip because does not have "brown" label
+					S(N("dog cat", CL("file_b", 3, "file_b", 15), Label("brown"))), //skip because "file_:15-21" is in FocusFiles but "cat" is in SkipStirngs
+					S(N("fish", CL("file_b", 17), Label("brown"))),                 //skip because "_b:17" is in SkipFiles, even though "fish" is in FocusStrings
+					S(N("biscuit", CL("file_b", 20), Pending, Label("brown"))),     //skip because spec is flagged pending
+					S(N("pony", CL("c", 3), Label("brown"))),                       //skip because "c" is not in FocusFiles or FocusStrings
+					S(N("goat", CL("d", 17), Label("brown"))),                      //skip because "goat" is in FocusStrings but "d" is not in FocusFiles
+				}
+
+				conf.FocusFiles = []string{"file_:1,15-21"}
+				conf.SkipFiles = []string{"_b:17"}
+				conf.FocusStrings = []string{"goat", "dog", "fish", "biscuit"}
+				conf.SkipStrings = []string{"cat"}
+				conf.LabelFilter = "brown"
+			})
+
+			It("applies all filters", func() {
+				specs, hasProgrammaticFocus := internal.ApplyFocusToSpecs(specs, description, suiteLabels, conf)
+				Ω(harvestSkips(specs)).Should(Equal([]bool{true, false, true, true, true, true, true, true}))
+				Ω(hasProgrammaticFocus).Should(BeTrue())
+			})
+		})
 	})
 })

internal/internal_integration/focus_test.go

@@ -122,7 +122,7 @@ var _ = Describe("Focus", func() {
 		BeforeEach(func() {
 			conf.FocusStrings = []string{"blue", "green"}
 			conf.SkipStrings = []string{"red"}
-			success, _ := RunFixture("cli focus tests", func() {
+			success, hasProgrammaticFocus := RunFixture("cli focus tests", func() {
 				It("blue.1", rt.T("blue.1"))
 				It("blue.2", rt.T("blue.2"))
 				Describe("blue.container", func() {
@@ -137,9 +137,10 @@ var _ = Describe("Focus", func() {
 				Describe("red.2", func() {
 					It("green.2", rt.T("green.2"))
 				})
-				FIt("red.3", rt.T("red.3"))
+				It("red.3", rt.T("red.3"))
 			})
 			Ω(success).Should(BeTrue())
+			Ω(hasProgrammaticFocus).Should(BeFalse())
 		})
 
 		It("should run tests that match", func() {
@@ -157,6 +158,46 @@ var _ = Describe("Focus", func() {
 		})
 	})
 
+	Describe("with a combination of programmatic focus and config.FocusStrings and config.SkipStrings", func() {
+		BeforeEach(func() {
+			conf.FocusStrings = []string{"blue", "green"}
+			conf.SkipStrings = []string{"red"}
+			success, hasProgrammaticFocus := RunFixture("cli focus tests", func() {
+				It("blue.1", rt.T("blue.1"))
+				FIt("blue.2", rt.T("blue.2"))
+				FDescribe("blue.container", func() {
+					It("yellow.1", rt.T("yellow.1"))
+					It("red.1", rt.T("red.1"))
+					PIt("blue.3", rt.T("blue.3"))
+				})
+				Describe("green.container", func() {
+					It("yellow.2", rt.T("yellow.2"))
+					It("green.1", rt.T("green.1"))
+				})
+				FDescribe("red.2", func() {
+					It("green.2", rt.T("green.2"))
+				})
+				FIt("red.3", rt.T("red.3"))
+			})
+			Ω(success).Should(BeTrue())
+			Ω(hasProgrammaticFocus).Should(BeTrue())
+		})
+
+		It("should run tests that match all the filters", func() {
+			Ω(rt.TrackedRuns()).Should(ConsistOf("blue.2", "yellow.1"))
+		})
+
+		It("should report on the tests correctly", func() {
+			Ω(reporter.Did.WithState(types.SpecStateSkipped).Names()).Should(ConsistOf("blue.1", "red.1", "yellow.2", "green.1", "green.2", "red.3"))
+			Ω(reporter.Did.WithState(types.SpecStatePending).Names()).Should(ConsistOf("blue.3"))
+			Ω(reporter.Did.WithState(types.SpecStatePassed).Names()).Should(ConsistOf("blue.2", "yellow.1"))
+		})
+
+		It("report on the suite with accurate numbers", func() {
+			Ω(reporter.End).Should(BeASuiteSummary(true, NPassed(2), NSkipped(6), NPending(1), NSpecs(9), NWillRun(2)))
+		})
+	})
+
 	Describe("when no tests will end up running", func() {
 		BeforeEach(func() {
 			conf.FocusStrings = []string{"red"}

internal/internal_integration/labels_test.go

@@ -12,7 +12,7 @@ var _ = Describe("Labels", func() {
 	Describe("when a suite has labelled tests", func() {
 		fixture := func() {
 			Describe("outer container", func() {
-				It("A", rt.T("A"), Focus, Label("cat"))
+				It("A", rt.T("A"), Label("cat"))
 				It("B", rt.T("B"), Label("dog"))
 				Describe("container", Label("cow", "cat"), func() {
 					It("C", rt.T("C"))