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.

Sign up for GitHub

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

Separate out integration tests: #6723

Closed
wants to merge 3 commits into from
Closed

Separate out integration tests: #6723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3e1bd59
Separate out integration tests:
jacobweinstock Sep 22, 2023
58c4344
Add a test marker for integration test:
jacobweinstock Sep 27, 2023
afed219
Fix linting issue
jacobweinstock Sep 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,18 +422,20 @@ verify-generate: generate ## Verify if generated zz_generated.deepcopy.go files
exit 1;\
fi


.PHONY: test
test: unit-test capd-test ## Run unit and capd tests

ifneq "$(INTEGRATION_TESTS_ENABLED)" "false"
export INTEGRATION_TESTS_ENABLED = true
endif

.PHONY: unit-test
unit-test: ## Run unit tests
unit-test: $(SETUP_ENVTEST)
unit-test: $(SETUP_ENVTEST)
unit-test: KUBEBUILDER_ASSETS ?= $(shell $(SETUP_ENVTEST) use --use-env -p path --arch $(GO_ARCH) $(KUBEBUILDER_ENVTEST_KUBERNETES_VERSION))
unit-test:
KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" $(GO_TEST) $(UNIT_TEST_PACKAGES) -cover -tags "$(BUILD_TAGS)" $(GO_TEST_FLAGS)


# unit-test-patch is a convenience target that restricts test runs to modified
# packages' tests. This is not a replacement for running the unit-test target,
# but can be useful for red-green development or targeted refactoring.
Expand Down
9 changes: 9 additions & 0 deletions controllers/cloudstack_datacenter_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"github.com/aws/eks-anywhere/controllers"
"github.com/aws/eks-anywhere/internal/test"
anywherev1 "github.com/aws/eks-anywhere/pkg/api/v1alpha1"
"github.com/aws/eks-anywhere/pkg/constants"
"github.com/aws/eks-anywhere/pkg/providers/cloudstack"
"github.com/aws/eks-anywhere/pkg/providers/cloudstack/decoder"
)

