Enable race detector in tests

Makefile

@@ -1,5 +1,5 @@
 TEST?=./...
-TESTARGS?=
+TESTARGS?=-race
 DEPS = $(shell go list -f '{{range .TestImports}}{{.}} {{end}}' ./...)
 CURRENT_DIR = $(shell pwd)
 SOURCEDIR = $(CURRENT_DIR)

consul/consul_stub.go

@@ -1,11 +1,14 @@
 package consul
 
 import (
+	"sync"
+
 	"github.com/allegro/marathon-consul/apps"
 	consulapi "github.com/hashicorp/consul/api"
 )
 
 type ConsulStub struct {
+	sync.RWMutex
 	services         map[apps.TaskId]*consulapi.AgentServiceRegistration
 	ErrorServices    map[apps.TaskId]error
 	ErrorGetServices map[string]error
@@ -25,7 +28,9 @@ func NewConsulStubWithTag(tag string) *ConsulStub {
 	}
 }
 
-func (c ConsulStub) GetAllServices() ([]*consulapi.CatalogService, error) {
+func (c *ConsulStub) GetAllServices() ([]*consulapi.CatalogService, error) {
+	c.RLock()
+	defer c.RUnlock()
 	var catalog []*consulapi.CatalogService
 	for _, s := range c.services {
 		catalog = append(catalog, &consulapi.CatalogService{
@@ -40,9 +45,11 @@ func (c ConsulStub) GetAllServices() ([]*consulapi.CatalogService, error) {
 	return catalog, nil
 }
 
-func (c ConsulStub) GetServices(name string) ([]*consulapi.CatalogService, error) {
-	if error, ok := c.ErrorGetServices[name]; ok {
-		return nil, error
+func (c *ConsulStub) GetServices(name string) ([]*consulapi.CatalogService, error) {
+	c.RLock()
+	defer c.RUnlock()
+	if err, ok := c.ErrorGetServices[name]; ok {
+		return nil, err
 	}
 	var catalog []*consulapi.CatalogService
 	for _, s := range c.services {
@@ -61,6 +68,8 @@ func (c ConsulStub) GetServices(name string) ([]*consulapi.CatalogService, error
 }
 
 func (c *ConsulStub) Register(task *apps.Task, app *apps.App) error {
+	c.Lock()
+	defer c.Unlock()
 	if err, ok := c.ErrorServices[task.ID]; ok {
 		return err
 	} else {
@@ -78,6 +87,8 @@ func (c *ConsulStub) ServiceName(app *apps.App) string {
 }
 
 func (c *ConsulStub) Deregister(serviceId apps.TaskId, agent string) error {
+	c.Lock()
+	defer c.Unlock()
 	if err, ok := c.ErrorServices[serviceId]; ok {
 		return err
 	} else {
@@ -87,6 +98,8 @@ func (c *ConsulStub) Deregister(serviceId apps.TaskId, agent string) error {
 }
 
 func (c *ConsulStub) RegisteredServicesIds() []string {
+	c.RLock()
+	defer c.RUnlock()
 	services, _ := c.GetAllServices()
 	servicesIds := []string{}
 	for _, consulService := range services {

metrics/metrics_test.go

@@ -11,7 +11,6 @@ import (
 )
 
 func TestMark(t *testing.T) {
-	t.Parallel()
 	// given
 	Init(Config{Target: "stdout", Prefix: ""})
 
@@ -33,8 +32,6 @@ func TestMark(t *testing.T) {
 }
 
 func TestTime(t *testing.T) {
-	t.Parallel()
-
 	// given
 	Init(Config{Target: "stdout", Prefix: ""})
 
@@ -56,8 +53,6 @@ func TestTime(t *testing.T) {
 }
 
 func TestUpdateGauge(t *testing.T) {
-	t.Parallel()
-
 	// given
 	Init(Config{Target: "stdout", Prefix: ""})
 

sync/sync_test.go

@@ -1,19 +1,18 @@
 package sync
 
 import (
+	"fmt"
+	"os"
+	"sync"
 	"testing"
+	"time"
 
+	"github.com/allegro/marathon-consul/apps"
 	"github.com/allegro/marathon-consul/consul"
 	"github.com/allegro/marathon-consul/marathon"
 	. "github.com/allegro/marathon-consul/utils"
 	consulapi "github.com/hashicorp/consul/api"
 	"github.com/stretchr/testify/assert"
-
-	"fmt"
-	"os"
-	"time"
-
-	"github.com/allegro/marathon-consul/apps"
 )
 
 func TestSyncJob_ShouldSyncOnLeadership(t *testing.T) {
@@ -32,10 +31,10 @@ func TestSyncJob_ShouldSyncOnLeadership(t *testing.T) {
 	ticker := sync.StartSyncServicesJob()
 
 	// then
+	ticker.Stop()
 	select {
 	case <-time.After(15 * time.Millisecond):
-		ticker.Stop()
-		assert.Equal(t, 2, services.RegistrationsCount(app.Tasks[0].ID.String()))
+		assert.Equal(t, 1, services.RegistrationsCount(app.Tasks[0].ID.String()))
 	}
 }
 
@@ -56,10 +55,7 @@ func TestSyncJob_ShouldNotSyncWhenDisabled(t *testing.T) {
 
 	// then
 	assert.Nil(t, ticker)
-	select {
-	case <-time.After(15 * time.Millisecond):
-		assert.Equal(t, 0, services.RegistrationsCount(app.Tasks[0].ID.String()))
-	}
+	assert.Equal(t, 0, services.RegistrationsCount(app.Tasks[0].ID.String()))
 }
 
 func TestSyncJob_ShouldDefaultLeaderConfigurationToResolvedHostname(t *testing.T) {
@@ -78,10 +74,10 @@ func TestSyncJob_ShouldDefaultLeaderConfigurationToResolvedHostname(t *testing.T
 	ticker := sync.StartSyncServicesJob()
 
 	// then
+	ticker.Stop()
 	select {
 	case <-time.After(15 * time.Millisecond):
-		ticker.Stop()
-		assert.Equal(t, 2, services.RegistrationsCount(app.Tasks[0].ID.String()))
+		assert.Equal(t, 1, services.RegistrationsCount(app.Tasks[0].ID.String()))
 	}
 }
 
@@ -134,6 +130,7 @@ func TestSyncServices_ShouldSyncOnForceWithoutLeadership(t *testing.T) {
 }
 
 type ConsulServicesMock struct {
+	sync.RWMutex
 	registrations map[string]int
 }
 
@@ -152,11 +149,15 @@ func (c *ConsulServicesMock) GetAllServices() ([]*consulapi.CatalogService, erro
 }
 
 func (c *ConsulServicesMock) Register(task *apps.Task, app *apps.App) error {
+	c.Lock()
+	defer c.Unlock()
 	c.registrations[task.ID.String()]++
 	return nil
 }
 
 func (c *ConsulServicesMock) RegistrationsCount(instanceId string) int {
+	c.RLock()
+	defer c.RUnlock()
 	return c.registrations[instanceId]
 }