From 8ec2ca0e4c3653e13f0c29f18279617028e3017a Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Fri, 10 Dec 2021 15:38:45 -0800 Subject: [PATCH 1/3] chore(e2e): naively parallelize CI jobs by chunking alphabetically Signed-off-by: Eric Stroczynski --- .github/workflows/e2e-tests.yml | 7 ++- Makefile | 12 +++- test/e2e/split/main.go | 100 ++++++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 3 deletions(-) create mode 100644 test/e2e/split/main.go diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index fbbde9c463..8422c50f14 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -9,16 +9,19 @@ on: workflow_dispatch: jobs: e2e-tests: + strategy: + matrix: + parallel-id: [0, 1, 2, 3] runs-on: ubuntu-latest steps: - uses: actions/checkout@v1 - uses: actions/setup-go@v2 with: go-version: '~1.17' - - run: make e2e-local E2E_NODES=2 ARTIFACTS_DIR=./artifacts/ + - run: make e2e-local E2E_TEST_CHUNK=${{ matrix.parallel-id }} E2E_TEST_NUM_CHUNKS=${{ strategy.job-total }} E2E_NODES=2 ARTIFACTS_DIR=./artifacts/ - name: Archive Test Artifacts # test results, failed or not, are always uploaded. if: ${{ always() }} uses: actions/upload-artifact@v2 with: - name: e2e-test-output-${{(github.event.pull_request.head.sha||github.sha)}}-${{ github.run_id }} + name: e2e-test-output-${{ (github.event.pull_request.head.sha || github.sha) }}-${{ github.run_id }}-${{ matrix.parallel-id }} path: ${{ github.workspace }}/bin/artifacts/* diff --git a/Makefile b/Makefile index eb768d6f0d..a23c984b30 100644 --- a/Makefile +++ b/Makefile @@ -126,7 +126,17 @@ setup-bare: clean e2e.namespace E2E_NODES ?= 1 E2E_FLAKE_ATTEMPTS ?= 1 E2E_TIMEOUT ?= 90m -E2E_OPTS ?= $(if $(E2E_SEED),-seed '$(E2E_SEED)') $(if $(TEST),-focus '$(TEST)') -flakeAttempts $(E2E_FLAKE_ATTEMPTS) -nodes $(E2E_NODES) -timeout $(E2E_TIMEOUT) -v -randomizeSuites -race -trace -progress +E2E_COND_OPTS := $(if $(E2E_SEED),-seed '$(E2E_SEED)') +# Optionally run an individual chunk of e2e test specs. +# Do not use this from the CLI; this is intended to be used by CI only. +E2E_TEST_CHUNK ?= all +E2E_TEST_NUM_CHUNKS ?= 4 +ifeq (all,$(E2E_TEST_CHUNK)) +E2E_COND_OPTS := $(E2E_COND_OPTS) $(if $(TEST),-focus '$(TEST)') +else +E2E_COND_OPTS := $(E2E_COND_OPTS) -focus "$(shell go run ./test/e2e/split/... -chunks $(E2E_TEST_NUM_CHUNKS) -print-chunk $(E2E_TEST_CHUNK) ./test/e2e)" +endif +E2E_OPTS ?= $(E2E_COND_OPTS) -flakeAttempts $(E2E_FLAKE_ATTEMPTS) -nodes $(E2E_NODES) -timeout $(E2E_TIMEOUT) -v -randomizeSuites -race -trace -progress E2E_INSTALL_NS ?= operator-lifecycle-manager E2E_TEST_NS ?= operators diff --git a/test/e2e/split/main.go b/test/e2e/split/main.go new file mode 100644 index 0000000000..6c29a8231b --- /dev/null +++ b/test/e2e/split/main.go @@ -0,0 +1,100 @@ +package main + +import ( + "flag" + "fmt" + "io/ioutil" + "log" + "math" + "os" + "path/filepath" + "regexp" + "sort" + "strings" +) + +var topDescribeRE = regexp.MustCompile(`var _ = Describe\("(.+)", func\(.*`) + +func main() { + var numChunks, printChunk int + flag.IntVar(&numChunks, "chunks", 1, "Number of chunks to create focus regexps for") + flag.IntVar(&printChunk, "print-chunk", 0, "Chunk to print a regexp for") + flag.Parse() + + if printChunk >= numChunks { + log.Fatalf("the chunk to print (%d) must be a smaller number than the number of chunks (%d)", printChunk, numChunks) + } + + dir := flag.Arg(0) + + // Clean dir. + var err error + if dir, err = filepath.Abs(dir); err != nil { + log.Fatal(err) + } + wd, err := os.Getwd() + if err != nil { + log.Fatal(err) + } + if dir, err = filepath.Rel(wd, dir); err != nil { + log.Fatal(err) + } + + // Find all Ginkgo specs in dir's test files. + // These can be grouped independently. + describeTable := make(map[string]struct{}) + matches, err := filepath.Glob(filepath.Join(dir, "*_test.go")) + if err != nil { + log.Fatal(err) + } + for _, match := range matches { + b, err := ioutil.ReadFile(match) + if err != nil { + log.Fatal(err) + } + specNames := topDescribeRE.FindAllSubmatch(b, -1) + if len(specNames) == 0 { + log.Printf("%s: found no top level describes, skipping", match) + continue + } + for _, possibleNames := range specNames { + if len(possibleNames) != 2 { + log.Printf("%s: expected to find 2 submatch, found %d:", match, len(possibleNames)) + for _, name := range possibleNames { + log.Printf("\t%s\n", string(name)) + } + continue + } + describe := strings.TrimSpace(string(possibleNames[1])) + describeTable[describe] = struct{}{} + } + } + + describes := make([]string, len(describeTable)) + i := 0 + for describeKey := range describeTable { + describes[i] = describeKey + i++ + } + sort.Strings(describes) + + chunks := make([][]string, numChunks) + interval := int(math.Ceil(float64(len(describes)) / float64(numChunks))) + currIdx := 0 + for chunkIdx := 0; chunkIdx < numChunks; chunkIdx++ { + nextIdx := int(math.Min(float64(currIdx+interval), float64(len(describes)))) + chunks[chunkIdx] = describes[currIdx:nextIdx] + currIdx = nextIdx + } + + sb := strings.Builder{} + sb.WriteString("(") + sb.WriteString(chunks[printChunk][0]) + for _, test := range chunks[printChunk][1:] { + sb.WriteString("|") + sb.WriteString(test) + } + sb.WriteString(").*") + + fmt.Println(sb.String()) +} From 139ecd736b430c691d1172a1015e490dac7453b3 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Tue, 14 Dec 2021 11:42:49 -0800 Subject: [PATCH 2/3] build word trie from specs, add unit tests Signed-off-by: Eric Stroczynski --- .github/workflows/e2e-tests.yml | 2 + Makefile | 11 +- test/e2e/split/integration_test.sh | 14 +++ test/e2e/split/main.go | 177 +++++++++++++++++++++++++---- test/e2e/split/main_test.go | 141 +++++++++++++++++++++++ 5 files changed, 313 insertions(+), 32 deletions(-) create mode 100755 test/e2e/split/integration_test.sh create mode 100644 test/e2e/split/main_test.go diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 8422c50f14..cf9b1a52ef 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -10,6 +10,7 @@ on: jobs: e2e-tests: strategy: + fail-fast: false matrix: parallel-id: [0, 1, 2, 3] runs-on: ubuntu-latest @@ -25,3 +26,4 @@ jobs: with: name: e2e-test-output-${{ (github.event.pull_request.head.sha || github.sha) }}-${{ github.run_id }}-${{ matrix.parallel-id }} path: ${{ github.workspace }}/bin/artifacts/* +# TODO: create job to combine test artifacts using code in https://github.com/operator-framework/operator-lifecycle-manager/pull/1476 diff --git a/Makefile b/Makefile index a23c984b30..7247096f2c 100644 --- a/Makefile +++ b/Makefile @@ -43,7 +43,7 @@ all: test build test: clean cover.out unit: kubebuilder - KUBEBUILDER_ASSETS=$(KUBEBUILDER_ASSETS) go test $(MOD_FLAGS) $(SPECIFIC_UNIT_TEST) -tags "json1" -race -count=1 ./pkg/... + KUBEBUILDER_ASSETS=$(KUBEBUILDER_ASSETS) go test $(MOD_FLAGS) $(SPECIFIC_UNIT_TEST) -tags "json1" -race -count=1 ./pkg/... ./test/e2e/split/... # Ensure kubebuilder is installed before continuing KUBEBUILDER_ASSETS_ERR := not detected in $(KUBEBUILDER_ASSETS), to override the assets path set the KUBEBUILDER_ASSETS environment variable, for install instructions see https://book.kubebuilder.io/quick-start.html @@ -126,17 +126,14 @@ setup-bare: clean e2e.namespace E2E_NODES ?= 1 E2E_FLAKE_ATTEMPTS ?= 1 E2E_TIMEOUT ?= 90m -E2E_COND_OPTS := $(if $(E2E_SEED),-seed '$(E2E_SEED)') # Optionally run an individual chunk of e2e test specs. # Do not use this from the CLI; this is intended to be used by CI only. E2E_TEST_CHUNK ?= all E2E_TEST_NUM_CHUNKS ?= 4 -ifeq (all,$(E2E_TEST_CHUNK)) -E2E_COND_OPTS := $(E2E_COND_OPTS) $(if $(TEST),-focus '$(TEST)') -else -E2E_COND_OPTS := $(E2E_COND_OPTS) -focus "$(shell go run ./test/e2e/split/... -chunks $(E2E_TEST_NUM_CHUNKS) -print-chunk $(E2E_TEST_CHUNK) ./test/e2e)" +ifneq (all,$(E2E_TEST_CHUNK)) +TEST := $(shell go run ./test/e2e/split/... -chunks $(E2E_TEST_NUM_CHUNKS) -print-chunk $(E2E_TEST_CHUNK) ./test/e2e) endif -E2E_OPTS ?= $(E2E_COND_OPTS) -flakeAttempts $(E2E_FLAKE_ATTEMPTS) -nodes $(E2E_NODES) -timeout $(E2E_TIMEOUT) -v -randomizeSuites -race -trace -progress +E2E_OPTS ?= $(if $(E2E_SEED),-seed '$(E2E_SEED)') $(if $(TEST),-focus '$(TEST)') -flakeAttempts $(E2E_FLAKE_ATTEMPTS) -nodes $(E2E_NODES) -timeout $(E2E_TIMEOUT) -v -randomizeSuites -race -trace -progress E2E_INSTALL_NS ?= operator-lifecycle-manager E2E_TEST_NS ?= operators diff --git a/test/e2e/split/integration_test.sh b/test/e2e/split/integration_test.sh new file mode 100755 index 0000000000..c3b7115539 --- /dev/null +++ b/test/e2e/split/integration_test.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash + +function get_total_specs() { + go run github.com/onsi/ginkgo/ginkgo -noColor -dryRun -v -seed 1 "$@" ./test/e2e | grep -Po "Ran \K([0-9]+)(?= of .+ Specs in .+ seconds)" +} + +unfocused_specs=$(get_total_specs) +regexp=$(go run ./test/e2e/split/... -chunks 1 -print-chunk 0 ./test/e2e) +focused_specs=$(get_total_specs -focus "$regexp") + +if ! [ $unfocused_specs -eq $focused_specs ]; then + echo "expected number of unfocused specs $unfocused_specs to equal focus specs $focused_specs" + exit 1 +fi diff --git a/test/e2e/split/main.go b/test/e2e/split/main.go index 6c29a8231b..d6ce2c957c 100644 --- a/test/e2e/split/main.go +++ b/test/e2e/split/main.go @@ -3,6 +3,7 @@ package main import ( "flag" "fmt" + "io" "io/ioutil" "log" "math" @@ -13,44 +14,92 @@ import ( "strings" ) -var topDescribeRE = regexp.MustCompile(`var _ = Describe\("(.+)", func\(.*`) +// TODO: configurable log verbosity. + +type options struct { + numChunks int + printChunk int + printDebug bool + writer io.Writer +} func main() { - var numChunks, printChunk int - flag.IntVar(&numChunks, "chunks", 1, "Number of chunks to create focus regexps for") - flag.IntVar(&printChunk, "print-chunk", 0, "Chunk to print a regexp for") + opts := options{ + writer: os.Stdout, + } + flag.IntVar(&opts.numChunks, "chunks", 1, "Number of chunks to create focus regexps for") + flag.IntVar(&opts.printChunk, "print-chunk", 0, "Chunk to print a regexp for") + flag.BoolVar(&opts.printDebug, "print-debug", false, "Print all spec prefixes in non-regexp format. Use for debugging") flag.Parse() - if printChunk >= numChunks { - log.Fatalf("the chunk to print (%d) must be a smaller number than the number of chunks (%d)", printChunk, numChunks) + if opts.printChunk >= opts.numChunks { + exitIfErr(fmt.Errorf("the chunk to print (%d) must be a smaller number than the number of chunks (%d)", opts.printChunk, opts.numChunks)) } dir := flag.Arg(0) + if dir == "" { + exitIfErr(fmt.Errorf("test directory required as the argument")) + } // Clean dir. var err error - if dir, err = filepath.Abs(dir); err != nil { - log.Fatal(err) - } + dir, err = filepath.Abs(dir) + exitIfErr(err) wd, err := os.Getwd() + exitIfErr(err) + dir, err = filepath.Rel(wd, dir) + exitIfErr(err) + + exitIfErr(opts.run(dir)) +} + +func exitIfErr(err error) { if err != nil { log.Fatal(err) } - if dir, err = filepath.Rel(wd, dir); err != nil { - log.Fatal(err) +} + +func (opts options) run(dir string) error { + describes, err := findDescribes(dir) + if err != nil { + return err } + // Find minimal prefixes for all spec strings so no spec runs are duplicated across chunks. + prefixes := findMinimalWordPrefixes(describes) + sort.Strings(prefixes) + + var out string + if opts.printDebug { + out = strings.Join(prefixes, "\n") + } else { + out, err = createChunkRegexp(opts.numChunks, opts.printChunk, prefixes) + if err != nil { + return err + } + } + + fmt.Fprint(opts.writer, out) + return nil +} + +// TODO: this is hacky because top-level tests may be defined elsewise. +// A better strategy would be to use the output of `ginkgo -noColor -dryRun` +// like https://github.com/operator-framework/operator-lifecycle-manager/pull/1476 does. +var topDescribeRE = regexp.MustCompile(`var _ = Describe\("(.+)", func\(.*`) + +func findDescribes(dir string) ([]string, error) { // Find all Ginkgo specs in dir's test files. // These can be grouped independently. describeTable := make(map[string]struct{}) matches, err := filepath.Glob(filepath.Join(dir, "*_test.go")) if err != nil { - log.Fatal(err) + return nil, err } for _, match := range matches { b, err := ioutil.ReadFile(match) if err != nil { - log.Fatal(err) + return nil, err } specNames := topDescribeRE.FindAllSubmatch(b, -1) if len(specNames) == 0 { @@ -76,25 +125,103 @@ func main() { describes[i] = describeKey i++ } - sort.Strings(describes) + return describes, nil +} + +func createChunkRegexp(numChunks, printChunk int, specs []string) (string, error) { + numSpecs := len(specs) + if numSpecs < numChunks { + return "", fmt.Errorf("have more desired chunks (%d) than specs (%d)", numChunks, numSpecs) + } + + // Create chunks of size ceil(number of specs/number of chunks) in alphanumeric order. + // This is deterministic on inputs. chunks := make([][]string, numChunks) - interval := int(math.Ceil(float64(len(describes)) / float64(numChunks))) + interval := int(math.Ceil(float64(numSpecs) / float64(numChunks))) currIdx := 0 for chunkIdx := 0; chunkIdx < numChunks; chunkIdx++ { - nextIdx := int(math.Min(float64(currIdx+interval), float64(len(describes)))) - chunks[chunkIdx] = describes[currIdx:nextIdx] + nextIdx := int(math.Min(float64(currIdx+interval), float64(numSpecs))) + chunks[chunkIdx] = specs[currIdx:nextIdx] currIdx = nextIdx } - sb := strings.Builder{} - sb.WriteString("(") - sb.WriteString(chunks[printChunk][0]) - for _, test := range chunks[printChunk][1:] { - sb.WriteString("|") - sb.WriteString(test) + chunk := chunks[printChunk] + if len(chunk) == 0 { + // This is a panic because the caller may skip this error, resulting in missed test specs. + panic(fmt.Sprintf("bug: chunk %d has no elements", printChunk)) } - sb.WriteString(").*") - fmt.Println(sb.String()) + // Write out the regexp to focus chunk specs via `ginkgo -focus `. + var reStr string + if len(chunk) == 1 { + reStr = fmt.Sprintf("%s .*", chunk[0]) + } else { + sb := strings.Builder{} + sb.WriteString(chunk[0]) + for _, test := range chunk[1:] { + sb.WriteString("|") + sb.WriteString(test) + } + reStr = fmt.Sprintf("(%s) .*", sb.String()) + } + + return reStr, nil +} + +func findMinimalWordPrefixes(specs []string) (prefixes []string) { + + // Create a word trie of all spec strings. + t := make(wordTrie) + for _, spec := range specs { + t.push(spec) + } + + // Now find the first branch point for each path in the trie by DFS. + for word, node := range t { + var prefixElements []string + next: + if word != "" { + prefixElements = append(prefixElements, word) + } + if len(node.children) == 1 { + for nextWord, nextNode := range node.children { + word, node = nextWord, nextNode + } + goto next + } + // TODO: this might need to be joined by "\s+" + // in case multiple spaces were used in the spec name. + prefixes = append(prefixes, strings.Join(prefixElements, " ")) + } + + return prefixes +} + +// wordTrie is a trie of word nodes, instead of individual characters. +type wordTrie map[string]*wordTrieNode + +type wordTrieNode struct { + word string + children map[string]*wordTrieNode +} + +// push creates s branch of the trie from each word in s. +func (t wordTrie) push(s string) { + split := strings.Split(s, " ") + + curr := &wordTrieNode{word: "", children: t} + for _, sp := range split { + if sp = strings.TrimSpace(sp); sp == "" { + continue + } + next, hasNext := curr.children[sp] + if !hasNext { + next = &wordTrieNode{word: sp, children: make(map[string]*wordTrieNode)} + curr.children[sp] = next + } + curr = next + } + // Add termination node so "foo" and "foo bar" have a branching point of "foo". + curr.children[""] = &wordTrieNode{} } diff --git a/test/e2e/split/main_test.go b/test/e2e/split/main_test.go new file mode 100644 index 0000000000..ec2a5410fa --- /dev/null +++ b/test/e2e/split/main_test.go @@ -0,0 +1,141 @@ +package main + +import ( + "os" + "os/exec" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestMain(t *testing.T) { + // This test makes sure that every spec gets run. + + cmd := exec.Command("./test/e2e/split/integration_test.sh") + cmd.Dir = "../../../" + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + err := cmd.Run() + require.NoError(t, err) +} + +func TestCreateChunkRegexp(t *testing.T) { + type spec struct { + name string + numChunks int + printChunk int + specs []string + expRE string + expError string + } + + cases := []spec{ + { + name: "singlePrefix1", + numChunks: 1, printChunk: 0, + specs: []string{"foo"}, + expRE: "foo .*", + }, + { + name: "multiplePrefixes1", + numChunks: 1, printChunk: 0, + specs: []string{"bar foo", "baz", "foo"}, + expRE: "(bar foo|baz|foo) .*", + }, + { + name: "multiplePrefixes2", + numChunks: 3, printChunk: 0, + specs: []string{"bar foo", "baz", "foo"}, + expRE: "bar foo .*", + }, + { + name: "multiplePrefixes3", + numChunks: 3, printChunk: 2, + specs: []string{"bar foo", "baz", "foo"}, + expRE: "foo .*", + }, + { + name: "empty", + numChunks: 1, printChunk: 0, + specs: nil, + expError: "have more desired chunks (1) than specs (0)", + }, + { + name: "singleSpecTooManyChunks", + numChunks: 2, printChunk: 1, + specs: []string{"foo"}, + expError: "have more desired chunks (2) than specs (1)", + }, + { + name: "multipleSpecTooManyChunks", + numChunks: 3, printChunk: 1, + specs: []string{"foo", "bar"}, + expError: "have more desired chunks (3) than specs (2)", + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + re, err := createChunkRegexp(c.numChunks, c.printChunk, c.specs) + if c.expError != "" { + require.EqualError(t, err, c.expError) + } else { + require.NoError(t, err) + require.Equal(t, c.expRE, re) + } + }) + } +} + +func TestFindMinimalWordPrefixes(t *testing.T) { + type spec struct { + name string + specs []string + expPrefixes []string + } + + cases := []spec{ + { + name: "empty", + specs: nil, + expPrefixes: nil, + }, + { + name: "singleSpec", + specs: []string{"foo"}, + expPrefixes: []string{"foo"}, + }, + { + name: "twoSpecsSingleWordPrefix", + specs: []string{"foo", "foo bar"}, + expPrefixes: []string{"foo"}, + }, + { + name: "twoMultiWordSpecsSingleWordPrefix", + specs: []string{"foo bar", "foo baz"}, + expPrefixes: []string{"foo"}, + }, + { + name: "twoMultiWordSpecsLongPrefix", + specs: []string{"foo bar", "foo bar baz"}, + expPrefixes: []string{"foo bar"}, + }, + { + name: "threeSpecsSingleWordPrefix", + specs: []string{"foo", "foo bar", "foo baz"}, + expPrefixes: []string{"foo"}, + }, + { + name: "multiplePrefixes", + specs: []string{"foo", "foo bar", "foo bar baz", "bar foo", "baz buf", "baz bar foo"}, + expPrefixes: []string{"foo", "bar foo", "baz"}, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + prefixes := findMinimalWordPrefixes(c.specs) + require.ElementsMatch(t, c.expPrefixes, prefixes) + }) + } +} From baf0cadda295a53d405e1450ae06d7d4d71003ca Mon Sep 17 00:00:00 2001 From: timflannagan Date: Mon, 17 Jan 2022 21:25:43 -0500 Subject: [PATCH 3/3] .github: Aggregate e2e matrix jobs into a single status Signed-off-by: timflannagan --- .github/workflows/e2e-tests.yml | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index cf9b1a52ef..9adfb08cbc 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -8,7 +8,7 @@ on: pull_request: workflow_dispatch: jobs: - e2e-tests: + e2e: strategy: fail-fast: false matrix: @@ -26,4 +26,16 @@ jobs: with: name: e2e-test-output-${{ (github.event.pull_request.head.sha || github.sha) }}-${{ github.run_id }}-${{ matrix.parallel-id }} path: ${{ github.workspace }}/bin/artifacts/* -# TODO: create job to combine test artifacts using code in https://github.com/operator-framework/operator-lifecycle-manager/pull/1476 + # TODO: create job to combine test artifacts using code in https://github.com/operator-framework/operator-lifecycle-manager/pull/1476 + e2e-tests: + if: ${{ always() }} + runs-on: ubuntu-latest + needs: e2e + steps: + - run: | + echo "Matrix result: ${{ needs.e2e.result }}" + - name: check individual matrix results + if: ${{ needs.e2e.result == 'failure' }} + run: | + echo 'Failure: at least one e2e matrix job has failed' + exit 1