From a5260a0bd87f83b17744445476aaf7f107ca24e4 Mon Sep 17 00:00:00 2001
From: Tom Clegg <tom@curoverse.com>
Date: Thu, 26 May 2016 15:49:49 -0400
Subject: [PATCH] 9272: Fix some race conditions in flaky tests.

---
 services/crunch-run/crunchrun_test.go | 33 ++++++++++++++++++++++++---
 services/crunch-run/logging_test.go   | 23 +++++++++++--------
 2 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/services/crunch-run/crunchrun_test.go b/services/crunch-run/crunchrun_test.go
index 1f2aa201c4..b0646289b2 100644
--- a/services/crunch-run/crunchrun_test.go
+++ b/services/crunch-run/crunchrun_test.go
@@ -18,6 +18,7 @@ import (
 	"os/exec"
 	"sort"
 	"strings"
+	"sync"
 	"syscall"
 	"testing"
 	"time"
@@ -40,6 +41,7 @@ type ArvTestClient struct {
 	ContainerRecord
 	Logs          map[string]*bytes.Buffer
 	WasSetRunning bool
+	sync.Mutex
 }
 
 type KeepTestClient struct {
@@ -130,6 +132,9 @@ func (this *ArvTestClient) Create(resourceType string,
 	parameters arvadosclient.Dict,
 	output interface{}) error {
 
+	this.Mutex.Lock()
+	defer this.Mutex.Unlock()
+
 	this.Calls += 1
 	this.Content = append(this.Content, parameters)
 
@@ -168,13 +173,35 @@ func (this *ArvTestClient) Get(resourceType string, uuid string, parameters arva
 }
 
 func (this *ArvTestClient) Update(resourceType string, uuid string, parameters arvadosclient.Dict, output interface{}) (err error) {
+	this.Mutex.Lock()
+	defer this.Mutex.Unlock()
 	this.Calls += 1
 	this.Content = append(this.Content, parameters)
 	if resourceType == "containers" {
 		if parameters["container"].(arvadosclient.Dict)["state"] == "Running" {
 			this.WasSetRunning = true
 		}
+	}
+	return nil
+}
 
+// CalledWith returns the parameters from the first API call whose
+// parameters match jpath/string. E.g., CalledWith(c, "foo.bar",
+// "baz") returns parameters with parameters["foo"]["bar"]=="baz". If
+// no call matches, it returns nil.
+func (this *ArvTestClient) CalledWith(jpath, expect string) arvadosclient.Dict {
+	call: for _, content := range this.Content {
+		var v interface{} = content
+		for _, k := range strings.Split(jpath, ".") {
+			if dict, ok := v.(arvadosclient.Dict); !ok {
+				continue call
+			} else {
+				v = dict[k]
+			}
+		}
+		if v, ok := v.(string); ok && v == expect {
+			return content
+		}
 	}
 	return nil
 }
@@ -613,7 +640,7 @@ func (s *TestSuite) TestCancel(c *C) {
 
 	go func() {
 		for cr.ContainerID == "" {
-			time.Sleep(1 * time.Second)
+			time.Sleep(time.Millisecond)
 		}
 		cr.SigChan <- syscall.SIGINT
 	}()
@@ -751,10 +778,10 @@ func (s *TestSuite) TestStdout(c *C) {
 		t.finish <- dockerclient.WaitResult{ExitCode: 0}
 	})
 
-	c.Check(api.Calls, Equals, 6)
+	c.Assert(api.Calls, Equals, 6)
 	c.Check(api.Content[5]["container"].(arvadosclient.Dict)["exit_code"], Equals, 0)
 	c.Check(api.Content[5]["container"].(arvadosclient.Dict)["state"], Equals, "Complete")
-	c.Check(api.Content[2]["collection"].(arvadosclient.Dict)["manifest_text"], Equals, "./a/b 307372fa8fd5c146b22ae7a45b49bc31+6 0:6:c.out\n")
+	c.Check(api.CalledWith("collection.manifest_text", "./a/b 307372fa8fd5c146b22ae7a45b49bc31+6 0:6:c.out\n"), Not(IsNil))
 }
 
 // Used by the TestStdoutWithWrongPath*()
diff --git a/services/crunch-run/logging_test.go b/services/crunch-run/logging_test.go
index 79214fca7d..1c8c635e9b 100644
--- a/services/crunch-run/logging_test.go
+++ b/services/crunch-run/logging_test.go
@@ -79,18 +79,21 @@ func (s *LoggingTestSuite) TestWriteMultipleLogs(c *C) {
 	stdout.Print("Doing stuff")
 	cr.CrunchLog.Print("Goodbye")
 	stdout.Print("Blurb")
-
 	cr.CrunchLog.Close()
-	logtext1 := "2015-12-29T15:51:45.000000001Z Hello world!\n" +
-		"2015-12-29T15:51:45.000000003Z Goodbye\n"
-	c.Check(api.Content[0]["log"].(arvadosclient.Dict)["event_type"], Equals, "crunch-run")
-	c.Check(api.Content[0]["log"].(arvadosclient.Dict)["properties"].(map[string]string)["text"], Equals, logtext1)
-
 	stdout.Close()
-	logtext2 := "2015-12-29T15:51:45.000000002Z Doing stuff\n" +
-		"2015-12-29T15:51:45.000000004Z Blurb\n"
-	c.Check(api.Content[1]["log"].(arvadosclient.Dict)["event_type"], Equals, "stdout")
-	c.Check(api.Content[1]["log"].(arvadosclient.Dict)["properties"].(map[string]string)["text"], Equals, logtext2)
+
+	logText := make(map[string]string)
+	for _, content := range api.Content {
+		log := content["log"].(arvadosclient.Dict)
+		logText[log["event_type"].(string)] += log["properties"].(map[string]string)["text"]
+	}
+
+	c.Check(logText["crunch-run"], Equals, `2015-12-29T15:51:45.000000001Z Hello world!
+2015-12-29T15:51:45.000000003Z Goodbye
+`)
+	c.Check(logText["stdout"], Equals, `2015-12-29T15:51:45.000000002Z Doing stuff
+2015-12-29T15:51:45.000000004Z Blurb
+`)
 
 	mt, err := cr.LogCollection.ManifestText()
 	c.Check(err, IsNil)
-- 
2.30.2