From dc8e7a2f39b599c0cd081723241099351361e2c2 Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Fri, 2 Oct 2020 22:40:23 -0400 Subject: [PATCH 1/2] Enable cache mutation detector in unit tests Signed-off-by: Monis Khan --- hack/module.sh | 12 +++++++++-- internal/controller/controller_test.go | 29 ++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 internal/controller/controller_test.go diff --git a/hack/module.sh b/hack/module.sh index 665b9a34..1d3005d4 100755 --- a/hack/module.sh +++ b/hack/module.sh @@ -42,13 +42,21 @@ function with_modules() { local cmd_function="${1}" cmd="$(${cmd_function})" + # start the cache mutation detector by default so that cache mutators will be found + local kube_cache_mutation_detector="${KUBE_CACHE_MUTATION_DETECTOR:-true}" + + # panic the server on watch decode errors since they are considered coder mistakes + local kube_panic_watch_decode_error="${KUBE_PANIC_WATCH_DECODE_ERROR:-true}" + + env_vars="KUBE_CACHE_MUTATION_DETECTOR=${kube_cache_mutation_detector} KUBE_PANIC_WATCH_DECODE_ERROR=${kube_panic_watch_decode_error}" + pushd "${ROOT}" >/dev/null for mod_file in $(find . -maxdepth 4 -not -path "./generated/*" -name go.mod | sort); do mod_dir="$(dirname "${mod_file}")" ( echo "=> " - echo " cd ${mod_dir} && ${cmd}" - cd "${mod_dir}" && ${cmd} + echo " cd ${mod_dir} && ${env_vars} ${cmd}" + cd "${mod_dir}" && env ${env_vars} ${cmd} ) done popd >/dev/null diff --git a/internal/controller/controller_test.go b/internal/controller/controller_test.go new file mode 100644 index 00000000..0cf97e21 --- /dev/null +++ b/internal/controller/controller_test.go @@ -0,0 +1,29 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "testing" + + "github.com/stretchr/testify/require" + "k8s.io/client-go/tools/cache" +) + +func TestCacheMutationDetectorEnabled(t *testing.T) { + // this is a bit of simplistic test to check if we have a real cache mutation detector. + // if we actually start mutating an informer cache in this test, the test will almost + // always fail because the go race detector will see the mutation. + // the cache mutation detector will certainly make certain races more common and thus + // easily detected by the race detector, but its real use is against a compiled binary + // such as pinniped-server running in a pod - that binary has no race detector at runtime. + + c := cache.NewCacheMutationDetector("test pinniped") + + type isRealCacheMutationDetector interface { + CompareObjects() // this is brittle, but this function name has never changed... + } + + _, ok := c.(isRealCacheMutationDetector) + require.Truef(t, ok, "%T is not a real cache mutation detector", c) +} From b60542f0d13ae9ffa1a53512f9ee3d39182a4d3c Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Wed, 16 Dec 2020 21:09:02 -0600 Subject: [PATCH 2/2] Clean this test up a trivial amount using `require.Implementsf()`. Signed-off-by: Matt Moyer --- internal/controller/controller_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/internal/controller/controller_test.go b/internal/controller/controller_test.go index 0cf97e21..bfb35495 100644 --- a/internal/controller/controller_test.go +++ b/internal/controller/controller_test.go @@ -20,10 +20,8 @@ func TestCacheMutationDetectorEnabled(t *testing.T) { c := cache.NewCacheMutationDetector("test pinniped") - type isRealCacheMutationDetector interface { + type realCacheMutationDetector interface { CompareObjects() // this is brittle, but this function name has never changed... } - - _, ok := c.(isRealCacheMutationDetector) - require.Truef(t, ok, "%T is not a real cache mutation detector", c) + require.Implementsf(t, (*realCacheMutationDetector)(nil), c, "%T is not a real cache mutation detector", c) }