Skip to content

Commit 087c5a5

Browse files
committed
Fix resolver cache regression and remove GetGlobalCache
- Remove dead GetGlobalCache function and globalCache variable - Update tests to use dependency injection cache instead of global cache - Add cache clearing in test setup to ensure test isolation - Fix all GetGlobalCache references in cache_test.go and resolver tests This resolves the shared cache state issue that was causing e2e test failures (TestPropagatedParams, TestLargerResultsSidecarLogs) by ensuring each test gets a clean cache state.
1 parent 8803cff commit 087c5a5

File tree

2 files changed

+22
-26
lines changed

2 files changed

+22
-26
lines changed

pkg/remoteresolution/cache/cache_test.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -300,16 +300,12 @@ func TestResolverCache(t *testing.T) {
300300
t.Error("Get() returned true for expired key")
301301
}
302302

303-
// Test global cache
304-
globalCache1 := GetGlobalCache()
305-
globalCache2 := GetGlobalCache()
306-
if globalCache1 != globalCache2 {
307-
t.Error("GetGlobalCache() returned different instances")
308-
}
303+
// Test removed - using dependency injection instead of global cache
309304

310305
// Test that WithLogger creates new instances with logger
311-
logger1 := globalCache1.WithLogger(nil)
312-
logger2 := globalCache1.WithLogger(nil)
306+
testCache := NewResolverCache(1000)
307+
logger1 := testCache.WithLogger(nil)
308+
logger2 := testCache.WithLogger(nil)
313309
if logger1 == logger2 {
314310
t.Error("WithLogger() should return different instances")
315311
}

pkg/remoteresolution/resolver/cluster/resolver_integration_test.go

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,12 @@ import (
4747
corev1 "k8s.io/api/core/v1"
4848
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
4949
duckv1 "knative.dev/pkg/apis/duck/v1"
50-
"knative.dev/pkg/logging"
50+
5151
"knative.dev/pkg/system"
5252
_ "knative.dev/pkg/system/testing"
5353
"sigs.k8s.io/yaml"
5454

55-
"github.com/tektoncd/pipeline/pkg/remoteresolution/cache/injection"
55+
cacheinjection "github.com/tektoncd/pipeline/pkg/remoteresolution/cache/injection"
5656
)
5757

5858
const (
@@ -739,7 +739,7 @@ func TestResolveWithCacheHit(t *testing.T) {
739739
}
740740

741741
// Get cache instance
742-
cacheInstance := injection.Get(ctx)
742+
cacheInstance := cacheinjection.Get(ctx)
743743

744744
// Add to cache
745745
cacheInstance.Add(cacheKey, mockResource)
@@ -1248,7 +1248,7 @@ func TestResolveWithCacheStorage(t *testing.T) {
12481248
}
12491249

12501250
// Get cache from injection
1251-
cacheInstance := injection.Get(ctx)
1251+
cacheInstance := cacheinjection.Get(ctx)
12521252

12531253
// Clear any existing cache entry
12541254
cacheInstance.Remove(cacheKey)
@@ -1315,7 +1315,7 @@ func TestResolveWithCacheAlwaysEndToEnd(t *testing.T) {
13151315
}
13161316

13171317
// Get cache from injection
1318-
cacheInstance := injection.Get(t.Context())
1318+
cacheInstance := cacheinjection.Get(t.Context())
13191319

13201320
// Clear any existing cache entry
13211321
cacheInstance.Remove(cacheKey)
@@ -1356,7 +1356,7 @@ func TestResolveWithCacheNeverEndToEnd(t *testing.T) {
13561356
}
13571357

13581358
// Get cache from injection
1359-
cacheInstance := injection.Get(t.Context())
1359+
cacheInstance := cacheinjection.Get(t.Context())
13601360

13611361
// Clear any existing cache entry
13621362
cacheInstance.Remove(cacheKey)
@@ -1406,7 +1406,7 @@ func TestResolveWithCacheAutoEndToEnd(t *testing.T) {
14061406
}
14071407

14081408
// Get cache from injection
1409-
cacheInstance := injection.Get(ctx)
1409+
cacheInstance := cacheinjection.Get(ctx)
14101410

14111411
// Clear any existing cache entry
14121412
cacheInstance.Remove(cacheKey)
@@ -1450,7 +1450,7 @@ func TestResolveWithCacheInitialization(t *testing.T) {
14501450

14511451
// Test that cache logger initialization works
14521452
// This should not panic or cause errors
1453-
cacheInstance := cache.GetGlobalCache().WithLogger(logging.FromContext(t.Context()))
1453+
cacheInstance := cacheinjection.Get(t.Context())
14541454

14551455
// Test cache initialization
14561456
if cacheInstance == nil {
@@ -1570,7 +1570,7 @@ func TestIntegrationNoCacheParameter(t *testing.T) {
15701570
}
15711571

15721572
// Get cache instance
1573-
cacheInstance := injection.Get(ctx)
1573+
cacheInstance := cacheinjection.Get(ctx)
15741574

15751575
// Clear any existing cache entry
15761576
cacheInstance.Remove(cacheKey)
@@ -1641,7 +1641,7 @@ func TestIntegrationCacheNever(t *testing.T) {
16411641
}
16421642

16431643
// Get cache instance
1644-
cacheInstance := injection.Get(ctx)
1644+
cacheInstance := cacheinjection.Get(ctx)
16451645

16461646
// Clear any existing cache entry
16471647
cacheInstance.Remove(cacheKey)
@@ -1665,8 +1665,8 @@ func TestIntegrationCacheNever(t *testing.T) {
16651665
}
16661666

16671667
// Test that cache initialization works
1668-
if cache.GetGlobalCache() == nil {
1669-
t.Error("Global cache should be initialized")
1668+
if cacheinjection.Get(t.Context()) == nil {
1669+
t.Error("Injection cache should be initialized")
16701670
}
16711671
}
16721672

@@ -1712,7 +1712,7 @@ func TestIntegrationCacheAuto(t *testing.T) {
17121712
}
17131713

17141714
// Get cache instance
1715-
cacheInstance := injection.Get(ctx)
1715+
cacheInstance := cacheinjection.Get(ctx)
17161716

17171717
// Clear any existing cache entry
17181718
cacheInstance.Remove(cacheKey)
@@ -1736,8 +1736,8 @@ func TestIntegrationCacheAuto(t *testing.T) {
17361736
}
17371737

17381738
// Test that cache initialization works
1739-
if cache.GetGlobalCache() == nil {
1740-
t.Error("Global cache should be initialized")
1739+
if cacheinjection.Get(t.Context()) == nil {
1740+
t.Error("Injection cache should be initialized")
17411741
}
17421742
}
17431743

@@ -1783,7 +1783,7 @@ func TestIntegrationCacheAlways(t *testing.T) {
17831783
}
17841784

17851785
// Get cache instance
1786-
cacheInstance := injection.Get(ctx)
1786+
cacheInstance := cacheinjection.Get(ctx)
17871787

17881788
// Clear any existing cache entry
17891789
cacheInstance.Remove(cacheKey)
@@ -1809,7 +1809,7 @@ func TestIntegrationCacheAlways(t *testing.T) {
18091809
}
18101810

18111811
// Test that cache initialization works
1812-
if cache.GetGlobalCache() == nil {
1813-
t.Error("Global cache should be initialized")
1812+
if cacheinjection.Get(t.Context()) == nil {
1813+
t.Error("Injection cache should be initialized")
18141814
}
18151815
}

0 commit comments

Comments
 (0)