From 66f84c7549436bdbbb4dfe2883fd47d9be159ba0 Mon Sep 17 00:00:00 2001
From: Michael Bridgen <michael@weave.works>
Date: Fri, 16 Feb 2018 16:13:16 +0000
Subject: [PATCH] Reorder mock daemon start to avoid data race

We modify the mock cluster after having started the daemon loop, and
the latter may call the former in its own goroutine.
---
 daemon/daemon_test.go | 58 +++++++++++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 22 deletions(-)

diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go
index 1ca12d1f9f..87bcdd5c80 100644
--- a/daemon/daemon_test.go
+++ b/daemon/daemon_test.go
@@ -50,7 +50,8 @@ var (
 
 // When I ping, I should get a response
 func TestDaemon_Ping(t *testing.T) {
-	d, clean, _, _ := mockDaemon(t)
+	d, start, clean, _, _ := mockDaemon(t)
+	start()
 	defer clean()
 	ctx := context.Background()
 	if d.Ping(ctx) != nil {
@@ -60,7 +61,8 @@ func TestDaemon_Ping(t *testing.T) {
 
 // When I ask a version, I should get a version
 func TestDaemon_Version(t *testing.T) {
-	d, clean, _, _ := mockDaemon(t)
+	d, start, clean, _, _ := mockDaemon(t)
+	start()
 	defer clean()
 
 	ctx := context.Background()
@@ -75,7 +77,8 @@ func TestDaemon_Version(t *testing.T) {
 
 // When I export it should export the current (mocked) k8s cluster
 func TestDaemon_Export(t *testing.T) {
-	d, clean, _, _ := mockDaemon(t)
+	d, start, clean, _, _ := mockDaemon(t)
+	start()
 	defer clean()
 
 	ctx := context.Background()
@@ -91,7 +94,8 @@ func TestDaemon_Export(t *testing.T) {
 
 // When I call list services, it should list all the services
 func TestDaemon_ListServices(t *testing.T) {
-	d, clean, _, _ := mockDaemon(t)
+	d, start, clean, _, _ := mockDaemon(t)
+	start()
 	defer clean()
 
 	ctx := context.Background()
@@ -126,7 +130,8 @@ func TestDaemon_ListServices(t *testing.T) {
 
 // When I call list images for a service, it should return images
 func TestDaemon_ListImages(t *testing.T) {
-	d, clean, _, _ := mockDaemon(t)
+	d, start, clean, _, _ := mockDaemon(t)
+	start()
 	defer clean()
 
 	ctx := context.Background()
@@ -156,10 +161,9 @@ func TestDaemon_ListImages(t *testing.T) {
 
 // When I call notify, it should cause a sync
 func TestDaemon_NotifyChange(t *testing.T) {
-	d, clean, mockK8s, events := mockDaemon(t)
-	defer clean()
-	w := newWait(t)
+	d, start, clean, mockK8s, events := mockDaemon(t)
 
+	w := newWait(t)
 	ctx := context.Background()
 
 	var syncCalled int
@@ -173,6 +177,9 @@ func TestDaemon_NotifyChange(t *testing.T) {
 		return nil
 	}
 
+	start()
+	defer clean()
+
 	d.NotifyChange(ctx, api.Change{Kind: api.GitChange, Source: api.GitUpdate{}})
 	w.Eventually(func() bool {
 		syncMu.Lock()
@@ -208,7 +215,8 @@ func TestDaemon_NotifyChange(t *testing.T) {
 // When I ask about a Job, it should tell me about a job
 // When I perform a release, it should update the git repo
 func TestDaemon_Release(t *testing.T) {
-	d, clean, _, _ := mockDaemon(t)
+	d, start, clean, _, _ := mockDaemon(t)
+	start()
 	defer clean()
 	w := newWait(t)
 
@@ -262,7 +270,8 @@ func TestDaemon_Release(t *testing.T) {
 // When I update a policy, I expect it to add to the queue
 // When I update a policy, it should add an annotation to the manifest
 func TestDaemon_PolicyUpdate(t *testing.T) {
-	d, clean, _, _ := mockDaemon(t)
+	d, start, clean, _, _ := mockDaemon(t)
+	start()
 	defer clean()
 	w := newWait(t)
 
@@ -293,7 +302,8 @@ func TestDaemon_PolicyUpdate(t *testing.T) {
 // that is about to take place. Then it should return empty once it is
 // complete
 func TestDaemon_SyncStatus(t *testing.T) {
-	d, clean, _, _ := mockDaemon(t)
+	d, start, clean, _, _ := mockDaemon(t)
+	start()
 	defer clean()
 	w := newWait(t)
 
@@ -313,7 +323,8 @@ func TestDaemon_SyncStatus(t *testing.T) {
 
 // When I restart fluxd, there won't be any jobs in the cache
 func TestDaemon_JobStatusWithNoCache(t *testing.T) {
-	d, clean, _, _ := mockDaemon(t)
+	d, start, clean, _, _ := mockDaemon(t)
+	start()
 	defer clean()
 	w := newWait(t)
 
@@ -336,7 +347,7 @@ func makeImageInfo(ref string, t time.Time) image.Info {
 	return image.Info{ID: r, CreatedAt: t}
 }
 
-func mockDaemon(t *testing.T) (*Daemon, func(), *cluster.Mock, *mockEventWriter) {
+func mockDaemon(t *testing.T) (*Daemon, func(), func(), *cluster.Mock, *mockEventWriter) {
 	logger := log.NewNopLogger()
 
 	singleService := cluster.Controller{
@@ -421,14 +432,10 @@ func mockDaemon(t *testing.T) (*Daemon, func(), *cluster.Mock, *mockEventWriter)
 
 	events := &mockEventWriter{}
 
-	// Shutdown chans and waitgroups
+	// Shutdown chan and waitgroups
 	shutdown := make(chan struct{})
 	wg := &sync.WaitGroup{}
 
-	wg.Add(1)
-	go repo.Start(shutdown, wg)
-	gittest.WaitForRepoReady(repo, t)
-
 	// Jobs queue (starts itself)
 	jobs := job.NewQueue(shutdown, wg)
 
@@ -447,15 +454,22 @@ func mockDaemon(t *testing.T) (*Daemon, func(), *cluster.Mock, *mockEventWriter)
 		LoopVars:       &LoopVars{},
 	}
 
-	wg.Add(1)
-	go d.Loop(shutdown, wg, logger)
+	start := func() {
+		wg.Add(1)
+		go repo.Start(shutdown, wg)
+		gittest.WaitForRepoReady(repo, t)
+
+		wg.Add(1)
+		go d.Loop(shutdown, wg, logger)
+	}
 
-	return d, func() {
+	stop := func() {
 		// Close daemon first so we don't get errors if the queue closes before the daemon
 		close(shutdown)
 		wg.Wait()
 		repoCleanup()
-	}, k8s, events
+	}
+	return d, start, stop, k8s, events
 }
 
 type mockEventWriter struct {