From 1b06c02fb8b1dcb6b5d3b490fa913a2533a681af Mon Sep 17 00:00:00 2001 From: Leo Di Donato <120051+leodido@users.noreply.github.com> Date: Mon, 10 Nov 2025 19:39:47 +0000 Subject: [PATCH 1/2] test: remove flaky TestS3Cache_VerificationOverhead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test was measuring sub-millisecond durations (~10-12µs) which are dominated by measurement noise, Go runtime scheduling variance, and test framework overhead. This resulted in: - Inconsistent results across runs - Negative overhead values (impossible in reality) - Flaky CI failures The test itself acknowledged these limitations in its comments and directed users to use benchmarks instead. The existing benchmark functions (BenchmarkS3Cache_DownloadBaseline and BenchmarkS3Cache_DownloadWithVerification) provide accurate and reliable performance measurements. Co-authored-by: Ona --- .../cache/remote/s3_performance_test.go | 134 ------------------ 1 file changed, 134 deletions(-) diff --git a/pkg/leeway/cache/remote/s3_performance_test.go b/pkg/leeway/cache/remote/s3_performance_test.go index 7971077..e566b14 100644 --- a/pkg/leeway/cache/remote/s3_performance_test.go +++ b/pkg/leeway/cache/remote/s3_performance_test.go @@ -289,140 +289,6 @@ func BenchmarkS3Cache_DownloadWithVerification(b *testing.B) { } } -// TestS3Cache_VerificationOverhead validates verification overhead -// Note: This test may show inconsistent results due to S3Cache optimizations -// For accurate performance measurements, use the benchmark functions instead -func TestS3Cache_VerificationOverhead(t *testing.T) { - if testing.Short() { - t.Skip("skipping performance test in short mode") - } - - t.Log("Note: For accurate performance measurements, run benchmarks:") - t.Log("go test -bench=BenchmarkS3Cache_DownloadBaseline") - t.Log("go test -bench=BenchmarkS3Cache_DownloadWithVerification") - - sizes := []struct { - name string - size int64 - }{ - {"1MB", 1 * 1024 * 1024}, - {"10MB", 10 * 1024 * 1024}, - {"50MB", 50 * 1024 * 1024}, - } - - const targetOverhead = 100.0 // Lenient target due to test limitations - const iterations = 3 // Average over multiple runs for better accuracy - - for _, tt := range sizes { - t.Run(tt.name, func(t *testing.T) { - // Measure baseline (no verification) - var baselineTotal time.Duration - for i := 0; i < iterations; i++ { - duration := measureDownloadTimePerf(t, tt.size, false) - baselineTotal += duration - } - baselineAvg := baselineTotal / iterations - - // Measure with SLSA verification - var verifiedTotal time.Duration - for i := 0; i < iterations; i++ { - duration := measureDownloadTimePerf(t, tt.size, true) - verifiedTotal += duration - } - verifiedAvg := verifiedTotal / iterations - - // Calculate overhead percentage - overhead := float64(verifiedAvg-baselineAvg) / float64(baselineAvg) * 100 - - t.Logf("Size: %s, Baseline: %v, Verified: %v, Overhead: %.2f%%", - tt.name, baselineAvg, verifiedAvg, overhead) - - // Assert overhead is within target - if overhead > targetOverhead { - t.Errorf("Verification overhead %.2f%% exceeds target of %.2f%%", - overhead, targetOverhead) - } else { - t.Logf("✓ Overhead %.2f%% is within target", overhead) - } - }) - } -} - -// measureDownloadTimePerf measures a single download operation for performance testing -func measureDownloadTimePerf(t *testing.T, size int64, withVerification bool) time.Duration { - // Create test artifact with unique name to avoid caching - artifactPath := createSizedArtifact(t, size) - defer os.Remove(artifactPath) - - // Setup cache with unique package name to avoid caching - packageName := fmt.Sprintf("test-package-%d", time.Now().UnixNano()) - config := &cache.RemoteConfig{ - BucketName: "test-bucket", - } - - if withVerification { - attestation := createMockAttestation(t) - config.SLSA = &cache.SLSAConfig{ - Verification: true, - SourceURI: "github.com/gitpod-io/leeway", - RequireAttestation: false, - } - - mockStorage := createRealisticMockS3Storage(t, artifactPath, attestation) - // Update storage with unique package name - data := mockStorage.objects["test-package:v1.tar.gz"] - delete(mockStorage.objects, "test-package:v1.tar.gz") - delete(mockStorage.objects, "test-package:v1.tar.gz.att") - mockStorage.objects[packageName+":v1.tar.gz"] = data - mockStorage.objects[packageName+":v1.tar.gz.att"] = attestation - - mockVerifier := &realisticMockVerifier{} - - s3Cache := &S3Cache{ - storage: mockStorage, - cfg: config, - slsaVerifier: mockVerifier, - } - - tmpDir := t.TempDir() - localCache, _ := local.NewFilesystemCache(tmpDir) - pkg := &mockPackagePerf{version: "v1", fullName: packageName} - - // Ensure package doesn't exist locally to force download - packages := []cache.Package{pkg} - - start := time.Now() - err := s3Cache.Download(context.Background(), localCache, packages) - require.NoError(t, err) - - return time.Since(start) - } else { - mockStorage := createRealisticMockS3Storage(t, artifactPath, nil) - // Update storage with unique package name - data := mockStorage.objects["test-package:v1.tar.gz"] - delete(mockStorage.objects, "test-package:v1.tar.gz") - mockStorage.objects[packageName+":v1.tar.gz"] = data - - s3Cache := &S3Cache{ - storage: mockStorage, - cfg: config, - } - - tmpDir := t.TempDir() - localCache, _ := local.NewFilesystemCache(tmpDir) - pkg := &mockPackagePerf{version: "v1", fullName: packageName} - - // Ensure package doesn't exist locally to force download - packages := []cache.Package{pkg} - - start := time.Now() - err := s3Cache.Download(context.Background(), localCache, packages) - require.NoError(t, err) - - return time.Since(start) - } -} - // BenchmarkS3Cache_ParallelDownloads measures concurrent download performance func BenchmarkS3Cache_ParallelDownloads(b *testing.B) { if testing.Short() { From a4e978c2742a237404ac494159a0145011af124b Mon Sep 17 00:00:00 2001 From: Leo Di Donato <120051+leodido@users.noreply.github.com> Date: Mon, 10 Nov 2025 19:46:28 +0000 Subject: [PATCH 2/2] test: remove unused createRealisticMockS3Storage helper This helper function was only used by the removed TestS3Cache_VerificationOverhead test and is no longer needed. Co-authored-by: Ona --- pkg/leeway/cache/remote/s3_performance_test.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/pkg/leeway/cache/remote/s3_performance_test.go b/pkg/leeway/cache/remote/s3_performance_test.go index e566b14..bf00f16 100644 --- a/pkg/leeway/cache/remote/s3_performance_test.go +++ b/pkg/leeway/cache/remote/s3_performance_test.go @@ -132,24 +132,6 @@ func (m *realisticMockVerifier) VerifyArtifact(ctx context.Context, artifactPath return nil // Success } -// Test helper: Create realistic mock S3 storage for performance testing -func createRealisticMockS3Storage(t testing.TB, artifactPath string, attestation []byte) *realisticMockS3Storage { - data, err := os.ReadFile(artifactPath) - require.NoError(t, err) - - storage := &realisticMockS3Storage{ - objects: map[string][]byte{ - "test-package:v1.tar.gz": data, - }, - } - - if attestation != nil { - storage.objects["test-package:v1.tar.gz.att"] = attestation - } - - return storage -} - // Test helper: Create realistic mock S3 storage for multiple packages func createRealisticMockS3StorageMultiple(t testing.TB, packageCount int) *realisticMockS3Storage { storage := &realisticMockS3Storage{