From 9fc8bedb5667d24d3a3c7843dc28a229efffb1e6 Mon Sep 17 00:00:00 2001 From: Kyle Evans Date: Thu, 7 May 2020 19:23:52 -0500 Subject: [PATCH] archiver: tests: integrate new WaitForCompletion a little better We can use this to wait for archives to come in, rather than spinning and hoping with a timeout. --- services/archiver/archiver_test.go | 34 ++++++++++-------------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/services/archiver/archiver_test.go b/services/archiver/archiver_test.go index 77f9199788..e2bcc3da9e 100644 --- a/services/archiver/archiver_test.go +++ b/services/archiver/archiver_test.go @@ -49,27 +49,23 @@ func waitForCount(t *testing.T, num int) { } func releaseOneEntry(t *testing.T, inFlight []*ArchiveRequest) { - var nowQueued, numQueued int + var numQueued int numQueued = len(archiveInProgress) - // Release one, then wait up to 10 seconds for it to complete. - queueMutex.Lock() + // Release one, then WaitForCompletion. We'll get signalled when ready. + // This works out to be quick and race-free, as we'll get signalled when the + // archival goroutine proceeds to dequeue the now-complete archive but we + // can't pick up the queue lock again until it's done removing it from + // archiveInProgress. We'll remain waiting on the queue lock in + // WaitForCompletion() until we can safely acquire the lock. + LockQueue() archiveQueueReleaseCond.Signal() - queueMutex.Unlock() - timeout := time.Now().Add(10 * time.Second) - for { - nowQueued = len(archiveInProgress) - if nowQueued != numQueued || time.Now().After(timeout) { - break - } - } - - // Make sure we didn't just timeout. - assert.NotEqual(t, numQueued, nowQueued) + WaitForCompletion() + UnlockQueue() // Also make sure that we released only one. - assert.Equal(t, numQueued-1, nowQueued) + assert.Equal(t, numQueued-1, len(archiveInProgress)) } func TestArchive_Basic(t *testing.T) { @@ -187,15 +183,7 @@ func TestArchive_Basic(t *testing.T) { assert.Equal(t, 2, len(archiveInProgress)) releaseOneEntry(t, inFlight) assert.Equal(t, 1, len(archiveInProgress)) - - // Test waiting for completion on one, which should be relatively - // straightforward. We'll hold the queue-lock and release an entry. It will - // wait to acquire the queue lock, which we'll drop when we - // WaitForCompletion(), to be woken up later. - LockQueue() releaseOneEntry(t, inFlight) - WaitForCompletion() - UnlockQueue() assert.Equal(t, 0, len(archiveInProgress)) zipReq2 = DeriveRequestFrom(ctx, firstCommit+".zip")