Fixes unit tests indeterminism (#6496)

We were getting sporadic coverage differences on `controller/k8s/test_helper.go` and `pkg/healthcheck/healthcheck_test.go` on pushes unrelated to those files.

For the former, the problem was in tests in `controller/k8s/api_test.go` that compared slices of pods and services by sorting them. The `Sort` interface was implemented through the methods in `test_helper.go`. There is indeterminism in that sorting at the go library level apparently, in that the `Swap` method is not always called, which impacted the coverage report. The fix consists on comparing those slices item by item without needing to sort beforehand.

As for `healthcheck_test.go`, `validateControlPlanePods()` in `healthcheck.go` short-circuits on the first pod having all its containers ready. The unit tests iterate over maps, an iteration we know is not deterministic, so sometimes the short-circuiting avoided to ever cover the `!container.Ready` block, thus affecting the coverage report. This is fixed by adding a new small test that makes sure that block is covered.
This commit is contained in:
Alejandro Pedraza 2021-07-19 02:12:45 -05:00 committed by GitHub
parent c6fcddfad1
commit ae62d92f7d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 40 additions and 20 deletions

View File

@ -5,7 +5,6 @@ import (
"errors"
"fmt"
"reflect"
"sort"
"testing"
"k8s.io/apimachinery/pkg/labels"
@ -942,11 +941,22 @@ status:
t.Fatalf("api.GetPodsFor() unexpected error, expected [%s] got: [%s]", exp.err, err)
}
sort.Sort(byPod(pods))
sort.Sort(byPod(k8sResultPods))
if !reflect.DeepEqual(pods, k8sResultPods) {
if len(pods) != len(k8sResultPods) {
t.Fatalf("Expected: %+v, Got: %+v", k8sResultPods, pods)
}
for _, pod := range pods {
found := false
for _, resultPod := range k8sResultPods {
if reflect.DeepEqual(pod, resultPod) {
found = true
break
}
}
if !found {
t.Fatalf("Expected: %+v, Got: %+v", k8sResultPods, pods)
}
}
}
})
}
@ -1348,11 +1358,22 @@ spec:
t.Fatalf("api.GetServicesFor() unexpected error, expected [%s] got: [%s]", exp.err, err)
}
sort.Sort(byService(k8sResultServices))
sort.Sort(byService(services))
if !reflect.DeepEqual(services, k8sResultServices) {
if len(services) != len(k8sResultServices) {
t.Fatalf("Expected: %+v, Got: %+v", k8sResultServices, services)
}
for _, service := range services {
found := false
for _, resultService := range k8sResultServices {
if reflect.DeepEqual(service, resultService) {
found = true
break
}
}
if !found {
t.Fatalf("Expected: %+v, Got: %+v", k8sResultServices, services)
}
}
}
})

View File

@ -2,7 +2,6 @@ package k8s
import (
"github.com/linkerd/linkerd2/pkg/k8s"
corev1 "k8s.io/api/core/v1"
)
// NewFakeAPI provides a mock Kubernetes API for testing.
@ -35,15 +34,3 @@ func NewFakeAPI(configs ...string) (*API, error) {
ES,
), nil
}
type byPod []*corev1.Pod
func (bp byPod) Len() int { return len(bp) }
func (bp byPod) Swap(i, j int) { bp[i], bp[j] = bp[j], bp[i] }
func (bp byPod) Less(i, j int) bool { return bp[i].Name <= bp[j].Name }
type byService []*corev1.Service
func (bs byService) Len() int { return len(bs) }
func (bs byService) Swap(i, j int) { bs[i], bs[j] = bs[j], bs[i] }
func (bs byService) Less(i, j int) bool { return bs[i].Name <= bs[j].Name }

View File

@ -1462,6 +1462,18 @@ func TestValidateControlPlanePods(t *testing.T) {
}
})
// This test is just for ensuring full coverage of the validateControlPlanePods function
t.Run("Returns an error if the only pod is not ready", func(t *testing.T) {
pods := []corev1.Pod{
pod("linkerd-proxy-injector-5f79ff4844-", corev1.PodRunning, false),
}
err := validateControlPlanePods(pods)
if err == nil {
t.Fatal("Expected error, got nothing")
}
})
t.Run("Returns nil if, HA mode, at least one pod of each control plane component is ready", func(t *testing.T) {
pods := []corev1.Pod{
pod("linkerd-destination-9843948665-48082", corev1.PodRunning, true),