From bd14322ffa4ecd7415a669ba9c5bc82bfddc5a04 Mon Sep 17 00:00:00 2001 From: Derek Su Date: Tue, 2 Jul 2024 16:08:40 +0000 Subject: [PATCH] fix(test): fix failed test cases due to race condition Longhorn 8880 Signed-off-by: Derek Su --- sparse/test/client_test.go | 74 +++++++++++++++++++++++++++++++++----- sparse/test/fiemap_test.go | 8 ++++- sparse/test/ssync_test.go | 17 +++++++-- 3 files changed, 88 insertions(+), 11 deletions(-) diff --git a/sparse/test/client_test.go b/sparse/test/client_test.go index 2575b72..cbb6eea 100644 --- a/sparse/test/client_test.go +++ b/sparse/test/client_test.go @@ -4,6 +4,7 @@ import ( "context" "crypto/rand" "os" + "sync" "testing" "github.com/stretchr/testify/assert" @@ -251,11 +252,20 @@ func TestSyncAnyFile(t *testing.T) { func testSyncAnyFile(t *testing.T, src, dst string, directIO, fastSync bool) { // Sync + var wg sync.WaitGroup + wg.Add(1) go func() { - _ = rest.TestServer(context.Background(), port, dst, timeout) + defer wg.Done() + + err := rest.TestServer(context.Background(), port, dst, timeout) + // http server is closed by the client after file transfer is done, so the error + // "http: Server closed" is expected. + assert.True(t, err == nil || err.Error() == "http: Server closed", "Unexpected error: %v", err) }() err := SyncFile(src, localhost+":"+port, timeout, directIO, fastSync) + wg.Wait() + // Verify if err != nil { t.Fatal("sync error") @@ -271,12 +281,20 @@ func testSyncAnyFile(t *testing.T, src, dst string, directIO, fastSync bool) { func testSyncAnyFileExpectFailure(t *testing.T, src, dst string, directIO, fastSync bool) { // Sync + var wg sync.WaitGroup + wg.Add(1) go func() { + defer wg.Done() + err := rest.TestServer(context.Background(), port, dst, timeout) - assert.Nil(t, err) + // http server is closed by the client after file transfer is done, so the error + // "http: Server closed" is expected. + assert.True(t, err == nil || err.Error() == "http: Server closed", "Unexpected error: %v", err) }() err := SyncFile(src, localhost+":"+port, timeout, directIO, fastSync) + wg.Wait() + // Verify if err == nil { t.Fatal("sync error") @@ -733,12 +751,20 @@ func testSyncFile(t *testing.T, layoutLocal, layoutRemote []FileInterval, direct } // Sync + var wg sync.WaitGroup + wg.Add(1) go func() { + defer wg.Done() + err := rest.TestServer(context.Background(), port, remotePath, timeout) - assert.Nil(t, err) + // http server is closed by the client after file transfer is done, so the error + // "http: Server closed" is expected. + assert.True(t, err == nil || err.Error() == "http: Server closed", "Unexpected error: %v", err) }() err := SyncFile(localPath, localhost+":"+port, timeout, true /* directIO */, false /* fastSync */) + wg.Wait() + // Verify if err != nil { t.Fatal("sync error") @@ -775,37 +801,61 @@ func Benchmark_1G_InitFiles(b *testing.B) { } func Benchmark_1G_SendFiles_Whole(b *testing.B) { + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + err := rest.TestServer(context.Background(), port, remoteBigPath, timeout) - assert.Nil(b, err) + // http server is closed by the client after file transfer is done, so the error + // "http: Server closed" is expected. + assert.True(b, err == nil || err.Error() == "http: Server closed", "Unexpected error: %v", err) }() err := SyncFile(localBigPath, localhost+":"+port, timeout, true /* directIO */, false /* fastSync */) + wg.Wait() + if err != nil { b.Fatal("sync error") } } func Benchmark_1G_SendFiles_Whole_No_DirectIO(b *testing.B) { + var wg sync.WaitGroup + wg.Add(1) go func() { + defer wg.Done() + err := rest.TestServer(context.Background(), port, remoteBigPath, timeout) - assert.Nil(b, err) + // http server is closed by the client after file transfer is done, so the error + // "http: Server closed" is expected. + assert.True(b, err == nil || err.Error() == "http: Server closed", "Unexpected error: %v", err) }() err := SyncFile(localBigPath, localhost+":"+port, timeout, false /* directIO */, false /* fastSync */) + wg.Wait() + if err != nil { b.Fatal("sync error") } } func Benchmark_1G_SendFiles_Diff(b *testing.B) { - + var wg sync.WaitGroup + wg.Add(1) go func() { + defer wg.Done() + err := rest.TestServer(context.Background(), port, remoteBigPath, timeout) - assert.Nil(b, err) + // http server is closed by the client after file transfer is done, so the error + // "http: Server closed" is expected. + assert.True(b, err == nil || err.Error() == "http: Server closed", "Unexpected error: %v", err) }() err := SyncFile(localBigPath, localhost+":"+port, timeout, true /* directIO */, false /* fastSync */) + wg.Wait() + if err != nil { b.Fatal("sync error") } @@ -867,12 +917,20 @@ func TestSyncSnapshotZeroByte(t *testing.T) { func testSyncAnyContent(t *testing.T, snapshotName string, dstFileName string, rw ReaderWriterAt, snapshotSize int64) { // Sync + var wg sync.WaitGroup + wg.Add(1) go func() { + defer wg.Done() + err := rest.TestServer(context.Background(), port, dstFileName, timeout) - assert.Nil(t, err) + // http server is closed by the client after file transfer is done, so the error + // "http: Server closed" is expected. + assert.True(t, err == nil || err.Error() == "http: Server closed", "Unexpected error: %v", err) }() err := SyncContent(snapshotName, rw, snapshotSize, localhost+":"+port, timeout, true, false) + wg.Wait() + // Verify if err != nil { t.Fatalf("sync error: %v", err) diff --git a/sparse/test/fiemap_test.go b/sparse/test/fiemap_test.go index ddb47ea..5660b7a 100644 --- a/sparse/test/fiemap_test.go +++ b/sparse/test/fiemap_test.go @@ -6,6 +6,7 @@ import ( "math/rand" "os" "path/filepath" + "sync" "testing" "time" @@ -54,9 +55,13 @@ func TestFileSync(t *testing.T) { // defer fileCleanup(dstPath) log.Info("Syncing file...") startTime := time.Now() + var wg sync.WaitGroup + wg.Add(1) go func() { + defer wg.Done() + err := rest.TestServer(context.Background(), port, dstPath, timeout) - assert.Nil(t, err) + assert.True(t, err == nil || err.Error() == "http: Server closed", "Unexpected error: %v", err) }() time.Sleep(time.Second) err := SyncFile(srcPath, localhost+":"+port, timeout, true, false) @@ -64,6 +69,7 @@ func TestFileSync(t *testing.T) { t.Fatalf("sync error: %v", err) } log.Infof("Syncing done, size: %v elapsed: %.2fs", testFileSize, time.Since(startTime).Seconds()) + wg.Wait() startTime = time.Now() log.Info("Checking...") diff --git a/sparse/test/ssync_test.go b/sparse/test/ssync_test.go index c050b24..6784587 100644 --- a/sparse/test/ssync_test.go +++ b/sparse/test/ssync_test.go @@ -8,6 +8,7 @@ import ( "os" "strconv" "strings" + "sync" "testing" "time" @@ -198,10 +199,15 @@ func RandomSync(t *testing.T, size, seed int64, srcPath, dstPath string, dstCrea } log.Infof("Syncing with directIO: %v size: %v", directIO, size) + var wg sync.WaitGroup + wg.Add(1) go func() { + defer wg.Done() + err := rest.TestServer(context.Background(), port, dstPath, timeout) - assert.Nil(t, err) + assert.True(t, err == nil || err.Error() == "http: Server closed", "Unexpected error: %v", err) }() + startTime := time.Now() err := SyncFile(srcPath, localhost+":"+port, timeout, directIO, fastSync) if err != nil { @@ -209,6 +215,8 @@ func RandomSync(t *testing.T, size, seed int64, srcPath, dstPath string, dstCrea } log.Infof("Syncing done, size: %v elapsed: %.2fs", size, time.Since(startTime).Seconds()) + wg.Wait() + startTime = time.Now() err = checkSparseFiles(srcPath, dstPath) if err != nil { @@ -264,9 +272,13 @@ func TestSyncCancellation(t *testing.T) { dstName := tempFilePath(dstPrefix) ctx, cancelFunc := context.WithCancel(context.Background()) + var wg sync.WaitGroup + wg.Add(1) go func() { + defer wg.Done() + err := rest.TestServer(ctx, port, dstName, timeout) - assert.Nil(t, err) + assert.True(t, err == nil || err.Error() == "http: Server closed", "Unexpected error: %v", err) }() client := http.Client{} @@ -311,4 +323,5 @@ func TestSyncCancellation(t *testing.T) { if httpErr == nil || !strings.Contains(httpErr.Error(), "connection refused") { t.Fatalf("Unexpected error: %v", httpErr) } + wg.Wait() }