From 35e92e6ab6ddbdf908aabd1bc8b9cbaf01ceafd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9na=C3=AFc=20Huard?= Date: Tue, 7 Nov 2023 23:33:09 +0100 Subject: [PATCH] [CONTINT-258] Leverage the testify assertion helpers in e2e tests (#20625) --- test/new-e2e/tests/containers/base_test.go | 50 ++++++------ test/new-e2e/tests/containers/ecs_test.go | 31 +++---- test/new-e2e/tests/containers/k8s_test.go | 95 ++++++++++------------ 3 files changed, 78 insertions(+), 98 deletions(-) diff --git a/test/new-e2e/tests/containers/base_test.go b/test/new-e2e/tests/containers/base_test.go index 6ccaf12f5c9bb..77be297dd1e60 100644 --- a/test/new-e2e/tests/containers/base_test.go +++ b/test/new-e2e/tests/containers/base_test.go @@ -84,7 +84,9 @@ func (mc *myCollectT) Errorf(format string, args ...interface{}) { } func (suite *baseSuite) testMetric(args *testMetricArgs) { - suite.Run(fmt.Sprintf("%s{%s}", args.Filter.Name, strings.Join(args.Filter.Tags, ",")), func() { + prettyMetricQuery := fmt.Sprintf("%s{%s}", args.Filter.Name, strings.Join(args.Filter.Tags, ",")) + + suite.Run(prettyMetricQuery, func() { var expectedTags []*regexp.Regexp if args.Expect.Tags != nil { expectedTags = lo.Map(*args.Expect.Tags, func(tag string, _ int) *regexp.Regexp { return regexp.MustCompile(tag) }) @@ -99,7 +101,7 @@ func (suite *baseSuite) testMetric(args *testMetricArgs) { }) if _, err := suite.datadogClient.PostEvent(&datadog.Event{ - Title: pointer.Ptr(fmt.Sprintf("testMetric %s{%s}", args.Filter.Name, strings.Join(args.Filter.Tags, ","))), + Title: pointer.Ptr(fmt.Sprintf("testMetric %s", prettyMetricQuery)), Text: pointer.Ptr(fmt.Sprintf(`%%%%%% ### Result @@ -127,25 +129,25 @@ func (suite *baseSuite) testMetric(args *testMetricArgs) { defer func() { if suite.T().Failed() { - sendEvent("error", fmt.Sprintf("Failed finding %s{%s} with proper tags", args.Filter.Name, strings.Join(args.Filter.Tags, ","))) + sendEvent("error", fmt.Sprintf("Failed finding %s with proper tags", prettyMetricQuery)) } else { sendEvent("success", "All good!") } }() suite.EventuallyWithTf(func(collect *assert.CollectT) { - myCollect := &myCollectT{ + c := &myCollectT{ CollectT: collect, errors: []error{}, } - // To enforce the use of myCollect instead + // To enforce the use of myCollectT instead collect = nil //nolint:ineffassign defer func() { - if len(myCollect.errors) == 0 { + if len(c.errors) == 0 { sendEvent("success", "All good!") } else { - sendEvent("warning", errors.Join(myCollect.errors...).Error()) + sendEvent("warning", errors.Join(c.errors...).Error()) } }() @@ -153,37 +155,35 @@ func (suite *baseSuite) testMetric(args *testMetricArgs) { args.Filter.Name, fakeintake.WithTags[*aggregator.MetricSeries](args.Filter.Tags), ) - if err != nil { - myCollect.Errorf("%w", err) + // Can be replaced by require.NoErrorf(…) once https://github.com/stretchr/testify/pull/1481 is merged + if !assert.NoErrorf(c, err, "Failed to query fake intake") { return } - if len(metrics) == 0 { - myCollect.Errorf("No `%s{%s}` metrics yet", args.Filter.Name, strings.Join(args.Filter.Tags, ",")) + // Can be replaced by require.NoEmptyf(…) once https://github.com/stretchr/testify/pull/1481 is merged + if !assert.NotEmptyf(c, metrics, "No `%s` metrics yet", prettyMetricQuery) { return } // Check tags if expectedTags != nil { - if err := assertTags(metrics[len(metrics)-1].GetTags(), expectedTags); err != nil { - myCollect.Errorf("Tags mismatch on `%s`: %w", args.Filter.Name, err) - } + err := assertTags(metrics[len(metrics)-1].GetTags(), expectedTags) + assert.NoErrorf(c, err, "Tags mismatch on `%s`", prettyMetricQuery) } // Check value if args.Expect.Value != nil { - if lo.CountBy(metrics[len(metrics)-1].GetPoints(), func(v *gogen.MetricPayload_MetricPoint) bool { + assert.NotEmptyf(c, lo.Filter(metrics[len(metrics)-1].GetPoints(), func(v *gogen.MetricPayload_MetricPoint, _ int) bool { return v.GetValue() >= args.Expect.Value.Min && v.GetValue() <= args.Expect.Value.Max - }) == 0 { - myCollect.Errorf( - "No value of `%s{%s}` is in the range [%f;%f]", - args.Filter.Name, - strings.Join(args.Filter.Tags, ","), - args.Expect.Value.Min, - args.Expect.Value.Max, - ) - } + }), "No value of `%s` is in the range [%f;%f]: %v", + prettyMetricQuery, + args.Expect.Value.Min, + args.Expect.Value.Max, + lo.Map(metrics[len(metrics)-1].GetPoints(), func(v *gogen.MetricPayload_MetricPoint, _ int) float64 { + return v.GetValue() + }), + ) } - }, 2*time.Minute, 10*time.Second, "Failed finding %s{%s} with proper tags", args.Filter.Name, strings.Join(args.Filter.Tags, ",")) + }, 2*time.Minute, 10*time.Second, "Failed finding `%s` with proper tags and value", prettyMetricQuery) }) } diff --git a/test/new-e2e/tests/containers/ecs_test.go b/test/new-e2e/tests/containers/ecs_test.go index e48e1cb6a8c94..94a64366c6fd7 100644 --- a/test/new-e2e/tests/containers/ecs_test.go +++ b/test/new-e2e/tests/containers/ecs_test.go @@ -98,7 +98,7 @@ func (suite *ecsSuite) Test00UpAndRunning() { client := awsecs.NewFromConfig(cfg) suite.Run("ECS tasks are ready", func() { - suite.EventuallyWithTf(func(collect *assert.CollectT) { + suite.EventuallyWithTf(func(c *assert.CollectT) { var initToken string for nextToken := &initToken; nextToken != nil; { if nextToken == &initToken { @@ -110,8 +110,8 @@ func (suite *ecsSuite) Test00UpAndRunning() { MaxResults: pointer.Ptr(int32(10)), // Because `DescribeServices` takes at most 10 services in input NextToken: nextToken, }) - if err != nil { - collect.Errorf("Failed to list ECS services: %w", err) + // Can be replaced by require.NoErrorf(…) once https://github.com/stretchr/testify/pull/1481 is merged + if !assert.NoErrorf(c, err, "Failed to list ECS services") { return } @@ -121,15 +121,12 @@ func (suite *ecsSuite) Test00UpAndRunning() { Cluster: &suite.ecsClusterName, Services: servicesList.ServiceArns, }) - if err != nil { - collect.Errorf("Failed to describe ECS services %v: %w", servicesList.ServiceArns, err) + if !assert.NoErrorf(c, err, "Failed to describe ECS services %v", servicesList.ServiceArns) { continue } for _, serviceDescription := range servicesDescription.Services { - if serviceDescription.DesiredCount == 0 { - collect.Errorf("ECS service %s has no task.", *serviceDescription.ServiceName) - } + assert.NotZerof(c, serviceDescription.DesiredCount, "ECS service %s has no task", *serviceDescription.ServiceName) for nextToken := &initToken; nextToken != nil; { if nextToken == &initToken { @@ -143,8 +140,7 @@ func (suite *ecsSuite) Test00UpAndRunning() { MaxResults: pointer.Ptr(int32(100)), // Because `DescribeTasks` takes at most 100 tasks in input NextToken: nextToken, }) - if err != nil { - collect.Errorf("Failed to list ECS tasks for service %s: %w", *serviceDescription.ServiceName, err) + if !assert.NoErrorf(c, err, "Failed to list ECS tasks for service %s", *serviceDescription.ServiceName) { break } @@ -154,20 +150,15 @@ func (suite *ecsSuite) Test00UpAndRunning() { Cluster: &suite.ecsClusterName, Tasks: tasksList.TaskArns, }) - if err != nil { - collect.Errorf("Failed to describe ECS tasks %v: %w", tasksList.TaskArns, err) + if !assert.NoErrorf(c, err, "Failed to describe ECS tasks %v", tasksList.TaskArns) { continue } for _, taskDescription := range tasksDescription.Tasks { - if *taskDescription.LastStatus != string(awsecstypes.DesiredStatusRunning) || - taskDescription.HealthStatus == awsecstypes.HealthStatusUnhealthy { - collect.Errorf("Task %s of service %s is %s %s.", - *taskDescription.TaskArn, - *serviceDescription.ServiceName, - *taskDescription.LastStatus, - taskDescription.HealthStatus) - } + assert.Equalf(c, string(awsecstypes.DesiredStatusRunning), *taskDescription.LastStatus, + "Task %s of service %s is not running", *taskDescription.TaskArn, *serviceDescription.ServiceName) + assert.NotEqualf(c, awsecstypes.HealthStatusUnhealthy, taskDescription.HealthStatus, + "Task %s of service %s is unhealthy", *taskDescription.TaskArn, *serviceDescription.ServiceName) } } } diff --git a/test/new-e2e/tests/containers/k8s_test.go b/test/new-e2e/tests/containers/k8s_test.go index 7466e3996ecde..bbb7d1693d691 100644 --- a/test/new-e2e/tests/containers/k8s_test.go +++ b/test/new-e2e/tests/containers/k8s_test.go @@ -84,89 +84,75 @@ func (suite *k8sSuite) Test00UpAndRunning() { ctx := context.Background() suite.Run("agent pods are ready and not restarting", func() { - suite.EventuallyWithTf(func(collect *assert.CollectT) { + suite.EventuallyWithTf(func(c *assert.CollectT) { linuxNodes, err := suite.K8sClient.CoreV1().Nodes().List(ctx, metav1.ListOptions{ LabelSelector: fields.OneTermEqualSelector("kubernetes.io/os", "linux").String(), }) - if err != nil { - collect.Errorf("Failed to list Linux nodes: %w", err) + // Can be replaced by require.NoErrorf(…) once https://github.com/stretchr/testify/pull/1481 is merged + if !assert.NoErrorf(c, err, "Failed to list Linux nodes") { return } windowsNodes, err := suite.K8sClient.CoreV1().Nodes().List(ctx, metav1.ListOptions{ LabelSelector: fields.OneTermEqualSelector("kubernetes.io/os", "windows").String(), }) - if err != nil { - collect.Errorf("Failed to list Windows nodes: %w", err) + // Can be replaced by require.NoErrorf(…) once https://github.com/stretchr/testify/pull/1481 is merged + if !assert.NoErrorf(c, err, "Failed to list Windows nodes") { return } linuxPods, err := suite.K8sClient.CoreV1().Pods("datadog").List(ctx, metav1.ListOptions{ LabelSelector: fields.OneTermEqualSelector("app", suite.AgentLinuxHelmInstallName+"-datadog").String(), }) - if err != nil { - collect.Errorf("Failed to list Linux datadog agent pods: %w", err) + // Can be replaced by require.NoErrorf(…) once https://github.com/stretchr/testify/pull/1481 is merged + if !assert.NoErrorf(c, err, "Failed to list Linux datadog agent pods") { return } windowsPods, err := suite.K8sClient.CoreV1().Pods("datadog").List(ctx, metav1.ListOptions{ LabelSelector: fields.OneTermEqualSelector("app", suite.AgentWindowsHelmInstallName+"-datadog").String(), }) - if err != nil { - collect.Errorf("Failed to list Windows datadog agent pods: %w", err) + // Can be replaced by require.NoErrorf(…) once https://github.com/stretchr/testify/pull/1481 is merged + if !assert.NoErrorf(c, err, "Failed to list Windows datadog agent pods") { return } clusterAgentPods, err := suite.K8sClient.CoreV1().Pods("datadog").List(ctx, metav1.ListOptions{ LabelSelector: fields.OneTermEqualSelector("app", suite.AgentLinuxHelmInstallName+"-datadog-cluster-agent").String(), }) - if err != nil { - collect.Errorf("Failed to list datadog cluster agent pods: %w", err) + // Can be replaced by require.NoErrorf(…) once https://github.com/stretchr/testify/pull/1481 is merged + if !assert.NoErrorf(c, err, "Failed to list datadog cluster agent pods") { return } clusterChecksPods, err := suite.K8sClient.CoreV1().Pods("datadog").List(ctx, metav1.ListOptions{ LabelSelector: fields.OneTermEqualSelector("app", suite.AgentLinuxHelmInstallName+"-datadog-clusterchecks").String(), }) - if err != nil { - collect.Errorf("Failed to list datadog cluster checks runner pods: %w", err) + // Can be replaced by require.NoErrorf(…) once https://github.com/stretchr/testify/pull/1481 is merged + if !assert.NoErrorf(c, err, "Failed to list datadog cluster checks runner pods") { return } - dogstatsdStandalonePods, err := suite.K8sClient.CoreV1().Pods("dogstatsd-standalone").List(ctx, metav1.ListOptions{ + dogstatsdPods, err := suite.K8sClient.CoreV1().Pods("dogstatsd-standalone").List(ctx, metav1.ListOptions{ LabelSelector: fields.OneTermEqualSelector("app", "dogstatsd-standalone").String(), }) - if err != nil { - collect.Errorf("Failed to list dogstatsd standalone pods: %w", err) + // Can be replaced by require.NoErrorf(…) once https://github.com/stretchr/testify/pull/1481 is merged + if !assert.NoErrorf(c, err, "Failed to list dogstatsd standalone pods") { return } - if len(linuxPods.Items) != len(linuxNodes.Items) { - collect.Errorf("There is only %d Linux datadog agent pods for %d Linux nodes.", len(linuxPods.Items), len(linuxNodes.Items)) - } - if len(windowsPods.Items) != len(windowsNodes.Items) { - collect.Errorf("There is only %d Windows datadog agent pods for %d Windows nodes.", len(windowsPods.Items), len(windowsNodes.Items)) - } - if len(clusterAgentPods.Items) == 0 { - collect.Errorf("There isn’t any cluster agent pod.") - } - if len(clusterChecksPods.Items) == 0 { - collect.Errorf("There isn’t any cluster checks worker pod.") - } - if len(dogstatsdStandalonePods.Items) != len(linuxNodes.Items) { - collect.Errorf("There is only %d dogstatsd standalone pods for %d Linux nodes.", len(dogstatsdStandalonePods.Items), len(linuxNodes.Items)) - } + assert.Len(c, linuxPods.Items, len(linuxNodes.Items)) + assert.Len(c, windowsPods.Items, len(windowsNodes.Items)) + assert.NotEmpty(c, clusterAgentPods.Items) + assert.NotEmpty(c, clusterChecksPods.Items) + assert.Len(c, dogstatsdPods.Items, len(linuxNodes.Items)) - for _, podList := range []*corev1.PodList{linuxPods, windowsPods, clusterAgentPods, clusterChecksPods} { + for _, podList := range []*corev1.PodList{linuxPods, windowsPods, clusterAgentPods, clusterChecksPods, dogstatsdPods} { for _, pod := range podList.Items { for _, containerStatus := range append(pod.Status.InitContainerStatuses, pod.Status.ContainerStatuses...) { - if !containerStatus.Ready { - collect.Errorf("Container %s of pod %s isn’t ready.", containerStatus.Name, pod.Name) - } - if containerStatus.RestartCount > 0 { - collect.Errorf("Container %s of pod %s has restarted %d times.", containerStatus.Name, pod.Name, containerStatus.RestartCount) - } + assert.Truef(c, containerStatus.Ready, "Container %s of pod %s isn’t ready", containerStatus.Name, pod.Name) + assert.Zerof(c, containerStatus.RestartCount, "Container %s of pod %s has restarted", containerStatus.Name, pod.Name) } } } @@ -209,7 +195,7 @@ func (suite *k8sSuite) Test00UpAndRunning() { if suite.NoError(err) && len(linuxPods.Items) >= 1 { stdout, stderr, err := suite.podExec("datadog", linuxPods.Items[0].Name, tt.container, []string{"agent", "version"}) if suite.NoError(err) { - suite.Equalf(stderr, "", "Standard error of `agent version` should be empty,") + suite.Emptyf(stderr, "Standard error of `agent version` should be empty,") match := versionExtractor.FindStringSubmatch(stdout) if suite.Equalf(2, len(match), "'Commit' not found in the output of `agent version`.") { if len(GitCommit) == 10 && len(match[1]) == 7 { @@ -289,6 +275,10 @@ func (suite *k8sSuite) TestNginx() { `^kube_deployment:nginx$`, `^kube_namespace:workload-nginx$`, }, + Value: &testMetricExpectValueArgs{ + Max: 5, + Min: 1, + }, }, }) @@ -345,6 +335,10 @@ func (suite *k8sSuite) TestRedis() { `^kube_deployment:redis$`, `^kube_namespace:workload-redis$`, }, + Value: &testMetricExpectValueArgs{ + Max: 5, + Min: 1, + }, }, }) @@ -638,7 +632,7 @@ func (suite *k8sSuite) testHPA(namespace, deployment string) { } }() - suite.EventuallyWithTf(func(collect *assert.CollectT) { + suite.EventuallyWithTf(func(c *assert.CollectT) { metrics, err := suite.Fakeintake.FilterMetrics( "kubernetes_state.deployment.replicas_available", fakeintake.WithTags[*aggregator.MetricSeries]([]string{ @@ -646,13 +640,12 @@ func (suite *k8sSuite) testHPA(namespace, deployment string) { "kube_deployment:" + deployment, }), ) - if err != nil { - collect.Errorf("%w", err) + // Can be replaced by require.NoErrorf(…) once https://github.com/stretchr/testify/pull/1481 is merged + if !assert.NoErrorf(c, err, "Failed to query fake intake") { return } - if len(metrics) == 0 { - collect.Errorf("No `kubernetes_state.deployment.replicas_available{kube_namespace:%s,kube_deployment:%s}` metrics yet", namespace, deployment) - sendEvent("error", fmt.Sprintf("No `kubernetes_state.deployment.replicas_available{kube_namespace:%s,kube_deployment:%s}` metrics yet", namespace, deployment), nil) + if !assert.NotEmptyf(c, metrics, "No `kubernetes_state.deployment.replicas_available{kube_namespace:%s,kube_deployment:%s}` metrics yet", namespace, deployment) { + sendEvent("warning", fmt.Sprintf("No `kubernetes_state.deployment.replicas_available{kube_namespace:%s,kube_deployment:%s}` metrics yet", namespace, deployment), nil) return } @@ -669,13 +662,13 @@ func (suite *k8sSuite) testHPA(namespace, deployment string) { continue } - if point.Value > prevValue+0.5 { + if !scaleUp && point.Value > prevValue+0.5 { scaleUp = true sendEvent("success", "Scale up detected.", pointer.Ptr(int(point.Timestamp))) if scaleDown { break out } - } else if point.Value < prevValue-0.5 { + } else if !scaleDown && point.Value < prevValue-0.5 { scaleDown = true sendEvent("success", "Scale down detected.", pointer.Ptr(int(point.Timestamp))) if scaleUp { @@ -685,12 +678,8 @@ func (suite *k8sSuite) testHPA(namespace, deployment string) { prevValue = point.Value } } - if !scaleUp { - collect.Errorf("No scale up detected") - } - if !scaleDown { - collect.Errorf("No scale down detected") - } + assert.Truef(c, scaleUp, "No scale up detected") + assert.Truef(c, scaleDown, "No scale down detected") }, 20*time.Minute, 10*time.Second, "Failed to witness scale up and scale down of %s.%s", namespace, deployment) }) }