Skip to content

Commit

Permalink
cmd/critest: fix empty ginkgo flag's value issue
Browse files Browse the repository at this point in the history
In commit 53ad8bb[1], the ginkgo uses
flags.StringVar for some flags, for instance, the --skip or --focus.

But in the following release of ginkgo(>=v1.15.0[2]), the ginkgo starts to
use string slice instead of the StringVar, which means we can't get the
user's input by the flags.Value.String()[3]. The String() always returns
empty value.

Based on that, we should use ginkgo API to generate flags instead of
manually handler in cri-test.

Reference:

[1]: https://github.com/kubernetes-sigs/cri-tools/blob/53ad8bb7f97e1b1d1c0c0634e43a3c2b8b07b718/vendor/github.com/onsi/ginkgo/config/config.go L79
[2]: https://github.com/onsi/ginkgo/blob/v1.15.0/config/config.go L84
[3]: https://github.com/onsi/ginkgo/blob/v1.15.0/config/config.go L68-L71

Signed-off-by: Wei Fu <fuweid89@gmail.com>
  • Loading branch information
fuweid committed May 11, 2022
1 parent 1a0b272 commit d4fe3e7
Showing 1 changed file with 38 additions and 3 deletions.
41 changes: 38 additions & 3 deletions cmd/critest/cri_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (

"github.com/onsi/ginkgo/v2"
"github.com/onsi/ginkgo/v2/reporters"
ginkgotypes "github.com/onsi/ginkgo/v2/types"
"github.com/onsi/gomega"

_ "github.com/kubernetes-sigs/cri-tools/pkg/benchmark"
Expand Down Expand Up @@ -135,14 +136,30 @@ func runParallelTestSuite(t *testing.T) {
}
defer os.Remove(tempFileName)

ginkgoArgs := []string{fmt.Sprintf("-nodes=%d", *parallel)}
ginkgoArgs, err := generateGinkgoRunFlags()
if err != nil {
t.Fatalf("Failed to generate ginkgo args: %v", err)
}
ginkgoArgs = append(ginkgoArgs, fmt.Sprintf("--nodes=%d", *parallel))

var testArgs []string
flag.Visit(func(f *flag.Flag) {
// NOTE(fuweid):
//
// The ginkgo has changed the flag var from string to string slice
// for some, like ginkgo.Skip/Focus.
//
// The --skip flag's config is https://github.com/onsi/ginkgo/blob/v2.0.0/types/config.go#L284.
// And the value will be appended to https://github.com/onsi/ginkgo/blob/v2.0.0/types/config.go#L22.
// The flag var is https://github.com/onsi/ginkgo/blob/v2.0.0/types/flags.go#L428,
// which means that we can't get value by interface String().
//
// So we need to skip the "ginkgo.*" flags and use ginkgo API
// to generate the flags.
if strings.HasPrefix(f.Name, "ginkgo.") {
flagName := strings.TrimPrefix(f.Name, "ginkgo.")
ginkgoArgs = append(ginkgoArgs, fmt.Sprintf("-%s=%s", flagName, f.Value.String()))
return
}

if f.Name == parallelFlag || f.Name == benchmarkFlag {
return
}
Expand Down Expand Up @@ -183,3 +200,21 @@ func TestCRISuite(t *testing.T) {
runTestSuite(t)
}
}

// generateGinkgoRunFlags is based on ginkgotypes.GenerateGinkgoTestRunArgs.
//
// Since the GenerateGinkgoTestRunArgs adds "ginkgo." as prefix for each
// flags and we use --nodes instead of ParallelConfigFlags, we need to call
// GenerateFlagArgs to get what we want.
func generateGinkgoRunFlags() ([]string, error) {
suiteConfig, reporterConfig := ginkgo.GinkgoConfiguration()

flags := ginkgotypes.SuiteConfigFlags
flags = flags.CopyAppend(ginkgotypes.ReporterConfigFlags...)

bindings := map[string]interface{}{
"S": &suiteConfig,
"R": &reporterConfig,
}
return ginkgotypes.GenerateFlagArgs(flags, bindings)
}

0 comments on commit d4fe3e7

Please sign in to comment.