test: reduce duplicate literals in helper tests

Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>

e2e/makefile_test.go

@@ -14,7 +14,6 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-
 package e2e
 
 import (
@@ -24,15 +23,23 @@ import (
 	"testing"
 )
 
+const (
+	testVersionArg           = "VERSION=test-version"
+	kubernetesBuildTarget    = "docker.build.provider.kubernetes"
+	kubernetesProviderImage  = "ghcr.io/external-secrets/provider-kubernetes:test-version"
+	helmDependencyEnsureCmd  = "../hack/helm.dependency.ensure.sh ../deploy/charts/external-secrets"
+	controllerImageLoadCount = `kind load docker-image --name="external-secrets" ghcr.io/external-secrets/external-secrets:test-version`
+)
+
 func TestClassicMakeTargetBuildsOnlyControllerImageOnce(t *testing.T) {
 	t.Parallel()
 
-	dryRun := runMakeDryRun(t, "test", "VERSION=test-version", `TEST_SUITES=provider`, `GINKGO_LABELS=kubernetes && !v2`)
+	dryRun := runMakeDryRun(t, "test", testVersionArg, `TEST_SUITES=provider`, `GINKGO_LABELS=kubernetes && !v2`)
 
 	if !strings.Contains(dryRun, "docker.build.controller.e2e") {
 		t.Fatalf("expected classic test dry-run to build the controller image via docker.build.controller.e2e, output:\n%s", dryRun)
 	}
-	if strings.Contains(dryRun, "docker.build.provider.kubernetes") {
+	if strings.Contains(dryRun, kubernetesBuildTarget) {
 		t.Fatalf("expected classic test dry-run to omit kubernetes provider image builds, output:\n%s", dryRun)
 	}
 	if strings.Contains(dryRun, "docker.build.provider.aws") {
@@ -41,13 +48,13 @@ func TestClassicMakeTargetBuildsOnlyControllerImageOnce(t *testing.T) {
 	if strings.Contains(dryRun, "docker.build.provider.fake") {
 		t.Fatalf("expected classic test dry-run to omit fake provider image builds, output:\n%s", dryRun)
 	}
-	if count := strings.Count(dryRun, `kind load docker-image --name="external-secrets" ghcr.io/external-secrets/external-secrets:test-version`); count != 1 {
+	if count := strings.Count(dryRun, controllerImageLoadCount); count != 1 {
 		t.Fatalf("expected classic test dry-run to load the controller image once, got %d occurrences, output:\n%s", count, dryRun)
 	}
-	if strings.Contains(dryRun, "ghcr.io/external-secrets/provider-kubernetes:test-version") {
+	if strings.Contains(dryRun, kubernetesProviderImage) {
 		t.Fatalf("expected classic test dry-run to avoid loading the kubernetes provider image, output:\n%s", dryRun)
 	}
-	if !strings.Contains(dryRun, "../hack/helm.dependency.ensure.sh ../deploy/charts/external-secrets") {
+	if !strings.Contains(dryRun, helmDependencyEnsureCmd) {
 		t.Fatalf("expected classic test dry-run to ensure helm dependencies before copying the chart, output:\n%s", dryRun)
 	}
 }
@@ -55,25 +62,25 @@ func TestClassicMakeTargetBuildsOnlyControllerImageOnce(t *testing.T) {
 func TestV2MakeTargetCanSkipKubernetesProviderBuild(t *testing.T) {
 	t.Parallel()
 
-	defaultDryRun := runMakeDryRun(t, "test.v2", "VERSION=test-version")
-	if !strings.Contains(defaultDryRun, "docker.build.provider.kubernetes") {
+	defaultDryRun := runMakeDryRun(t, "test.v2", testVersionArg)
+	if !strings.Contains(defaultDryRun, kubernetesBuildTarget) {
 		t.Fatalf("expected default test.v2 dry-run to build the kubernetes provider image, output:\n%s", defaultDryRun)
 	}
-	if !strings.Contains(defaultDryRun, "ghcr.io/external-secrets/provider-kubernetes:test-version") {
+	if !strings.Contains(defaultDryRun, kubernetesProviderImage) {
 		t.Fatalf("expected default test.v2 dry-run to still load the kubernetes provider image, output:\n%s", defaultDryRun)
 	}
-	if !strings.Contains(defaultDryRun, "../hack/helm.dependency.ensure.sh ../deploy/charts/external-secrets") {
+	if !strings.Contains(defaultDryRun, helmDependencyEnsureCmd) {
 		t.Fatalf("expected default test.v2 dry-run to ensure helm dependencies before copying the chart, output:\n%s", defaultDryRun)
 	}
 
-	skippedDryRun := runMakeDryRun(t, "test.v2", "VERSION=test-version", "SKIP_PROVIDER_KUBERNETES_BUILD=true")
-	if strings.Contains(skippedDryRun, "docker.build.provider.kubernetes") {
+	skippedDryRun := runMakeDryRun(t, "test.v2", testVersionArg, "SKIP_PROVIDER_KUBERNETES_BUILD=true")
+	if strings.Contains(skippedDryRun, kubernetesBuildTarget) {
 		t.Fatalf("expected skipped test.v2 dry-run to omit the kubernetes provider build, output:\n%s", skippedDryRun)
 	}
-	if !strings.Contains(skippedDryRun, "ghcr.io/external-secrets/provider-kubernetes:test-version") {
+	if !strings.Contains(skippedDryRun, kubernetesProviderImage) {
 		t.Fatalf("expected skipped test.v2 dry-run to still load the kubernetes provider image, output:\n%s", skippedDryRun)
 	}
-	if !strings.Contains(skippedDryRun, "../hack/helm.dependency.ensure.sh ../deploy/charts/external-secrets") {
+	if !strings.Contains(skippedDryRun, helmDependencyEnsureCmd) {
 		t.Fatalf("expected skipped test.v2 dry-run to ensure helm dependencies before copying the chart, output:\n%s", skippedDryRun)
 	}
 }

hack/helm_dependency_ensure_test.go

@@ -24,63 +24,54 @@ import (
 	"testing"
 )
 
+const (
+	buildMarkerFile  = "build-called"
+	helperScriptPath = "helm.dependency.ensure.sh"
+	chartPath        = "deploy/charts/external-secrets"
+	helmBinEnvPrefix = "HELM_BIN="
+	helperFailedMsg  = "helper failed: %v\n%s"
+	scriptPreamble   = "#!/usr/bin/env bash\nset -euo pipefail\n\n"
+	scriptFailure    = "echo \"unexpected helm invocation: $*\" >&2\nexit 1\n"
+)
+
 func TestHelmDependencyEnsureSkipsBuildWhenDependenciesAreReady(t *testing.T) {
 	t.Parallel()
 
-	workdir := t.TempDir()
-	marker := filepath.Join(workdir, "build-called")
-	fakeHelm := writeFakeHelm(t, workdir, fakeHelmScript("v0.5.2", "ok", marker))
-
-	cmd := exec.Command("bash", "helm.dependency.ensure.sh", "deploy/charts/external-secrets")
-	cmd.Dir = "."
-	cmd.Env = append(os.Environ(), "HELM_BIN="+fakeHelm)
-	output, err := cmd.CombinedOutput()
-	if err != nil {
-		t.Fatalf("helper failed: %v\n%s", err, string(output))
-	}
-
-	if _, err := os.Stat(marker); !os.IsNotExist(err) {
-		t.Fatalf("expected helper to skip helm dependency build when dependencies are already available")
-	}
+	assertHelperBuildBehavior(t, "v0.5.2", "ok", false, "expected helper to skip helm dependency build when dependencies are already available")
 }
 
 func TestHelmDependencyEnsureSkipsBuildWhenDependenciesAreUnpacked(t *testing.T) {
 	t.Parallel()
 
-	workdir := t.TempDir()
-	marker := filepath.Join(workdir, "build-called")
-	fakeHelm := writeFakeHelm(t, workdir, fakeHelmScript("v0.6.0", "unpacked", marker))
-
-	cmd := exec.Command("bash", "helm.dependency.ensure.sh", "deploy/charts/external-secrets")
-	cmd.Dir = "."
-	cmd.Env = append(os.Environ(), "HELM_BIN="+fakeHelm)
-	output, err := cmd.CombinedOutput()
-	if err != nil {
-		t.Fatalf("helper failed: %v\n%s", err, string(output))
-	}
-
-	if _, err := os.Stat(marker); !os.IsNotExist(err) {
-		t.Fatalf("expected helper to skip helm dependency build when dependencies are unpacked locally")
-	}
+	assertHelperBuildBehavior(t, "v0.6.0", "unpacked", false, "expected helper to skip helm dependency build when dependencies are unpacked locally")
 }
 
 func TestHelmDependencyEnsureBuildsWhenDependenciesAreMissing(t *testing.T) {
 	t.Parallel()
 
-	workdir := t.TempDir()
-	marker := filepath.Join(workdir, "build-called")
-	fakeHelm := writeFakeHelm(t, workdir, fakeHelmScript("v0.5.2", "missing", marker))
+	assertHelperBuildBehavior(t, "v0.5.2", "missing", true, "expected helper to invoke helm dependency build when dependencies are missing")
+}
 
-	cmd := exec.Command("bash", "helm.dependency.ensure.sh", "deploy/charts/external-secrets")
-	cmd.Dir = "."
-	cmd.Env = append(os.Environ(), "HELM_BIN="+fakeHelm)
-	output, err := cmd.CombinedOutput()
+func assertHelperBuildBehavior(t *testing.T, version, status string, expectBuild bool, failureMsg string) {
+	t.Helper()
+
+	workdir := t.TempDir()
+	marker := filepath.Join(workdir, buildMarkerFile)
+	fakeHelm := writeFakeHelm(t, workdir, fakeHelmScript(version, status, marker))
+	output, err := runHelper(t, fakeHelm)
 	if err != nil {
-		t.Fatalf("helper failed: %v\n%s", err, string(output))
+		t.Fatalf(helperFailedMsg, err, string(output))
 	}
 
-	if _, err := os.Stat(marker); err != nil {
-		t.Fatalf("expected helper to invoke helm dependency build when dependencies are missing: %v", err)
+	if expectBuild {
+		if _, err := os.Stat(marker); err != nil {
+			t.Fatalf("%s: %v", failureMsg, err)
+		}
+		return
+	}
+
+	if _, err := os.Stat(marker); !os.IsNotExist(err) {
+		t.Fatal(failureMsg)
 	}
 }
 
@@ -95,11 +86,22 @@ func writeFakeHelm(t *testing.T, dir, content string) string {
 	return path
 }
 
-func fakeHelmScript(version, status, marker string) string {
-	return fmt.Sprintf(`#!/usr/bin/env bash
-set -euo pipefail
+func runHelper(t *testing.T, fakeHelm string) ([]byte, error) {
+	t.Helper()
+
+	cmd := exec.Command("bash", helperScriptPath, chartPath)
+	cmd.Dir = "."
+	cmd.Env = append(os.Environ(), helmBinEnvPrefix+fakeHelm)
+
+	return cmd.CombinedOutput()
+}
 
-if [[ "$1" == "dependency" && "$2" == "list" ]]; then
+func fakeHelmScript(version, status, marker string) string {
+	return fmt.Sprintf(
+		"%s%s%s",
+		scriptPreamble,
+		fmt.Sprintf(
+			`if [[ "$1" == "dependency" && "$2" == "list" ]]; then
   cat <<'EOF'
 NAME                	VERSION	REPOSITORY                           	STATUS
 bitwarden-sdk-server	%s	oci://ghcr.io/external-secrets/charts	%s
@@ -112,7 +114,11 @@ if [[ "$1" == "dependency" && "$2" == "build" ]]; then
   exit 0
 fi
 
-echo "unexpected helm invocation: $*" >&2
-exit 1
-`, version, status, marker)
+`,
+			version,
+			status,
+			marker,
+		),
+		scriptFailure,
+	)
 }