Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: allow multiple -focus and -skip flags #736

Merged
merged 1 commit into from Jan 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 23 additions & 8 deletions config/config.go
Expand Up @@ -26,8 +26,8 @@ type GinkgoConfigType struct {
RandomSeed int64
RandomizeAllSpecs bool
RegexScansFilePath bool
FocusString string
SkipString string
FocusStrings []string
SkipStrings []string
SkipMeasurements bool
FailOnPending bool
FailFast bool
Expand Down Expand Up @@ -65,6 +65,11 @@ func processPrefix(prefix string) string {
return prefix
}

type flagFunc func(string)

func (f flagFunc) String() string { return "" }
func (f flagFunc) Set(s string) error { f(s); return nil }

func Flags(flagSet *flag.FlagSet, prefix string, includeParallelFlags bool) {
prefix = processPrefix(prefix)
flagSet.Int64Var(&(GinkgoConfig.RandomSeed), prefix+"seed", time.Now().Unix(), "The seed used to randomize the spec suite.")
Expand All @@ -75,8 +80,8 @@ func Flags(flagSet *flag.FlagSet, prefix string, includeParallelFlags bool) {

flagSet.BoolVar(&(GinkgoConfig.DryRun), prefix+"dryRun", false, "If set, ginkgo will walk the test hierarchy without actually running anything. Best paired with -v.")

flagSet.StringVar(&(GinkgoConfig.FocusString), prefix+"focus", "", "If set, ginkgo will only run specs that match this regular expression.")
flagSet.StringVar(&(GinkgoConfig.SkipString), prefix+"skip", "", "If set, ginkgo will only run specs that do not match this regular expression.")
flagSet.Var(flagFunc(flagFocus), prefix+"focus", "If set, ginkgo will only run specs that match this regular expression. Can be specified multiple times, values are ORed.")
flagSet.Var(flagFunc(flagSkip), prefix+"skip", "If set, ginkgo will only run specs that do not match this regular expression. Can be specified multiple times, values are ORed.")

flagSet.BoolVar(&(GinkgoConfig.RegexScansFilePath), prefix+"regexScansFilePath", false, "If set, ginkgo regex matching also will look at the file path (code location).")

Expand Down Expand Up @@ -133,12 +138,12 @@ func BuildFlagArgs(prefix string, ginkgo GinkgoConfigType, reporter DefaultRepor
result = append(result, fmt.Sprintf("--%sdryRun", prefix))
}

if ginkgo.FocusString != "" {
result = append(result, fmt.Sprintf("--%sfocus=%s", prefix, ginkgo.FocusString))
for _, s := range ginkgo.FocusStrings {
result = append(result, fmt.Sprintf("--%sfocus=%s", prefix, s))
}

if ginkgo.SkipString != "" {
result = append(result, fmt.Sprintf("--%sskip=%s", prefix, ginkgo.SkipString))
for _, s := range ginkgo.SkipStrings {
result = append(result, fmt.Sprintf("--%sskip=%s", prefix, s))
}

if ginkgo.FlakeAttempts > 1 {
Expand Down Expand Up @@ -211,3 +216,13 @@ func BuildFlagArgs(prefix string, ginkgo GinkgoConfigType, reporter DefaultRepor

return result
}

// flagFocus implements the -focus flag.
func flagFocus(arg string) {
GinkgoConfig.FocusStrings = append(GinkgoConfig.FocusStrings, arg)
}

// flagSkip implements the -skip flag.
func flagSkip(arg string) {
GinkgoConfig.SkipStrings = append(GinkgoConfig.SkipStrings, arg)
}
13 changes: 13 additions & 0 deletions integration/flags_test.go
Expand Up @@ -105,6 +105,19 @@ var _ = Describe("Flags Specs", func() {
Ω(output).Should(ContainSubstring("3 Skipped"))
})

It("should override the programmatic focus when told to skip (multiple options)", func() {
session := startGinkgo(pathToTest, "--noColor", "--skip=marshmallow", "--skip=failing", "--skip=flaky")
Eventually(session).Should(gexec.Exit(0))
output := string(session.Out.Contents())

Ω(output).ShouldNot(ContainSubstring("marshmallow"))
Ω(output).Should(ContainSubstring("chocolate"))
Ω(output).Should(ContainSubstring("smores"))
Ω(output).Should(ContainSubstring("11 Passed"))
Ω(output).Should(ContainSubstring("0 Failed"))
Ω(output).Should(ContainSubstring("1 Pending"))
Ω(output).Should(ContainSubstring("3 Skipped"))
})
It("should run the race detector when told to", func() {
if !raceDetectorSupported() {
Skip("race detection is not supported")
Expand Down
20 changes: 10 additions & 10 deletions internal/spec/specs.go
Expand Up @@ -4,6 +4,7 @@ import (
"math/rand"
"regexp"
"sort"
"strings"
)

type Specs struct {
Expand Down Expand Up @@ -46,11 +47,11 @@ func (e *Specs) Shuffle(r *rand.Rand) {
e.names = names
}

func (e *Specs) ApplyFocus(description string, focusString string, skipString string) {
if focusString == "" && skipString == "" {
func (e *Specs) ApplyFocus(description string, focus, skip []string) {
if len(focus)+len(skip) == 0 {
e.applyProgrammaticFocus()
} else {
e.applyRegExpFocusAndSkip(description, focusString, skipString)
e.applyRegExpFocusAndSkip(description, focus, skip)
}
}

Expand Down Expand Up @@ -90,14 +91,13 @@ func (e *Specs) toMatch(description string, i int) []byte {
}
}

func (e *Specs) applyRegExpFocusAndSkip(description string, focusString string, skipString string) {
var focusFilter *regexp.Regexp
if focusString != "" {
focusFilter = regexp.MustCompile(focusString)
func (e *Specs) applyRegExpFocusAndSkip(description string, focus, skip []string) {
var focusFilter, skipFilter *regexp.Regexp
if len(focus) > 0 {
focusFilter = regexp.MustCompile(strings.Join(focus, "|"))
}
var skipFilter *regexp.Regexp
if skipString != "" {
skipFilter = regexp.MustCompile(skipString)
if len(skip) > 0 {
skipFilter = regexp.MustCompile(strings.Join(skip, "|"))
}

for i, spec := range e.specs {
Expand Down
29 changes: 17 additions & 12 deletions internal/spec/specs_test.go
Expand Up @@ -111,7 +111,7 @@ var _ = Describe("Specs", func() {
Describe("with no programmatic focus", func() {
BeforeEach(func() {
specs = newSpecs("A1", noneFlag, "A2", noneFlag, "B1", noneFlag, "B2", pendingFlag)
specs.ApplyFocus("", "", "")
specs.ApplyFocus("", []string{}, []string{})
})

It("should not report as having programmatic specs", func() {
Expand All @@ -120,15 +120,20 @@ var _ = Describe("Specs", func() {
})

Describe("Applying focus/skip", func() {
var description, focusString, skipString string
var (
description string
focus, skip []string
)

BeforeEach(func() {
description, focusString, skipString = "", "", ""
description = ""
focus = []string{}
skip = []string{}
})

JustBeforeEach(func() {
specs = newSpecs("A1", focusedFlag, "A2", noneFlag, "B1", focusedFlag, "B2", pendingFlag)
specs.ApplyFocus(description, focusString, skipString)
specs.ApplyFocus(description, focus, skip)
})

Context("with neither a focus string nor a skip string", func() {
Expand All @@ -145,7 +150,7 @@ var _ = Describe("Specs", func() {

Context("with a focus regexp", func() {
BeforeEach(func() {
focusString = "A"
focus = []string{"A"}
})

It("should override the programmatic focus", func() {
Expand All @@ -161,7 +166,7 @@ var _ = Describe("Specs", func() {

Context("with a focus regexp", func() {
BeforeEach(func() {
focusString = "B"
focus = []string{"B"}
})

It("should not override any pendings", func() {
Expand All @@ -174,7 +179,7 @@ var _ = Describe("Specs", func() {
Context("with a description", func() {
BeforeEach(func() {
description = "C"
focusString = "C"
focus = []string{"C"}
})

It("should include the description in the focus determination", func() {
Expand All @@ -187,7 +192,7 @@ var _ = Describe("Specs", func() {
Context("with a description", func() {
BeforeEach(func() {
description = "C"
skipString = "C"
skip = []string{"C"}
})

It("should include the description in the focus determination", func() {
Expand All @@ -199,7 +204,7 @@ var _ = Describe("Specs", func() {

Context("with a skip regexp", func() {
BeforeEach(func() {
skipString = "A"
skip = []string{"A"}
})

It("should override the programmatic focus", func() {
Expand All @@ -215,8 +220,8 @@ var _ = Describe("Specs", func() {

Context("with both a focus and a skip regexp", func() {
BeforeEach(func() {
focusString = "1"
skipString = "B"
focus = []string{"1"}
skip = []string{"B"}
})

It("should AND the two", func() {
Expand Down Expand Up @@ -251,7 +256,7 @@ var _ = Describe("Specs", func() {
pendingInFocused,
focusedInPending,
})
specs.ApplyFocus("", "", "")
specs.ApplyFocus("", []string{}, []string{})
})

It("should not have a programmatic focus and should run all tests", func() {
Expand Down
2 changes: 1 addition & 1 deletion internal/suite/suite.go
Expand Up @@ -97,7 +97,7 @@ func (suite *Suite) generateSpecsIterator(description string, config config.Gink
specs.Shuffle(rand.New(rand.NewSource(config.RandomSeed)))
}

specs.ApplyFocus(description, config.FocusString, config.SkipString)
specs.ApplyFocus(description, config.FocusStrings, config.SkipStrings)

if config.SkipMeasurements {
specs.SkipMeasurements()
Expand Down
8 changes: 4 additions & 4 deletions internal/suite/suite_test.go
Expand Up @@ -40,7 +40,7 @@ var _ = Describe("Suite", func() {
runOrder []string
randomizeAllSpecs bool
randomSeed int64
focusString string
focusStrings []string
parallelNode int
parallelTotal int
runResult bool
Expand All @@ -58,7 +58,7 @@ var _ = Describe("Suite", func() {
randomSeed = 11
parallelNode = 1
parallelTotal = 1
focusString = ""
focusStrings = []string{}

runOrder = make([]string, 0)
specSuite.SetBeforeSuiteNode(f("BeforeSuite"), codelocation.New(0), 0)
Expand Down Expand Up @@ -91,7 +91,7 @@ var _ = Describe("Suite", func() {
runResult, hasProgrammaticFocus = specSuite.Run(fakeT, "suite description", []reporters.Reporter{fakeR}, writer, config.GinkgoConfigType{
RandomSeed: randomSeed,
RandomizeAllSpecs: randomizeAllSpecs,
FocusString: focusString,
FocusStrings: focusStrings,
ParallelNode: parallelNode,
ParallelTotal: parallelTotal,
})
Expand Down Expand Up @@ -186,7 +186,7 @@ var _ = Describe("Suite", func() {

Context("when provided with a filter", func() {
BeforeEach(func() {
focusString = `inner|\d`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change. Users who relied on -ginkgo.skip overwriting the old value will be surprised... I certainly was and it took me a while to figure out why some tests no longer ran.

The reason was this default:

	// Skip slow or distruptive tests by default.
	flag.Set("ginkgo.skip", `\[Slow|Disruptive\]`)
	flag.Parse()

We overwrite that default in our Makefile, but because of this change, the skip string above remained in effect.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1
maybe need keep this test, keep no breaking change.

focusStrings = []string{`inner`, `\d`}
})

It("converts the filter to a regular expression and uses it to filter the running specs", func() {
Expand Down