Enhance tests (use ginkgo/v2)

* Make sure `ginkgo/v2` is used everywhere * Adapt the test code in a first shot to make use of the new Ginkgo capabilities (`DeferCleanup`, easier global timeouts / thresholds)
ironcore-dev · Apr 27, 2022 · 80beded · 80beded
1 parent 82e4b74
commit 80beded
Show file tree

Hide file tree

Showing 27 changed files with 218 additions and 130 deletions.
diff --git a/Makefile b/Makefile
@@ -90,9 +90,12 @@ start-docs: ## Start the local mkdocs based development environment.
 clean-docs: ## Remove all local mkdocs Docker images (cleanup).
 	docker container prune --force --filter "label=project=onmetal_api_documentation"
 
-ENVTEST_ASSETS_DIR=$(shell pwd)/testbin
 .PHONY: test
-test: manifests generate fmt vet ## Run tests.
+test: manifests generate fmt vet test-only ## Run tests.
+
+ENVTEST_ASSETS_DIR=$(shell pwd)/testbin
+.PHONY: test-only
+test-only: ## Run *only* the tests - no generation, linting etc.
 	mkdir -p ${ENVTEST_ASSETS_DIR}
 	test -f ${ENVTEST_ASSETS_DIR}/setup-envtest.sh || curl -sSLo ${ENVTEST_ASSETS_DIR}/setup-envtest.sh https://raw.githubusercontent.com/kubernetes-sigs/controller-runtime/v0.8.3/hack/setup-envtest.sh
 	source ${ENVTEST_ASSETS_DIR}/setup-envtest.sh; fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./... -coverprofile cover.out

