From e02918cbe8889c1297f14dac5ebc868346acb867 Mon Sep 17 00:00:00 2001 From: Mykola Date: Mon, 24 Aug 2026 17:23:00 +0300 Subject: [PATCH 1/2] Isolate controller test storage Create a dedicated artifact server and storage directory for each reconciler test. Keep manager-owned reconcilers on their own storage and copy Helm fixtures into isolated cache tests. Fixes #1099 Signed-off-by: Mykola Assisted-by: Codex/GPT-5 --- internal/controller/bucket_controller_test.go | 10 ++++ .../gitrepository_controller_test.go | 16 ++++++ .../controller/helmchart_controller_test.go | 49 ++++++++++++------- .../helmrepository_controller_test.go | 8 +++ .../ocirepository_controller_test.go | 26 +++++++++- internal/controller/suite_test.go | 38 +++++++++----- 6 files changed, 117 insertions(+), 30 deletions(-) diff --git a/internal/controller/bucket_controller_test.go b/internal/controller/bucket_controller_test.go index 2b7dd4743..952b8e30b 100644 --- a/internal/controller/bucket_controller_test.go +++ b/internal/controller/bucket_controller_test.go @@ -58,6 +58,8 @@ import ( const EnvGcpStorageHost = "STORAGE_EMULATOR_HOST" func TestBucketReconciler_deleteBeforeFinalizer(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) namespaceName := "bucket-" + randStringRunes(5) @@ -196,6 +198,8 @@ func TestBucketReconciler_Reconcile(t *testing.T) { } func TestBucketReconciler_reconcileStorage(t *testing.T) { + testStorage := newTestStorageForTest(t) + tests := []struct { name string beforeFunc func(obj *sourcev1.Bucket, storage *storage.Storage) error @@ -433,6 +437,8 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) { } func TestBucketReconciler_reconcileSource_generic(t *testing.T) { + testStorage := newTestStorageForTest(t) + tests := []struct { name string bucketName string @@ -1002,6 +1008,8 @@ const gcsExternalAccountConfig = `{ }` func TestBucketReconciler_reconcileSource_gcs(t *testing.T) { + testStorage := newTestStorageForTest(t) + tests := []struct { name string bucketName string @@ -1512,6 +1520,8 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) { } func TestBucketReconciler_reconcileArtifact(t *testing.T) { + testStorage := newTestStorageForTest(t) + tests := []struct { name string beforeFunc func(t *WithT, obj *sourcev1.Bucket, index *index.Digester, dir string) diff --git a/internal/controller/gitrepository_controller_test.go b/internal/controller/gitrepository_controller_test.go index 84b2074a8..fdc374202 100644 --- a/internal/controller/gitrepository_controller_test.go +++ b/internal/controller/gitrepository_controller_test.go @@ -206,6 +206,8 @@ KXi2P5xm89dQni0kTeAAY= ) func TestGitRepositoryReconciler_deleteBeforeFinalizer(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) namespaceName := "gitrepo-" + randStringRunes(5) @@ -318,6 +320,8 @@ func TestGitRepositoryReconciler_Reconcile(t *testing.T) { } func TestGitRepositoryReconciler_reconcileSource_emptyRepository(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) server, err := gittestserver.NewTempGitServer() @@ -370,6 +374,8 @@ func TestGitRepositoryReconciler_reconcileSource_emptyRepository(t *testing.T) { } func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) { + testStorage := newTestStorageForTest(t) + type options struct { username string password string @@ -1115,6 +1121,8 @@ func TestGitRepositoryReconciler_getAuthOpts_provider(t *testing.T) { } func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) branches := []string{"staging"} @@ -1365,6 +1373,8 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T) } func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) { + testStorage := newTestStorageForTest(t) + tests := []struct { name string dir string @@ -1730,6 +1740,8 @@ func TestGitRepositoryReconciler_reconcileInclude(t *testing.T) { } func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) { + testStorage := newTestStorageForTest(t) + tests := []struct { name string beforeFunc func(obj *sourcev1.GitRepository, storage *storage.Storage) error @@ -1968,6 +1980,8 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) { } func TestGitRepositoryReconciler_reconcileDelete(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) r := &GitRepositoryReconciler{ @@ -2797,6 +2811,8 @@ func TestGitRepositoryReconciler_verifySignature(t *testing.T) { } func TestGitRepositoryReconciler_ConditionsUpdate(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) server, err := gittestserver.NewTempGitServer() diff --git a/internal/controller/helmchart_controller_test.go b/internal/controller/helmchart_controller_test.go index 29e8ba525..186a20722 100644 --- a/internal/controller/helmchart_controller_test.go +++ b/internal/controller/helmchart_controller_test.go @@ -82,6 +82,8 @@ import ( ) func TestHelmChartReconciler_deleteBeforeFinalizer(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) namespaceName := "helmchart-" + randStringRunes(5) @@ -333,6 +335,8 @@ func TestHelmChartReconciler_Reconcile(t *testing.T) { } func TestHelmChartReconciler_reconcileStorage(t *testing.T) { + testStorage := newTestStorageForTest(t) + tests := []struct { name string beforeFunc func(obj *sourcev1.HelmChart, storage *storage.Storage) error @@ -875,7 +879,7 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { beforeFunc func(obj *sourcev1.HelmChart, repository *sourcev1.HelmRepository) want sreconcile.Result wantErr error - assertFunc func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) + assertFunc func(g *WithT, obj *sourcev1.HelmChart, build chart.Build, storage *storage.Storage) cleanFunc func(g *WithT, build *chart.Build) }{ { @@ -884,7 +888,7 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { obj.Spec.Chart = "helmchart" }, want: sreconcile.ResultSuccess, - assertFunc: func(g *WithT, _ *sourcev1.HelmChart, build chart.Build) { + assertFunc: func(g *WithT, _ *sourcev1.HelmChart, build chart.Build, _ *storage.Storage) { g.Expect(build.Name).To(Equal(chartName)) g.Expect(build.Version).To(Equal(higherChartVersion)) g.Expect(build.Path).ToNot(BeEmpty()) @@ -915,7 +919,7 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { repository.Spec.SecretRef = &meta.LocalObjectReference{Name: "auth"} }, want: sreconcile.ResultSuccess, - assertFunc: func(g *WithT, _ *sourcev1.HelmChart, build chart.Build) { + assertFunc: func(g *WithT, _ *sourcev1.HelmChart, build chart.Build, _ *storage.Storage) { g.Expect(build.Name).To(Equal(chartName)) g.Expect(build.Version).To(Equal(chartVersion)) g.Expect(build.Path).ToNot(BeEmpty()) @@ -933,10 +937,10 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { obj.Status.Artifact = &meta.Artifact{Path: chartName + "-" + chartVersion + ".tgz"} }, want: sreconcile.ResultSuccess, - assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) { + assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build, storage *storage.Storage) { g.Expect(build.Name).To(Equal(chartName)) g.Expect(build.Version).To(Equal(chartVersion)) - g.Expect(build.Path).To(Equal(filepath.Join(serverFactory.Root(), obj.Status.Artifact.Path))) + g.Expect(build.Path).To(Equal(storage.LocalPath(*obj.Status.Artifact))) g.Expect(build.Path).To(BeARegularFile()) }, }, @@ -949,10 +953,10 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { obj.Status.ObservedValuesFiles = []string{"values.yaml", "override.yaml"} }, want: sreconcile.ResultSuccess, - assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) { + assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build, storage *storage.Storage) { g.Expect(build.Name).To(Equal(chartName)) g.Expect(build.Version).To(Equal(chartVersion)) - g.Expect(build.Path).To(Equal(filepath.Join(serverFactory.Root(), obj.Status.Artifact.Path))) + g.Expect(build.Path).To(Equal(storage.LocalPath(*obj.Status.Artifact))) g.Expect(build.Path).To(BeARegularFile()) g.Expect(build.ValuesFiles).To(Equal([]string{"values.yaml", "override.yaml"})) }, @@ -965,7 +969,7 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { obj.Spec.ValuesFiles = []string{"values.yaml", "override.yaml"} }, want: sreconcile.ResultSuccess, - assertFunc: func(g *WithT, _ *sourcev1.HelmChart, build chart.Build) { + assertFunc: func(g *WithT, _ *sourcev1.HelmChart, build chart.Build, _ *storage.Storage) { g.Expect(build.Name).To(Equal(chartName)) g.Expect(build.Version).To(Equal(higherChartVersion + "+3")) g.Expect(build.Path).ToNot(BeEmpty()) @@ -993,7 +997,7 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { obj.Spec.IgnoreMissingValuesFiles = true }, want: sreconcile.ResultSuccess, - assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) { + assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build, _ *storage.Storage) { g.Expect(build.Name).To(Equal(chartName)) g.Expect(build.Version).To(Equal(chartVersion + "+0")) g.Expect(build.ValuesFiles).To(BeEmpty()) @@ -1011,7 +1015,7 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { obj.Spec.IgnoreMissingValuesFiles = true }, want: sreconcile.ResultSuccess, - assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) { + assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build, _ *storage.Storage) { g.Expect(build.Name).To(Equal(chartName)) g.Expect(build.Version).To(Equal(chartVersion + "+0")) g.Expect(build.ValuesFiles).To(Equal([]string{"values.yaml", "override.yaml"})) @@ -1031,10 +1035,10 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { obj.Status.Artifact = &meta.Artifact{Path: chartName + "-" + chartVersion + ".tgz"} }, want: sreconcile.ResultSuccess, - assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) { + assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build, storage *storage.Storage) { g.Expect(build.Name).To(Equal(chartName)) g.Expect(build.Version).To(Equal(chartVersion)) - g.Expect(build.Path).ToNot(Equal(filepath.Join(serverFactory.Root(), obj.Status.Artifact.Path))) + g.Expect(build.Path).ToNot(Equal(storage.LocalPath(*obj.Status.Artifact))) g.Expect(build.Path).To(BeARegularFile()) }, cleanFunc: func(g *WithT, build *chart.Build) { @@ -1050,7 +1054,7 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { }, want: sreconcile.ResultEmpty, wantErr: &serror.Generic{Err: errors.New("failed to get authentication secret '/invalid': secrets \"invalid\" not found")}, - assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) { + assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build, _ *storage.Storage) { g.Expect(build.Complete()).To(BeFalse()) g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ @@ -1065,7 +1069,7 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { }, want: sreconcile.ResultEmpty, wantErr: &serror.Stalling{Err: errors.New("scheme \"file\" not supported")}, - assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) { + assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build, _ *storage.Storage) { g.Expect(build.Complete()).To(BeFalse()) g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ @@ -1080,7 +1084,7 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { }, want: sreconcile.ResultEmpty, wantErr: &serror.Stalling{Err: errors.New("missing protocol scheme")}, - assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) { + assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build, _ *storage.Storage) { g.Expect(build.Complete()).To(BeFalse()) g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ @@ -1101,7 +1105,10 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - server := testserver.NewHTTPServer(serverFactory.Root()) + serverRoot := t.TempDir() + g.Expect(os.CopyFS(serverRoot, os.DirFS(serverFactory.Root()))).To(Succeed()) + + server := testserver.NewHTTPServer(serverRoot) server.Start() defer server.Stop() @@ -1177,7 +1184,7 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { g.Expect(got).To(Equal(tt.want)) if tt.assertFunc != nil { - tt.assertFunc(g, obj, b) + tt.assertFunc(g, obj, b, testStorage) } }) } @@ -1668,6 +1675,8 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) { } func TestHelmChartReconciler_reconcileArtifact(t *testing.T) { + testStorage := newTestStorageForTest(t) + tests := []struct { name string build *chart.Build @@ -2025,6 +2034,8 @@ func TestHelmChartReconciler_getSource(t *testing.T) { } func TestHelmChartReconciler_reconcileDelete(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) r := &HelmChartReconciler{ @@ -2768,6 +2779,8 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { } func TestHelmChartRepository_reconcileSource_verifyOCISourceSignature_keyless(t *testing.T) { + testStorage := newTestStorageForTest(t) + tests := []struct { name string version string @@ -2944,6 +2957,8 @@ func TestHelmChartRepository_reconcileSource_verifyOCISourceSignature_keyless(t } func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignatureNotation(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) tmpDir := t.TempDir() diff --git a/internal/controller/helmrepository_controller_test.go b/internal/controller/helmrepository_controller_test.go index f76d4f221..33fb9572a 100644 --- a/internal/controller/helmrepository_controller_test.go +++ b/internal/controller/helmrepository_controller_test.go @@ -60,6 +60,8 @@ import ( ) func TestHelmRepositoryReconciler_deleteBeforeFinalizer(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) namespaceName := "helmrepo-" + randStringRunes(5) @@ -171,6 +173,8 @@ func TestHelmRepositoryReconciler_Reconcile(t *testing.T) { } func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) { + testStorage := newTestStorageForTest(t) + tests := []struct { name string beforeFunc func(obj *sourcev1.HelmRepository, storage *storage.Storage) error @@ -405,6 +409,8 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) { } func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) { + testStorage := newTestStorageForTest(t) + type options struct { username string password string @@ -1073,6 +1079,8 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) { } func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) { + testStorage := newTestStorageForTest(t) + tests := []struct { name string cache *cache.Cache diff --git a/internal/controller/ocirepository_controller_test.go b/internal/controller/ocirepository_controller_test.go index 3b18e1fe1..813a17cf7 100644 --- a/internal/controller/ocirepository_controller_test.go +++ b/internal/controller/ocirepository_controller_test.go @@ -84,6 +84,8 @@ var ( ) func TestOCIRepositoryReconciler_deleteBeforeFinalizer(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) namespaceName := "ocirepo-" + randStringRunes(5) @@ -240,7 +242,7 @@ func TestOCIRepository_Reconcile(t *testing.T) { g.Expect(obj.Status.Artifact.Metadata[oci.RevisionAnnotation]).To(ContainSubstring(tt.tag)) // Check if the artifact storage path matches the expected file path - localPath := testStorage.LocalPath(*obj.Status.Artifact) + localPath := managerStorage.LocalPath(*obj.Status.Artifact) t.Logf("artifact local path: %s", localPath) f, err := os.Open(localPath) @@ -414,6 +416,8 @@ func TestOCIRepository_Reconcile_MediaType(t *testing.T) { } func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) { + testStorage := newTestStorageForTest(t) + type secretOptions struct { username string password string @@ -1130,6 +1134,8 @@ func TestOCIRepository_ProxySecret(t *testing.T) { } func TestOCIRepository_reconcileSource_remoteReference(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) tmpDir := t.TempDir() @@ -1315,6 +1321,8 @@ func TestOCIRepository_reconcileSource_remoteReference(t *testing.T) { } func TestOCIRepository_reconcileSource_verifyOCISourceSignatureNotation(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) tests := []struct { @@ -1669,6 +1677,8 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignatureNotation(t *testi } func TestOCIRepository_reconcileSource_verifyOCISourceTrustPolicyNotation(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) tests := []struct { @@ -2013,6 +2023,8 @@ func TestOCIRepository_reconcileSource_verifyOCISourceTrustPolicyNotation(t *tes } func TestOCIRepository_reconcileSource_verifyOCISourceSignatureCosign(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) tests := []struct { @@ -2285,6 +2297,8 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignatureCosign(t *testing } func TestOCIRepository_reconcileSource_verifyOCISourceSignature_keyless(t *testing.T) { + testStorage := newTestStorageForTest(t) + tests := []struct { name string reference *sourcev1.OCIRepositoryRef @@ -2461,6 +2475,8 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature_keyless(t *testi } func TestOCIRepository_reconcileSource_noop(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) testRevision := "6.1.5@sha256:8e4057c22d531d40e12b065443cb0d80394b7257c4dc557cb1fbd4dce892b86d" @@ -2634,6 +2650,8 @@ func TestOCIRepository_reconcileSource_noop(t *testing.T) { } func TestOCIRepository_reconcileArtifact(t *testing.T) { + testStorage := newTestStorageForTest(t) + tests := []struct { name string targetPath string @@ -2880,6 +2898,8 @@ func TestOCIRepository_reconcileArtifact(t *testing.T) { } func TestOCIRepository_getArtifactRef(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) tmpDir := t.TempDir() @@ -3132,6 +3152,8 @@ func TestOCIRepository_objectLevelWorkloadIdentityFeatureGate(t *testing.T) { } func TestOCIRepository_reconcileStorage(t *testing.T) { + testStorage := newTestStorageForTest(t) + tests := []struct { name string beforeFunc func(obj *sourcev1.OCIRepository, storage *storage.Storage) error @@ -3377,6 +3399,8 @@ func TestOCIRepository_reconcileStorage(t *testing.T) { } func TestOCIRepository_ReconcileDelete(t *testing.T) { + testStorage := newTestStorageForTest(t) + g := NewWithT(t) r := &OCIRepositoryReconciler{ diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 53da2f74e..47a6ed059 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -81,12 +81,12 @@ const ( ) var ( - k8sClient client.Client - testEnv *testenv.Environment - testStorage *storage.Storage - testServer *testserver.ArtifactServer - testMetricsH controller.Metrics - ctx = ctrl.SetupSignalHandler() + k8sClient client.Client + testEnv *testenv.Environment + managerStorage *storage.Storage + testServer *testserver.ArtifactServer + testMetricsH controller.Metrics + ctx = ctrl.SetupSignalHandler() ) var ( @@ -301,7 +301,7 @@ func TestMain(m *testing.M) { fmt.Println("Starting the test storage server") testServer.Start() - testStorage, err = newTestStorage(testServer.HTTPServer) + managerStorage, err = newTestStorage(testServer.HTTPServer) if err != nil { panic(fmt.Sprintf("Failed to create a test storage: %v", err)) } @@ -324,7 +324,7 @@ func TestMain(m *testing.M) { Client: testEnv, EventRecorder: record.NewFakeRecorder(32), Metrics: testMetricsH, - Storage: testStorage, + Storage: managerStorage, }).SetupWithManager(testEnv, GitRepositoryReconcilerOptions{ RateLimiter: controller.GetDefaultRateLimiter(), }); err != nil { @@ -335,7 +335,7 @@ func TestMain(m *testing.M) { Client: testEnv, EventRecorder: record.NewFakeRecorder(32), Metrics: testMetricsH, - Storage: testStorage, + Storage: managerStorage, }).SetupWithManager(testEnv, BucketReconcilerOptions{ RateLimiter: controller.GetDefaultRateLimiter(), }); err != nil { @@ -349,7 +349,7 @@ func TestMain(m *testing.M) { Client: testEnv, EventRecorder: record.NewFakeRecorder(32), Metrics: testMetricsH, - Storage: testStorage, + Storage: managerStorage, }).SetupWithManager(testEnv, OCIRepositoryReconcilerOptions{ RateLimiter: controller.GetDefaultRateLimiter(), }); err != nil { @@ -361,7 +361,7 @@ func TestMain(m *testing.M) { EventRecorder: record.NewFakeRecorder(32), Metrics: testMetricsH, Getters: testGetters, - Storage: testStorage, + Storage: managerStorage, Cache: testCache, TTL: 1 * time.Second, CacheRecorder: cacheRecorder, @@ -376,7 +376,7 @@ func TestMain(m *testing.M) { EventRecorder: record.NewFakeRecorder(32), Metrics: testMetricsH, Getters: testGetters, - Storage: testStorage, + Storage: managerStorage, Cache: testCache, TTL: 1 * time.Second, CacheRecorder: cacheRecorder, @@ -500,6 +500,20 @@ func newTestStorage(s *testserver.HTTPServer) (*storage.Storage, error) { return st, nil } +func newTestStorageForTest(t *testing.T) *storage.Storage { + t.Helper() + + server := testserver.NewHTTPServer(t.TempDir()) + server.Start() + t.Cleanup(server.Stop) + + st, err := newTestStorage(server) + if err != nil { + t.Fatalf("Failed to create a test storage: %v", err) + } + return st +} + var letterRunes = []rune("abcdefghijklmnopqrstuvwxyz1234567890") func randStringRunes(n int) string { From dc60affb54642b29474d437d67da82a2431a7c0e Mon Sep 17 00:00:00 2001 From: Mykola Date: Mon, 24 Aug 2026 17:43:25 +0300 Subject: [PATCH 2/2] Optimize isolated test storage Reuse the suite artifact server with per-test URL prefixes to avoid spawning an HTTP server for every test. Seed only required Helm cache artifacts and add parallel, HTTP isolation, and cleanup coverage. Signed-off-by: Mykola Assisted-by: Codex/GPT-5 --- .../controller/helmchart_controller_test.go | 27 ++--- internal/controller/suite_test.go | 105 ++++++++++++++++-- 2 files changed, 110 insertions(+), 22 deletions(-) diff --git a/internal/controller/helmchart_controller_test.go b/internal/controller/helmchart_controller_test.go index 186a20722..41a311f51 100644 --- a/internal/controller/helmchart_controller_test.go +++ b/internal/controller/helmchart_controller_test.go @@ -879,7 +879,7 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { beforeFunc func(obj *sourcev1.HelmChart, repository *sourcev1.HelmRepository) want sreconcile.Result wantErr error - assertFunc func(g *WithT, obj *sourcev1.HelmChart, build chart.Build, storage *storage.Storage) + assertFunc func(g *WithT, obj *sourcev1.HelmChart, build chart.Build, testStorage *storage.Storage) cleanFunc func(g *WithT, build *chart.Build) }{ { @@ -937,10 +937,10 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { obj.Status.Artifact = &meta.Artifact{Path: chartName + "-" + chartVersion + ".tgz"} }, want: sreconcile.ResultSuccess, - assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build, storage *storage.Storage) { + assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build, testStorage *storage.Storage) { g.Expect(build.Name).To(Equal(chartName)) g.Expect(build.Version).To(Equal(chartVersion)) - g.Expect(build.Path).To(Equal(storage.LocalPath(*obj.Status.Artifact))) + g.Expect(build.Path).To(Equal(testStorage.LocalPath(*obj.Status.Artifact))) g.Expect(build.Path).To(BeARegularFile()) }, }, @@ -953,10 +953,10 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { obj.Status.ObservedValuesFiles = []string{"values.yaml", "override.yaml"} }, want: sreconcile.ResultSuccess, - assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build, storage *storage.Storage) { + assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build, testStorage *storage.Storage) { g.Expect(build.Name).To(Equal(chartName)) g.Expect(build.Version).To(Equal(chartVersion)) - g.Expect(build.Path).To(Equal(storage.LocalPath(*obj.Status.Artifact))) + g.Expect(build.Path).To(Equal(testStorage.LocalPath(*obj.Status.Artifact))) g.Expect(build.Path).To(BeARegularFile()) g.Expect(build.ValuesFiles).To(Equal([]string{"values.yaml", "override.yaml"})) }, @@ -1035,10 +1035,10 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { obj.Status.Artifact = &meta.Artifact{Path: chartName + "-" + chartVersion + ".tgz"} }, want: sreconcile.ResultSuccess, - assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build, storage *storage.Storage) { + assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build, testStorage *storage.Storage) { g.Expect(build.Name).To(Equal(chartName)) g.Expect(build.Version).To(Equal(chartVersion)) - g.Expect(build.Path).ToNot(Equal(storage.LocalPath(*obj.Status.Artifact))) + g.Expect(build.Path).ToNot(Equal(testStorage.LocalPath(*obj.Status.Artifact))) g.Expect(build.Path).To(BeARegularFile()) }, cleanFunc: func(g *WithT, build *chart.Build) { @@ -1105,10 +1105,7 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - serverRoot := t.TempDir() - g.Expect(os.CopyFS(serverRoot, os.DirFS(serverFactory.Root()))).To(Succeed()) - - server := testserver.NewHTTPServer(serverRoot) + server := testserver.NewHTTPServer(serverFactory.Root()) server.Start() defer server.Stop() @@ -1133,8 +1130,7 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { clientBuilder.WithObjects(tt.secret.DeepCopy()) } - testStorage, err := newTestStorage(server) - g.Expect(err).ToNot(HaveOccurred()) + testStorage := newTestStorageForTest(t) r := &HelmChartReconciler{ Client: clientBuilder.Build(), @@ -1169,6 +1165,11 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { if tt.beforeFunc != nil { tt.beforeFunc(obj, repository) } + if obj.Status.Artifact != nil { + artifact, err := os.ReadFile(filepath.Join(serverFactory.Root(), obj.Status.Artifact.Path)) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(os.WriteFile(testStorage.LocalPath(*obj.Status.Artifact), artifact, 0o600)).To(Succeed()) + } var b chart.Build if tt.cleanFunc != nil { diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 47a6ed059..3e897696d 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -36,6 +36,7 @@ import ( _ "github.com/distribution/distribution/v3/registry/auth/htpasswd" _ "github.com/distribution/distribution/v3/registry/storage/driver/inmemory" "github.com/miekg/dns" + . "github.com/onsi/gomega" "github.com/phayes/freeport" "github.com/sirupsen/logrus" "golang.org/x/crypto/bcrypt" @@ -485,10 +486,14 @@ func initTestTLS() { } func newTestStorage(s *testserver.HTTPServer) (*storage.Storage, error) { + return newTestStorageAt(s.Root(), s.URL()) +} + +func newTestStorageAt(root, address string) (*storage.Storage, error) { opts := &config.Options{ - StoragePath: s.Root(), - StorageAddress: s.URL(), - StorageAdvAddress: s.URL(), + StoragePath: root, + StorageAddress: address, + StorageAdvAddress: address, ArtifactRetentionTTL: retentionTTL, ArtifactRetentionRecords: retentionRecords, ArtifactDigestAlgo: digest.Canonical.String(), @@ -500,20 +505,102 @@ func newTestStorage(s *testserver.HTTPServer) (*storage.Storage, error) { return st, nil } -func newTestStorageForTest(t *testing.T) *storage.Storage { +func newTestStorageForTest(t testing.TB) *storage.Storage { t.Helper() - server := testserver.NewHTTPServer(t.TempDir()) - server.Start() - t.Cleanup(server.Stop) + root, err := os.MkdirTemp(testServer.Root(), "test-storage-") + if err != nil { + t.Fatalf("failed to create test storage directory: %v", err) + } + t.Cleanup(func() { + if err := os.RemoveAll(root); err != nil { + t.Errorf("failed to remove test storage directory: %v", err) + } + }) - st, err := newTestStorage(server) + address := testServer.URL() + "/" + filepath.Base(root) + st, err := newTestStorageAt(root, address) if err != nil { - t.Fatalf("Failed to create a test storage: %v", err) + t.Fatalf("failed to create test storage: %v", err) } return st } +func assertTestStorageArtifact(t testing.TB, st *storage.Storage, artifactPath, content string) { + t.Helper() + g := NewWithT(t) + + g.Expect(os.WriteFile(filepath.Join(st.BasePath, artifactPath), []byte(content), 0o600)).To(Succeed()) + + requestCtx, cancel := context.WithTimeout(t.Context(), timeout) + defer cancel() + request, err := http.NewRequestWithContext(requestCtx, http.MethodGet, st.Hostname+"/"+artifactPath, nil) + g.Expect(err).NotTo(HaveOccurred()) + response, err := http.DefaultClient.Do(request) + g.Expect(err).NotTo(HaveOccurred()) + defer func() { + g.Expect(response.Body.Close()).To(Succeed()) + }() + + g.Expect(response.StatusCode).To(Equal(http.StatusOK)) + body, err := io.ReadAll(response.Body) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(string(body)).To(Equal(content)) +} + +func TestNewTestStorageForTest(t *testing.T) { + t.Run("isolates artifact roots and URLs", func(t *testing.T) { + g := NewWithT(t) + first := newTestStorageForTest(t) + second := newTestStorageForTest(t) + + g.Expect(first.BasePath).NotTo(Equal(second.BasePath)) + g.Expect(first.Hostname).NotTo(Equal(second.Hostname)) + + const artifactPath = "artifact.txt" + artifacts := []struct { + name string + storage *storage.Storage + content string + }{ + {name: "first storage", storage: first, content: "first"}, + {name: "second storage", storage: second, content: "second"}, + } + + for _, tt := range artifacts { + t.Run(tt.name, func(t *testing.T) { + assertTestStorageArtifact(t, tt.storage, artifactPath, tt.content) + }) + } + }) + + t.Run("supports parallel test cases", func(t *testing.T) { + for _, content := range []string{"first", "second", "third", "fourth"} { + t.Run(content, func(t *testing.T) { + t.Parallel() + st := newTestStorageForTest(t) + assertTestStorageArtifact(t, st, "artifact.txt", content) + }) + } + }) + + t.Run("removes its artifact root during cleanup", func(t *testing.T) { + g := NewWithT(t) + var root string + + t.Run("create storage", func(t *testing.T) { + g := NewWithT(t) + st := newTestStorageForTest(t) + root = st.BasePath + g.Expect(os.WriteFile(filepath.Join(root, "artifact.txt"), []byte("content"), 0o600)).To(Succeed()) + }) + + _, err := os.Stat(root) + g.Expect(err).To(HaveOccurred()) + g.Expect(os.IsNotExist(err)).To(BeTrue()) + }) +} + var letterRunes = []rune("abcdefghijklmnopqrstuvwxyz1234567890") func randStringRunes(n int) string {