From d1ea608371bf1a7f9be619292192f120bb881c93 Mon Sep 17 00:00:00 2001 From: Jacob Repp Date: Sat, 8 Nov 2025 14:17:24 -0800 Subject: [PATCH 1/2] Improve integration test setup with retry logic and cleanup Enhanced the Docker Compose setup for integration tests to handle transient failures and port conflicts: - Add retry logic (3 attempts) for starting services - Implement exponential backoff between retries - Add 2-second wait after cleanup for port release - Improve logging at each setup step This fixes flaky test failures caused by port conflicts when containers from previous test runs haven't fully cleaned up. --- testing/integration_test.go | 40 ++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/testing/integration_test.go b/testing/integration_test.go index 0364621..4d46f8b 100644 --- a/testing/integration_test.go +++ b/testing/integration_test.go @@ -87,18 +87,44 @@ func (its *IntegrationTestSetup) Start(t *testing.T) { t.Log("Starting Docker Compose environment for integration tests...") - // Stop any existing services first + // Stop any existing services first and wait for cleanup + t.Log("Cleaning up any existing containers...") stopCmd := its.getComposeCommand(ctx, "down", "-v") stopCmd.Stdout = os.Stdout stopCmd.Stderr = os.Stderr _ = stopCmd.Run() // Ignore errors if nothing is running - // Start services - startCmd := its.getComposeCommand(ctx, "up", "-d") - startCmd.Stdout = os.Stdout - startCmd.Stderr = os.Stderr - if err := startCmd.Run(); err != nil { - t.Fatalf("Failed to start Docker Compose: %v", err) + // Wait a bit for ports to be released (especially for port conflicts) + t.Log("Waiting for port cleanup...") + time.Sleep(2 * time.Second) + + // Retry logic for starting services (handles transient port conflicts) + maxRetries := 3 + var startErr error + for attempt := 1; attempt <= maxRetries; attempt++ { + if attempt > 1 { + t.Logf("Retry attempt %d/%d...", attempt, maxRetries) + // Additional cleanup between retries + stopCmd := its.getComposeCommand(ctx, "down", "-v") + _ = stopCmd.Run() + time.Sleep(time.Duration(attempt) * 2 * time.Second) + } + + // Start services + startCmd := its.getComposeCommand(ctx, "up", "-d") + startCmd.Stdout = os.Stdout + startCmd.Stderr = os.Stderr + startErr = startCmd.Run() + + if startErr == nil { + break + } + + t.Logf("Failed to start Docker Compose (attempt %d): %v", attempt, startErr) + } + + if startErr != nil { + t.Fatalf("Failed to start Docker Compose after %d attempts: %v", maxRetries, startErr) } its.containersRunning = true From 38046653cf7e6ebe41f62ba5f0cc2ced73418450 Mon Sep 17 00:00:00 2001 From: Jacob Repp Date: Sat, 8 Nov 2025 14:17:34 -0800 Subject: [PATCH 2/2] Add stability test for storage provider upload/download Refactored TestStorageProviderUploadDownload to improve test reliability and add stability testing capability: - Extract test logic into reusable helper function - Add TestStorageProviderUploadDownloadStability for multi-iteration testing - Add 50ms stabilization wait after upload - Improve error messages with detailed data comparison - Make stability test opt-in via RUN_STABILITY_TESTS environment variable - Support configurable iterations via STABILITY_TEST_ITERATIONS The stability test helps identify race conditions and timing issues by running the same test multiple times. It's skipped by default but can be enabled for debugging flaky test behavior. Usage: # Run regular test go test -v -run TestStorageProviderUploadDownload # Run stability test with 10 iterations RUN_STABILITY_TESTS=1 STABILITY_TEST_ITERATIONS=10 \ go test -v -run TestStorageProviderUploadDownloadStability --- testing/storage_integration_test.go | 77 ++++++++++++++++++++++++----- 1 file changed, 64 insertions(+), 13 deletions(-) diff --git a/testing/storage_integration_test.go b/testing/storage_integration_test.go index 33ec244..702176c 100644 --- a/testing/storage_integration_test.go +++ b/testing/storage_integration_test.go @@ -17,7 +17,10 @@ package testing import ( "bytes" "context" + "fmt" "io" + "os" + "strconv" "strings" "testing" "time" @@ -199,15 +202,10 @@ func TestBundleBackupAndRestore(t *testing.T) { } } -// TestStorageProviderUploadDownload tests upload and download operations. -func TestStorageProviderUploadDownload(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test in short mode") - } - - setup := NewIntegrationTestSetup() - setup.Start(t) - defer setup.Stop(t) +// testStorageProviderUploadDownloadOnce performs a single upload/download test cycle. +// This helper function is used by both the regular test and the stability test. +func testStorageProviderUploadDownloadOnce(t *testing.T, setup *IntegrationTestSetup) { + t.Helper() accessKey, secretKey := setup.GetMinioCredentials() @@ -230,9 +228,9 @@ func TestStorageProviderUploadDownload(t *testing.T) { } defer provider.Close() - // Test data + // Test data with timestamp to ensure uniqueness testData := []byte("This is test data for storage provider") - testKey := "test-" + time.Now().Format("20060102-150405") + ".dat" + testKey := "test-" + time.Now().Format("20060102-150405.000000") + ".dat" // Upload (write) writer, err := provider.Writer(ctx, testKey) @@ -249,6 +247,9 @@ func TestStorageProviderUploadDownload(t *testing.T) { t.Logf("Uploaded test data with key: %s", testKey) + // Brief wait to ensure upload completes (helps with timing-sensitive operations) + time.Sleep(50 * time.Millisecond) + // Download (read) reader, err := provider.Reader(ctx, testKey) if err != nil { @@ -265,16 +266,66 @@ func TestStorageProviderUploadDownload(t *testing.T) { downloadedData := downloadBuffer.Bytes() if !bytes.Equal(downloadedData, testData) { t.Errorf("Downloaded data doesn't match. Got %d bytes, want %d bytes", len(downloadedData), len(testData)) + t.Errorf(" Expected: %q", string(testData)) + t.Errorf(" Got: %q", string(downloadedData)) } - t.Log("Successfully uploaded and downloaded data") - // Clean up if err := provider.Delete(ctx, testKey); err != nil { t.Logf("Warning: Failed to clean up test data: %v", err) } } +// TestStorageProviderUploadDownload tests upload and download operations. +func TestStorageProviderUploadDownload(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + + setup := NewIntegrationTestSetup() + setup.Start(t) + defer setup.Stop(t) + + testStorageProviderUploadDownloadOnce(t, setup) + t.Log("Successfully uploaded and downloaded data") +} + +// TestStorageProviderUploadDownloadStability runs multiple iterations to verify test stability. +// This test helps catch race conditions and timing issues that might cause flaky behavior. +func TestStorageProviderUploadDownloadStability(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + + // Skip stability test in CI unless explicitly enabled + if os.Getenv("RUN_STABILITY_TESTS") == "" { + t.Skip("Skipping stability test (set RUN_STABILITY_TESTS=1 to enable)") + } + + setup := NewIntegrationTestSetup() + setup.Start(t) + defer setup.Stop(t) + + iterations := 5 + if iterStr := os.Getenv("STABILITY_TEST_ITERATIONS"); iterStr != "" { + if n, err := strconv.Atoi(iterStr); err == nil && n > 0 { + iterations = n + } + } + + t.Logf("Running stability test with %d iterations", iterations) + + for i := 1; i <= iterations; i++ { + t.Run(fmt.Sprintf("Iteration_%d", i), func(t *testing.T) { + startTime := time.Now() + testStorageProviderUploadDownloadOnce(t, setup) + t.Logf("Iteration %d/%d completed in %v", i, iterations, time.Since(startTime)) + }) + } + + t.Logf("Stability test passed: %d/%d iterations successful", iterations, iterations) +} + // TestStorageHealthCheck tests the storage provider health check. func TestStorageHealthCheck(t *testing.T) { if testing.Short() {