diff --git a/apis/common/v1alpha1/common_types_test.go b/apis/common/v1alpha1/common_types_test.go
@@ -17,7 +17,7 @@
 package v1alpha1
 
 import (
-	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
 

diff --git a/apis/common/v1alpha1/suite_test.go b/apis/common/v1alpha1/suite_test.go
@@ -19,7 +19,7 @@ package v1alpha1
 import (
 	"testing"
 
-	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
 

diff --git a/apis/ipam/validation/prefix_test.go b/apis/ipam/validation/prefix_test.go
@@ -19,8 +19,7 @@ import (
 	"github.com/onmetal/onmetal-api/apis/ipam"
 	. "github.com/onmetal/onmetal-api/apis/ipam/validation"
 	. "github.com/onmetal/onmetal-api/testutils/validation"
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/types"
 	corev1 "k8s.io/api/core/v1"

diff --git a/apis/ipam/validation/prefixallocation_test.go b/apis/ipam/validation/prefixallocation_test.go
@@ -18,8 +18,7 @@ import (
 	commonv1alpha1 "github.com/onmetal/onmetal-api/apis/common/v1alpha1"
 	"github.com/onmetal/onmetal-api/apis/ipam"
 	. "github.com/onmetal/onmetal-api/testutils/validation"
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/types"
 	corev1 "k8s.io/api/core/v1"

diff --git a/apis/ipam/validation/prefixtemplate_test.go b/apis/ipam/validation/prefixtemplate_test.go
@@ -17,8 +17,7 @@ package validation_test
 import (
 	"github.com/onmetal/onmetal-api/apis/ipam"
 	. "github.com/onmetal/onmetal-api/testutils/validation"
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/types"
 	corev1 "k8s.io/api/core/v1"

diff --git a/apis/ipam/validation/validation_suite_test.go b/apis/ipam/validation/validation_suite_test.go
@@ -17,7 +17,7 @@ package validation_test
 import (
 	"testing"
 
-	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
 

diff --git a/apis/storage/validation/validation_suite_test.go b/apis/storage/validation/validation_suite_test.go
@@ -17,7 +17,7 @@ package validation_test
 import (
 	"testing"
 
-	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
 

diff --git a/apis/storage/validation/volume_test.go b/apis/storage/validation/volume_test.go
@@ -18,8 +18,7 @@ import (
 	"github.com/onmetal/onmetal-api/apis/storage"
 	. "github.com/onmetal/onmetal-api/apis/storage/validation"
 	. "github.com/onmetal/onmetal-api/testutils/validation"
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/types"
 	corev1 "k8s.io/api/core/v1"

diff --git a/apis/storage/validation/volumeclaim_test.go b/apis/storage/validation/volumeclaim_test.go
@@ -18,8 +18,7 @@ import (
 	"github.com/onmetal/onmetal-api/apis/storage"
 	. "github.com/onmetal/onmetal-api/apis/storage/validation"
 	. "github.com/onmetal/onmetal-api/testutils/validation"
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/types"
 	corev1 "k8s.io/api/core/v1"

diff --git a/apis/storage/validation/volumeclass_test.go b/apis/storage/validation/volumeclass_test.go
@@ -18,8 +18,7 @@ import (
 	"github.com/onmetal/onmetal-api/apis/storage"
 	. "github.com/onmetal/onmetal-api/apis/storage/validation"
 	. "github.com/onmetal/onmetal-api/testutils/validation"
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/types"
 	corev1 "k8s.io/api/core/v1"

diff --git a/apis/storage/validation/volumepool_test.go b/apis/storage/validation/volumepool_test.go
@@ -18,8 +18,7 @@ import (
 	"github.com/onmetal/onmetal-api/apis/storage"
 	. "github.com/onmetal/onmetal-api/apis/storage/validation"
 	. "github.com/onmetal/onmetal-api/testutils/validation"
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/types"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

diff --git a/controllers/compute/machine_controller_test.go b/controllers/compute/machine_controller_test.go
@@ -17,7 +17,7 @@
 package compute
 
 import (
-	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/v2"
 )
 
 var _ = Describe("machine controller", func() {

diff --git a/controllers/compute/machine_scheduler_test.go b/controllers/compute/machine_scheduler_test.go
@@ -15,7 +15,8 @@
 package compute
 
 import (
-	. "github.com/onsi/ginkgo"
+	"github.com/onmetal/onmetal-api/testutils"
+	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -26,6 +27,7 @@ import (
 )
 
 var _ = Describe("MachineScheduler", func() {
+	ctx := testutils.SetupContext()
 	ns := SetupTest(ctx)
 
 	It("should schedule machines on machine pools", func() {
@@ -219,7 +221,7 @@ var _ = Describe("MachineScheduler", func() {
 		Consistently(func() string {
 			Expect(k8sClient.Get(ctx, machineKey, machine)).To(Succeed())
 			return machine.Spec.MachinePoolRef.Name
-		}, timeout, interval).Should(BeEmpty())
+		}).Should(BeEmpty())
 
 		By("patching the machine to contain only one of the corresponding tolerations")
 		machineBase := machine.DeepCopy()
@@ -235,7 +237,7 @@ var _ = Describe("MachineScheduler", func() {
 		Consistently(func() string {
 			Expect(k8sClient.Get(ctx, machineKey, machine)).To(Succeed())
 			return machine.Spec.MachinePoolRef.Name
-		}, timeout, interval).Should(BeEmpty())
+		}).Should(BeEmpty())
 
 		By("patching the machine to contain all of the corresponding tolerations")
 		machineBase = machine.DeepCopy()

diff --git a/controllers/compute/machineclass_controller_test.go b/controllers/compute/machineclass_controller_test.go
@@ -17,7 +17,8 @@
 package compute
 
 import (
-	. "github.com/onsi/ginkgo"
+	"github.com/onmetal/onmetal-api/testutils"
+	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -28,6 +29,7 @@ import (
 )
 
 var _ = Describe("machineclass controller", func() {
+	ctx := testutils.SetupContext()
 	ns := SetupTest(ctx)
 	It("removes the finalizer from machineclass only if there's no machine still using the machineclass", func() {
 		By("creating the machineclass consumed by the machine")
@@ -60,7 +62,7 @@ var _ = Describe("machineclass controller", func() {
 			Expect(client.IgnoreNotFound(err)).To(Succeed(), "errors other than `not found` are not expected")
 			g.Expect(err).NotTo(HaveOccurred())
 			return machineClass.Finalizers
-		}, timeout, interval).Should(ContainElement(computev1alpha1.MachineClassFinalizer))
+		}).Should(ContainElement(computev1alpha1.MachineClassFinalizer))
 
 		By("checking the machineclass and its finalizer consistently exist upon deletion ")
 		Expect(k8sClient.Delete(ctx, machineClass)).Should(Succeed())
@@ -70,14 +72,14 @@ var _ = Describe("machineclass controller", func() {
 			Expect(client.IgnoreNotFound(err)).To(Succeed(), "errors other than `not found` are not expected")
 			g.Expect(err).NotTo(HaveOccurred())
 			return machineClass.Finalizers
-		}, timeout).Should(ContainElement(computev1alpha1.MachineClassFinalizer))
+		}).Should(ContainElement(computev1alpha1.MachineClassFinalizer))
 
 		By("checking the machineclass is eventually gone after the deletion of the machine")
 		Expect(k8sClient.Delete(ctx, m)).Should(Succeed())
 		Eventually(func() bool {
 			err := k8sClient.Get(ctx, machineClassKey, machineClass)
 			Expect(client.IgnoreNotFound(err)).To(Succeed(), "errors other than `not found` are not expected")
 			return apierrors.IsNotFound(err)
-		}, timeout, interval).Should(BeTrue(), "the error should be `not found`")
+		}).Should(BeTrue(), "the error should be `not found`")
 	})
 })
diff --git a/controllers/compute/suite_test.go b/controllers/compute/suite_test.go
@@ -24,7 +24,7 @@ import (
 	"github.com/onmetal/onmetal-api/envtestutils"
 	"github.com/onmetal/onmetal-api/envtestutils/apiserver"
 	"github.com/onmetal/onmetal-api/testutils/apiserverbin"
-	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -45,20 +45,26 @@ import (
 // http://onsi.github.io/ginkgo/ to learn more about Ginkgo.
 
 const (
-	interval = 50 * time.Millisecond
-	timeout  = 3 * time.Second
+	pollingInterval      = 50 * time.Millisecond
+	eventuallyTimeout    = 3 * time.Second
+	consistentlyDuration = 1 * time.Second
 )
 
 var (
-	ctx, cancel = context.WithCancel(context.Background())
-
 	cfg        *rest.Config
 	k8sClient  client.Client
 	testEnv    *envtest.Environment
 	testEnvExt *envtestutils.EnvironmentExtensions
 )
 
 func TestAPIs(t *testing.T) {
+	_, reporterConfig := GinkgoConfiguration()
+	reporterConfig.SlowSpecThreshold = 10 * time.Second
+	SetDefaultConsistentlyPollingInterval(pollingInterval)
+	SetDefaultEventuallyPollingInterval(pollingInterval)
+	SetDefaultEventuallyTimeout(eventuallyTimeout)
+	SetDefaultConsistentlyDuration(consistentlyDuration)
+
 	RegisterFailHandler(Fail)
 
 	RunSpecs(t, "Compute Controller Suite")
@@ -79,6 +85,7 @@ var _ = BeforeSuite(func() {
 	cfg, err = envtestutils.StartWithExtensions(testEnv, testEnvExt)
 	Expect(err).NotTo(HaveOccurred())
 	Expect(cfg).NotTo(BeNil())
+	DeferCleanup(envtestutils.StopWithExtensions, testEnv, testEnvExt)
 
 	Expect(computev1alpha1.AddToScheme(scheme.Scheme)).To(Succeed())
 	Expect(ipamv1alpha1.AddToScheme(scheme.Scheme)).To(Succeed())
@@ -98,15 +105,17 @@ var _ = BeforeSuite(func() {
 	})
 	Expect(err).NotTo(HaveOccurred())
 
+	ctx, cancel := context.WithCancel(context.Background())
+	DeferCleanup(cancel)
 	go func() {
 		defer GinkgoRecover()
 		err := apiSrv.Start(ctx)
 		Expect(err).NotTo(HaveOccurred())
 	}()
 
-	err = envtestutils.WaitUntilAPIServicesReadyWithTimeout(30*time.Second, testEnvExt, k8sClient, scheme.Scheme)
+	err = envtestutils.WaitUntilAPIServicesReadyWithTimeout(60*time.Second, testEnvExt, k8sClient, scheme.Scheme)
 	Expect(err).NotTo(HaveOccurred())
-}, 60)
+})
 
 // SetupTest returns a namespace which will be created before each ginkgo `It` block and deleted at the end of `It`
 // so that each test case can run in an independent way
@@ -151,6 +160,7 @@ func SetupTest(ctx context.Context) *corev1.Namespace {
 		}).SetupWithManager(k8sManager)).To(Succeed())
 
 		go func() {
+			defer GinkgoRecover()
 			Expect(k8sManager.Start(mgrCtx)).To(Succeed(), "failed to start manager")
 		}()
 	})
@@ -163,10 +173,3 @@ func SetupTest(ctx context.Context) *corev1.Namespace {
 
 	return ns
 }
-
-var _ = AfterSuite(func() {
-	cancel()
-	By("tearing down the test environment")
-	err := envtestutils.StopWithExtensions(testEnv, testEnvExt)
-	Expect(err).NotTo(HaveOccurred())
-})