Skip to content

Commit

Permalink
Programmatic focus is no longer overwrriten by CLI filters
Browse files Browse the repository at this point in the history
  • Loading branch information
onsi committed Jun 16, 2023
1 parent 4a70a38 commit d6bba86
Show file tree
Hide file tree
Showing 9 changed files with 160 additions and 114 deletions.
4 changes: 2 additions & 2 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion integration/_fixtures/filter_fixture/sprocket_c_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
. "github.com/onsi/ginkgo/v2"
)

var _ = FDescribe("SprocketC", func() {
var _ = Describe("SprocketC", func() {
It("cat", func() {

})
Expand Down
82 changes: 36 additions & 46 deletions integration/_fixtures/flags_fixture/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
11 changes: 2 additions & 9 deletions integration/filter_test.go
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down Expand Up @@ -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()))
}
})

Expand Down
22 changes: 10 additions & 12 deletions integration/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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]))
Expand All @@ -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)
Expand All @@ -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())

Expand Down Expand Up @@ -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"))
Expand Down
71 changes: 34 additions & 37 deletions internal/focus.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()))
})
}

Expand Down
Loading

0 comments on commit d6bba86

Please sign in to comment.