diff --git a/hack/verify-e2e-suites.sh b/hack/verify-e2e-suites.sh new file mode 100755 index 00000000000..1235a838c37 --- /dev/null +++ b/hack/verify-e2e-suites.sh @@ -0,0 +1,45 @@ +#!/usr/bin/env bash + +# Copyright 2021 The Kubernetes Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# This script checks that all E2E test suites are sane, i.e. can be +# started without an error. + +set -o errexit +set -o nounset +set -o pipefail + +KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/.. +source "${KUBE_ROOT}/hack/lib/init.sh" +source "${KUBE_ROOT}/hack/lib/util.sh" + +kube::golang::verify_go_version + +cd "${KUBE_ROOT}" + +kube::util::ensure-temp-dir + +for suite in $(git grep -l framework.AfterReadingAllFlags | grep -v -e ^test/e2e/framework -e ^hack | xargs -n 1 dirname | sort -u); do + # Build a binary and run it in the root directory to get paths that are + # relative to that instead of the package directory. + out="" + if (cd "$suite" && go test -c -o "${KUBE_TEMP}/e2e.bin" .) && out=$("${KUBE_TEMP}/e2e.bin" --list-tests); then + echo "E2E suite $suite passed." + else + echo >&2 "ERROR: E2E test suite invocation failed for $suite." + # shellcheck disable=SC2001 + echo "$out" | sed -e 's/^/ /' + fi +done diff --git a/test/e2e/framework/bugs.go b/test/e2e/framework/bugs.go new file mode 100644 index 00000000000..a8202353307 --- /dev/null +++ b/test/e2e/framework/bugs.go @@ -0,0 +1,108 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package framework + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + "sync" + + "github.com/onsi/ginkgo/v2/types" +) + +var ( + bugs []Bug + bugMutex sync.Mutex +) + +// RecordBug stores information about a bug in the E2E suite source code that +// cannot be reported through ginkgo.Fail because it was found outside of some +// test, for example during test registration. +// +// This can be used instead of raising a panic. Then all bugs can be reported +// together instead of failing after the first one. +func RecordBug(bug Bug) { + bugMutex.Lock() + defer bugMutex.Unlock() + + bugs = append(bugs, bug) +} + +type Bug struct { + FileName string + LineNumber int + Message string +} + +// NewBug creates a new bug with a location that is obtained by skipping a certain number +// of stack frames. Passing zero will record the source code location of the direct caller +// of NewBug. +func NewBug(message string, skip int) Bug { + location := types.NewCodeLocation(skip + 1) + return Bug{FileName: location.FileName, LineNumber: location.LineNumber, Message: message} +} + +// FormatBugs produces a report that includes all bugs recorded earlier via +// RecordBug. An error is returned with the report if there have been bugs. +func FormatBugs() error { + bugMutex.Lock() + defer bugMutex.Unlock() + + if len(bugs) == 0 { + return nil + } + + lines := make([]string, 0, len(bugs)) + wd, err := os.Getwd() + if err != nil { + return fmt.Errorf("get current directory: %v", err) + } + // Sort by file name, line number, message. For the sake of simplicity + // this uses the full file name even though the output the may use a + // relative path. Usually the result should be the same because full + // paths will all have the same prefix. + sort.Slice(bugs, func(i, j int) bool { + switch strings.Compare(bugs[i].FileName, bugs[j].FileName) { + case -1: + return true + case 1: + return false + } + if bugs[i].LineNumber < bugs[j].LineNumber { + return true + } + if bugs[i].LineNumber > bugs[j].LineNumber { + return false + } + return bugs[i].Message < bugs[j].Message + }) + for _, bug := range bugs { + // Use relative paths, if possible. + path := bug.FileName + if wd != "" { + if relpath, err := filepath.Rel(wd, bug.FileName); err == nil { + path = relpath + } + } + lines = append(lines, fmt.Sprintf("ERROR: %s:%d: %s\n", path, bug.LineNumber, strings.TrimSpace(bug.Message))) + } + return errors.New(strings.Join(lines, "")) +} diff --git a/test/e2e/framework/internal/unittests/bugs/bugs_test.go b/test/e2e/framework/internal/unittests/bugs/bugs_test.go new file mode 100644 index 00000000000..f314b117392 --- /dev/null +++ b/test/e2e/framework/internal/unittests/bugs/bugs_test.go @@ -0,0 +1,79 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package bugs + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "k8s.io/kubernetes/test/e2e/framework" +) + +// The line number of the following code is checked in BugOutput below. +// Be careful when moving it around or changing the import statements above. +// Here are some intentionally blank lines that can be removed to compensate +// for future additional import statements. +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// This must be line #50. + +func helper() { + framework.RecordBug(framework.NewBug("new bug", 0)) + framework.RecordBug(framework.NewBug("parent", 1)) +} + +func recordBugs() { + helper() + framework.RecordBug(framework.Bug{FileName: "buggy/buggy.go", LineNumber: 100, Message: "hello world"}) + framework.RecordBug(framework.Bug{FileName: "some/relative/path/buggy.go", LineNumber: 200, Message: " with spaces \n"}) +} + +const ( + numBugs = 3 + bugOutput = `ERROR: bugs_test.go:53: new bug +ERROR: bugs_test.go:58: parent +ERROR: buggy/buggy.go:100: hello world +ERROR: some/relative/path/buggy.go:200: with spaces +` +) + +func TestBugs(t *testing.T) { + assert.NoError(t, framework.FormatBugs()) + recordBugs() + err := framework.FormatBugs() + if assert.Error(t, err) { + assert.Equal(t, bugOutput, err.Error()) + } +} diff --git a/test/e2e/framework/test_context.go b/test/e2e/framework/test_context.go index 763fdd1d82a..97ca535f0e4 100644 --- a/test/e2e/framework/test_context.go +++ b/test/e2e/framework/test_context.go @@ -507,8 +507,16 @@ func AfterReadingAllFlags(t *TestContextType) { gomega.SetDefaultEventuallyTimeout(t.timeouts.PodStart) gomega.SetDefaultConsistentlyDuration(t.timeouts.PodStartShort) + // ginkgo.PreviewSpecs will expand all nodes and thus may find new bugs. + report := ginkgo.PreviewSpecs("Kubernetes e2e test statistics") + if err := FormatBugs(); err != nil { + // Refuse to do anything if the E2E suite is buggy. + fmt.Fprint(Output, "ERROR: E2E suite initialization was faulty, these errors must be fixed:") + fmt.Fprint(Output, "\n"+err.Error()) + os.Exit(1) + } if t.listLabels || t.listTests { - listTestInformation(ginkgo.PreviewSpecs("Kubernetes e2e test statistics")) + listTestInformation(report) os.Exit(0) }