func TestCloudStackDatacenterReconcilerSetupWithManager(t *testing.T) {
test.MarkIntegration(t)
Copy link
Member

Choose a reason for hiding this comment

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

this made me think
What if we made engineers have to call this/some other method in order to use envtest?

Right now this method just skips the test if the env var is not set. But if the engineer doesn't add this line, the test will compile and try to run.

We could however "require" this method. Right now all tests using envtest need a package level variable called env, that is setup from TestMain. What if we set a pattern where instead of accessing this variable directly, we call a method to retrieve it? That way engineers will "need" to call this method if they want access to envtest. Of course, they could still look for the package level variable, but if we name it something like internalEnvtest and add a decent comment, I doubt this will happen by mistake.

Quick example of what I'm proposing:

func TestCloudStackDatacenterReconcilerSetupWithManager(t *testing.T) {
  env := requireEnvtest(t)

The in this package main_test.go:

// internalEnvtest allows to use envtest when integration
// tests are enabled.
// DO NOT use it directly, instead use requireEnvtest.
var internalEnvtest *envtest.Environment

func requireEnvtest(tb testing.TB) *envtest.Environment {
  test.MarkIntegration(t)
  return internalEnvtest
}

Copy link
Contributor

@chrisdoherty4 chrisdoherty4 Oct 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sold on the value. envtest features in a subset of integration tests so its not a silver bullet. Moreover, developers have the freedom to tweak envtest configuration dependent on their usecase which may mean not using any of the existing setup (package state isn't required for envtest; see Hegel integration tests as an example).

I suggest we reserve this change for a separate PR.

client := env.Client()
r := controllers.NewCloudStackDatacenterReconciler(client, nil)

Expand All @@ -31,6 +33,7 @@ func TestCloudStackDatacenterReconcilerSetupWithManager(t *testing.T) {
}

func TestCloudStackDatacenterReconcilerSuccess(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
ctx := context.Background()

Expand Down Expand Up @@ -79,6 +82,7 @@ func TestCloudStackDatacenterReconcilerSuccess(t *testing.T) {
}

func TestCloudStackDatacenterReconcilerSetDefaultSuccess(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
ctx := context.Background()

Expand Down Expand Up @@ -146,6 +150,7 @@ func TestCloudStackDatacenterReconcilerSetDefaultSuccess(t *testing.T) {
}

func TestCloudstackDatacenterConfigReconcilerDelete(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
ctx := context.Background()

Expand All @@ -170,6 +175,7 @@ func TestCloudstackDatacenterConfigReconcilerDelete(t *testing.T) {
}

func TestCloudstackDatacenterConfigGetValidatorFailure(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
ctx := context.Background()

Expand Down Expand Up @@ -198,6 +204,7 @@ func TestCloudstackDatacenterConfigGetValidatorFailure(t *testing.T) {
}

func TestCloudstackDatacenterConfigGetDatacenterFailure(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
ctx := context.Background()
client := fake.NewClientBuilder().WithScheme(runtime.NewScheme()).Build()
Expand All @@ -217,6 +224,7 @@ func TestCloudstackDatacenterConfigGetDatacenterFailure(t *testing.T) {
}

func TestCloudstackDatacenterConfigGetExecConfigFailure(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
ctx := context.Background()

Expand All @@ -243,6 +251,7 @@ func TestCloudstackDatacenterConfigGetExecConfigFailure(t *testing.T) {
}

func TestCloudstackDatacenterConfigAccountNotPresentFailure(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
ctx := context.Background()
dcConfig := createCloudstackDatacenterConfig()
Expand Down
2 changes: 2 additions & 0 deletions controllers/cluster_controller_test_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
)

func TestClusterReconcilerEnsureOwnerReferences(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
ctx := context.Background()

Expand Down Expand Up @@ -204,6 +205,7 @@ func TestClusterReconcilerReconcileChildObjectNotFound(t *testing.T) {
}

func TestClusterReconcilerSetupWithManager(t *testing.T) {
test.MarkIntegration(t)
client := env.Client()
r := controllers.NewClusterReconciler(client, newRegistryForDummyProviderReconciler(), newMockAWSIamConfigReconciler(t), newMockClusterValidator(t), nil, nil)

Expand Down
7 changes: 6 additions & 1 deletion controllers/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@ import (
"os"
"testing"

"github.com/aws/eks-anywhere/internal/test"
"github.com/aws/eks-anywhere/internal/test/envtest"
)

var env *envtest.Environment

func TestMain(m *testing.M) {
os.Exit(envtest.RunWithEnvironment(m, envtest.WithAssignment(&env)))
if os.Getenv(test.IntegrationTestEnvVar) == "true" {
os.Exit(envtest.RunWithEnvironment(m, envtest.WithAssignment(&env)))
} else {
os.Exit(m.Run())
}
}
2 changes: 2 additions & 0 deletions controllers/nutanix_datacenter_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"github.com/aws/eks-anywhere/controllers"
"github.com/aws/eks-anywhere/internal/test"
anywherev1 "github.com/aws/eks-anywhere/pkg/api/v1alpha1"
"github.com/aws/eks-anywhere/pkg/providers/nutanix"
)

func TestNutanixDatacenterConfigReconcilerSetupWithManager(t *testing.T) {
test.MarkIntegration(t)
client := env.Client()
r := controllers.NewNutanixDatacenterReconciler(client, nutanix.NewDefaulter())

Expand Down
2 changes: 2 additions & 0 deletions controllers/snow_machineconfig_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (

"github.com/aws/eks-anywhere/controllers"
"github.com/aws/eks-anywhere/controllers/mocks"
"github.com/aws/eks-anywhere/internal/test"
anywherev1 "github.com/aws/eks-anywhere/pkg/api/v1alpha1"
)

Expand All @@ -27,6 +28,7 @@ var (
)

func TestSnowMachineConfigReconcilerSetupWithManager(t *testing.T) {
test.MarkIntegration(t)
client := env.Client()
r := controllers.NewSnowMachineConfigReconciler(client, nil)

Expand Down
2 changes: 2 additions & 0 deletions controllers/tinkerbell_datacenter_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import (
. "github.com/onsi/gomega"

"github.com/aws/eks-anywhere/controllers"
"github.com/aws/eks-anywhere/internal/test"
)

func TestTinkerbellDatacenterReconcilerSetupWithManager(t *testing.T) {
test.MarkIntegration(t)
client := env.Client()
r := controllers.NewTinkerbellDatacenterReconciler(client)

Expand Down
18 changes: 18 additions & 0 deletions internal/test/markers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package test

import (
"os"
"testing"
)

// IntegrationTestEnvVar is the name of the environment variable that gates when integration tests are run.
const IntegrationTestEnvVar = "INTEGRATION_TESTS_ENABLED"

// MarkIntegration marks a test as an integration test.
// Integration tests are only run when the environment variable INTEGRATION_TESTS_ENABLED is set to true.
func MarkIntegration(t *testing.T) {
t.Helper()
if os.Getenv(IntegrationTestEnvVar) != "true" {
t.Skipf("set env var '%v=true' to run this test", IntegrationTestEnvVar)
}
}
2 changes: 2 additions & 0 deletions pkg/clients/kubernetes/runtimeclient_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
)

func TestNewRuntimeClient(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
cfg := test.UseEnvTest(t)
rc := kubernetes.RestConfigurator(func(_ []byte) (*rest.Config, error) { return cfg, nil })
Expand Down Expand Up @@ -51,6 +52,7 @@ func TestNewRuntimeClientInvalidRestConfig(t *testing.T) {
}

func TestNewRuntimeClientInvalidScheme(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
cfg := test.UseEnvTest(t)
rc := kubernetes.RestConfigurator(func(_ []byte) (*rest.Config, error) { return cfg, nil })
Expand Down
6 changes: 6 additions & 0 deletions pkg/controller/clusters/controlplane_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
)

func TestReconcileControlPlaneStackedEtcd(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
c := env.Client()
api := envtest.NewAPIExpecter(t, c)
Expand All @@ -42,6 +43,7 @@ func TestReconcileControlPlaneStackedEtcd(t *testing.T) {
}

func TestReconcileControlPlaneExternalEtcdNewCluster(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
c := env.Client()
api := envtest.NewAPIExpecter(t, c)
Expand Down Expand Up @@ -72,6 +74,7 @@ func TestReconcileControlPlaneExternalEtcdNewCluster(t *testing.T) {
}

func TestReconcileControlPlaneExternalEtcdUpgradeWithDiff(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
c := env.Client()
api := envtest.NewAPIExpecter(t, c)
Expand Down Expand Up @@ -129,6 +132,7 @@ func TestReconcileControlPlaneExternalEtcdUpgradeWithDiff(t *testing.T) {
}

func TestReconcileControlPlaneExternalEtcdNotReady(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
c := env.Client()
api := envtest.NewAPIExpecter(t, c)
Expand Down Expand Up @@ -166,6 +170,7 @@ func TestReconcileControlPlaneExternalEtcdNotReady(t *testing.T) {
}

func TestReconcileControlPlaneExternalEtcdReadyControlPlaneUpgrade(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
c := env.Client()
api := envtest.NewAPIExpecter(t, c)
Expand Down Expand Up @@ -223,6 +228,7 @@ func TestReconcileControlPlaneExternalEtcdReadyControlPlaneUpgrade(t *testing.T)
}

func TestReconcileControlPlaneExternalEtcdUpgradeWithNoNamespace(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
c := env.Client()
api := envtest.NewAPIExpecter(t, c)
Expand Down
7 changes: 6 additions & 1 deletion pkg/controller/clusters/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@ import (
"os"
"testing"

"github.com/aws/eks-anywhere/internal/test"
"github.com/aws/eks-anywhere/internal/test/envtest"
)

var env *envtest.Environment

func TestMain(m *testing.M) {
os.Exit(envtest.RunWithEnvironment(m, envtest.WithAssignment(&env)))
if os.Getenv(test.IntegrationTestEnvVar) == "true" {
os.Exit(envtest.RunWithEnvironment(m, envtest.WithAssignment(&env)))
} else {
os.Exit(m.Run())
}
}
5 changes: 5 additions & 0 deletions pkg/controller/clusters/workers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
)

func TestReconcileWorkersSuccess(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
c := env.Client()
api := envtest.NewAPIExpecter(t, c)
Expand Down Expand Up @@ -66,6 +67,7 @@ func TestReconcileWorkersSuccess(t *testing.T) {
}

func TestReconcileWorkersErrorApplyingObjects(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
c := env.Client()
ctx := context.Background()
Expand Down Expand Up @@ -192,6 +194,7 @@ func dockerMachineTemplate(name, namespace string) *dockerv1.DockerMachineTempla
}

func TestReconcileWorkersForEKSAErrorGettingCAPICluster(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
c := fake.NewClientBuilder().WithScheme(runtime.NewScheme()).Build()
ctx := context.Background()
Expand All @@ -210,6 +213,7 @@ func TestReconcileWorkersForEKSAErrorGettingCAPICluster(t *testing.T) {
}

func TestReconcileWorkersForEKSANoCAPICluster(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
c := env.Client()
ctx := context.Background()
Expand All @@ -228,6 +232,7 @@ func TestReconcileWorkersForEKSANoCAPICluster(t *testing.T) {
}

func TestReconcileWorkersForEKSASuccess(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
c := env.Client()
api := envtest.NewAPIExpecter(t, c)
Expand Down
4 changes: 4 additions & 0 deletions pkg/controller/serverside/applier_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/aws/eks-anywhere/internal/test"
"github.com/aws/eks-anywhere/pkg/clients/kubernetes"
"github.com/aws/eks-anywhere/pkg/controller"
"github.com/aws/eks-anywhere/pkg/controller/serverside"
)

func TestObjectApplierApplySuccess(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
ctx := context.Background()
namespace := env.CreateNamespaceForTest(ctx, t)
Expand All @@ -27,6 +29,7 @@ func TestObjectApplierApplySuccess(t *testing.T) {
}

func TestObjectApplierApplyErrorFromGenerator(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
ctx := context.Background()

Expand All @@ -38,6 +41,7 @@ func TestObjectApplierApplyErrorFromGenerator(t *testing.T) {
}

func TestObjectApplierApplyErrorApplying(t *testing.T) {
test.MarkIntegration(t)
g := NewWithT(t)
ctx := context.Background()

Expand Down
7 changes: 6 additions & 1 deletion pkg/controller/serverside/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@ import (
"os"
"testing"

"github.com/aws/eks-anywhere/internal/test"
"github.com/aws/eks-anywhere/internal/test/envtest"
)

var env *envtest.Environment

func TestMain(m *testing.M) {
os.Exit(envtest.RunWithEnvironment(m, envtest.WithAssignment(&env)))
if os.Getenv(test.IntegrationTestEnvVar) == "true" {
os.Exit(envtest.RunWithEnvironment(m, envtest.WithAssignment(&env)))
} else {
os.Exit(m.Run())
}
}
4 changes: 4 additions & 0 deletions pkg/controller/serverside/reconcile_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ import (
clusterapiv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/aws/eks-anywhere/internal/test"
"github.com/aws/eks-anywhere/pkg/controller/clientutil"
"github.com/aws/eks-anywhere/pkg/controller/serverside"
)

func TestReconcileYaml(t *testing.T) {
test.MarkIntegration(t)
cluster1 := newCluster("cluster-1")
cluster2 := newCluster("cluster-2")
tests := []struct {
Expand Down Expand Up @@ -124,6 +126,7 @@ spec:
}

func TestReconcileUpdateObject(t *testing.T) {
test.MarkIntegration(t)
cluster1 := newCluster("cluster-1")

yaml := []byte(`apiVersion: cluster.x-k8s.io/v1beta1
Expand Down Expand Up @@ -193,6 +196,7 @@ spec:
}

func TestReconcileUpdateObjectError(t *testing.T) {
test.MarkIntegration(t)
cluster1 := newCluster("cluster-1")

yaml := []byte(`apiVersion: cluster.x-k8s.io/v1beta1
Expand Down
Loading