Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit 72e1838

Browse files
authored
fix(appliance): bypass setupenv-test for local testing if not installed (#64305)
<!-- PR description tips: https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e --> Bypass the requirement for `sigs.k8s.io/controller-runtime/tools/setup-envtest` when running `go test` in the `internal/appliance` package. Previously running `go test` without `sigs.k8s.io/controller-runtime/tools/setup-envtest` installed would fail the tests that required it. This changes this behavior to instead skip the tests requiring `setup-envtest` if the user doesn't have it installed locally. ## Test plan Tested locally without `setup-envtest` installed: ```shell go test ./internal/appliance/reconciler... -v === RUN TestApplianceTestSuite helpers_test.go:37: setup-envtest is not installed, skipping ApplianceTestSuite --- SKIP: TestApplianceTestSuite (0.00s) PASS ok github.com/sourcegraph/sourcegraph/internal/appliance/reconciler 0.907s ``` Tested locally with `setup-envtest` installed: ```shell go test ./internal/appliance/reconciler... -v ... --- PASS: TestApplianceTestSuite/TestDeployWorker/worker/with-blobstore (0.25s) --- PASS: TestApplianceTestSuite/TestDeployWorker/worker/with-replicas (0.24s) --- PASS: TestApplianceTestSuite/TestDoesNotDeleteUnownedResources (0.22s) --- PASS: TestApplianceTestSuite/TestFrontendDeploymentRollsWhenPGSecretsChange (1.36s) --- PASS: TestApplianceTestSuite/TestFrontendDeploymentRollsWhenPGSecretsChange/pgsql-auth (0.47s) --- PASS: TestApplianceTestSuite/TestFrontendDeploymentRollsWhenPGSecretsChange/codeinsights-db-auth (0.44s) --- PASS: TestApplianceTestSuite/TestFrontendDeploymentRollsWhenPGSecretsChange/codeintel-db-auth (0.45s) --- PASS: TestApplianceTestSuite/TestFrontendDeploymentRollsWhenRedisSecretsChange (0.93s) --- PASS: TestApplianceTestSuite/TestFrontendDeploymentRollsWhenRedisSecretsChange/redis-cache (0.45s) --- PASS: TestApplianceTestSuite/TestFrontendDeploymentRollsWhenRedisSecretsChange/redis-store (0.48s) --- PASS: TestApplianceTestSuite/TestMergeK8sObjects (0.00s) --- PASS: TestApplianceTestSuite/TestMergeK8sObjects/Successful_merge (0.00s) --- PASS: TestApplianceTestSuite/TestMergeK8sObjects/Merge_with_overlapping_keys (0.00s) --- PASS: TestApplianceTestSuite/TestMergeK8sObjects/Merge_with_empty_new_object (0.00s) --- PASS: TestApplianceTestSuite/TestMergeK8sObjects/merges_annotations (0.00s) --- PASS: TestApplianceTestSuite/TestNonNamespacedResourcesRemainWhenDisabled (0.45s) --- PASS: TestApplianceTestSuite/TestResourcesDeletedWhenDisabled (0.45s) --- PASS: TestApplianceTestSuite/TestStandardFeatures (2.43s) --- PASS: TestApplianceTestSuite/TestStandardFeatures/standard/blobstore-with-named-storage-class (0.24s) --- PASS: TestApplianceTestSuite/TestStandardFeatures/standard/frontend-with-no-cpu-memory-resources (0.25s) --- PASS: TestApplianceTestSuite/TestStandardFeatures/standard/precise-code-intel-with-env-vars (0.24s) --- PASS: TestApplianceTestSuite/TestStandardFeatures/standard/redis-with-multiple-custom-images (0.25s) --- PASS: TestApplianceTestSuite/TestStandardFeatures/standard/redis-with-storage (0.25s) --- PASS: TestApplianceTestSuite/TestStandardFeatures/standard/repo-updater-with-no-resources (0.26s) --- PASS: TestApplianceTestSuite/TestStandardFeatures/standard/repo-updater-with-pod-template-config (0.23s) --- PASS: TestApplianceTestSuite/TestStandardFeatures/standard/repo-updater-with-resources (0.25s) --- PASS: TestApplianceTestSuite/TestStandardFeatures/standard/repo-updater-with-sa-annotations (0.24s) --- PASS: TestApplianceTestSuite/TestStandardFeatures/standard/symbols-with-custom-image (0.23s) PASS ok github.com/sourcegraph/sourcegraph/internal/appliance/reconciler 20.576s ``` <!-- REQUIRED; info at https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles --> ## Changelog N/A as this is not a user facing change. <!-- OPTIONAL; info at https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c -->
1 parent 991355b commit 72e1838

File tree

7 files changed

+36
-5
lines changed

7 files changed

+36
-5
lines changed

internal/appliance/healthchecker/health_checker_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/sourcegraph/log"
1717
"github.com/sourcegraph/log/logr"
1818
"github.com/sourcegraph/log/logtest"
19+
1920
"github.com/sourcegraph/sourcegraph/internal/appliance/k8senvtest"
2021
"github.com/sourcegraph/sourcegraph/internal/k8s/resource/service"
2122
)
@@ -31,6 +32,11 @@ func TestMain(m *testing.M) {
3132
ctx, cancel = context.WithCancel(context.Background())
3233
defer cancel()
3334

35+
if !k8senvtest.ShouldRunSetupEnvTests() {
36+
fmt.Println("setup-envtest is not installed or we are not in CI, skipping Appliance healthchecker tests")
37+
os.Exit(0)
38+
}
39+
3440
logger := log.Scoped("appliance-healthchecker-tests")
3541
k8sConfig, cleanup, err := k8senvtest.SetupEnvtest(ctx, logr.New(logger), k8senvtest.NewNoopReconciler)
3642
if err != nil {

internal/appliance/k8senvtest/envtest.go

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,27 @@ func kubebuilderAssetPathLocalDev() (string, error) {
121121
}
122122

123123
func NewNoopReconciler(mgr ctrl.Manager) KubernetesController {
124-
return noopReconicler{}
124+
return noopReconciler{}
125125
}
126126

127-
type noopReconicler struct{}
127+
type noopReconciler struct{}
128128

129-
func (noopReconicler) SetupWithManager(_ ctrl.Manager) error { return nil }
129+
func (noopReconciler) SetupWithManager(_ ctrl.Manager) error { return nil }
130+
131+
func isSetupEnvTestInstalled() bool {
132+
_, err := exec.LookPath("setup-envtest")
133+
return err == nil
134+
}
135+
136+
// ShouldRunSetupEnvTests determines if test requiring setup-envtest should be run or not.
137+
// Generally we want to require these tests to run in CI but not locally unless the developer
138+
// has "setup-envtest" present on their machine.
139+
func ShouldRunSetupEnvTests() bool {
140+
// Assume we are in CI, require tests to run.
141+
if os.Getenv("BAZEL_TEST") != "" {
142+
return true
143+
}
144+
145+
// fall back to whether or not requirments are installed
146+
return isSetupEnvTestInstalled()
147+
}

internal/appliance/reconciler/frontend_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func (suite *ApplianceTestSuite) TestFrontendDeploymentRollsWhenPGSecretsChange(
9999
suite.Run(tc.secret, func() {
100100
// Create the frontend before the PGSQL secret exists. In general, this
101101
// might happen, depending on the order of the reconcile loop. If we
102-
// introducce concurrency to this, we'll have little control over what
102+
// introduce concurrency to this, we'll have little control over what
103103
// happens first.
104104
namespace := suite.createConfigMapAndAwaitReconciliation("frontend/default")
105105

@@ -142,7 +142,7 @@ func (suite *ApplianceTestSuite) TestFrontendDeploymentRollsWhenRedisSecretsChan
142142
suite.Run(tc.secret, func() {
143143
// Create the frontend before the PGSQL secret exists. In general, this
144144
// might happen, depending on the order of the reconcile loop. If we
145-
// introducce concurrency to this, we'll have little control over what
145+
// introduce concurrency to this, we'll have little control over what
146146
// happens first.
147147
namespace := suite.createConfigMapAndAwaitReconciliation("frontend/default")
148148

internal/appliance/reconciler/helpers_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717

1818
"github.com/sourcegraph/log/logr"
1919
"github.com/sourcegraph/log/logtest"
20+
2021
"github.com/sourcegraph/sourcegraph/internal/appliance/config"
2122
"github.com/sourcegraph/sourcegraph/internal/appliance/k8senvtest"
2223
)
@@ -32,6 +33,9 @@ type ApplianceTestSuite struct {
3233
}
3334

3435
func TestApplianceTestSuite(t *testing.T) {
36+
if !k8senvtest.ShouldRunSetupEnvTests() {
37+
t.Skip("setup-envtest is not installed or we are not in CI, skipping ApplianceTestSuite")
38+
}
3539
suite.Run(t, new(ApplianceTestSuite))
3640
}
3741

internal/appliance/selfupdate/integrationtest/integration_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/sourcegraph/log"
2020
"github.com/sourcegraph/log/logr"
2121
"github.com/sourcegraph/log/logtest"
22+
2223
"github.com/sourcegraph/sourcegraph/internal/appliance/config"
2324
"github.com/sourcegraph/sourcegraph/internal/appliance/k8senvtest"
2425
"github.com/sourcegraph/sourcegraph/internal/appliance/selfupdate"

internal/appliance/selfupdate/selfupdate.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"sigs.k8s.io/controller-runtime/pkg/client"
1212

1313
"github.com/sourcegraph/log"
14+
1415
"github.com/sourcegraph/sourcegraph/internal/appliance"
1516
"github.com/sourcegraph/sourcegraph/internal/releaseregistry"
1617
"github.com/sourcegraph/sourcegraph/lib/errors"

internal/appliance/selfupdate/selfupdate_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/stretchr/testify/require"
88

99
"github.com/sourcegraph/log/logtest"
10+
1011
"github.com/sourcegraph/sourcegraph/internal/releaseregistry"
1112
"github.com/sourcegraph/sourcegraph/internal/releaseregistry/mocks"
1213
)

0 commit comments

Comments
 (0)