test: enable race detection (#1262)

This change enables race detection when running the tests. It also fixes
a couple of existing race conditions.

Makefile

@@ -30,7 +30,7 @@ all: format build test
 
 test:
 	@echo ">> running tests"
-	@$(GO) test -short $(pkgs)
+	@$(GO) test -race -short $(pkgs)
 
 style:
 	@echo ">> checking code style"

dispatch/dispatch_test.go

@@ -192,7 +192,7 @@ func TestAggrGroup(t *testing.T) {
 
 	case batch := <-alertsCh:
 		if s := time.Since(last); s < opts.GroupWait {
-			t.Fatalf("received batch to early after %v", s)
+			t.Fatalf("received batch too early after %v", s)
 		}
 		exp := types.AlertSlice{a1}
 		sort.Sort(batch)
@@ -212,7 +212,7 @@ func TestAggrGroup(t *testing.T) {
 
 		case batch := <-alertsCh:
 			if s := time.Since(last); s < opts.GroupInterval {
-				t.Fatalf("received batch to early after %v", s)
+				t.Fatalf("received batch too early after %v", s)
 			}
 			exp := types.AlertSlice{a1, a3}
 			sort.Sort(batch)
@@ -262,7 +262,7 @@ func TestAggrGroup(t *testing.T) {
 			s := time.Since(last)
 			lastCurMtx.Unlock()
 			if s < opts.GroupInterval {
-				t.Fatalf("received batch to early after %v", s)
+				t.Fatalf("received batch too early after %v", s)
 			}
 			exp := types.AlertSlice{a1, a2, a3}
 			sort.Sort(batch)
@@ -274,9 +274,12 @@ func TestAggrGroup(t *testing.T) {
 	}
 
 	// Resolve all alerts, they should be removed after the next batch was sent.
-	a1.EndsAt = time.Now()
-	a2.EndsAt = time.Now()
-	a3.EndsAt = time.Now()
+	a1r, a2r, a3r := *a1, *a2, *a3
+	resolved := types.AlertSlice{&a1r, &a2r, &a3r}
+	for _, a := range resolved {
+		a.EndsAt = time.Now()
+		ag.insert(a)
+	}
 
 	select {
 	case <-time.After(2 * opts.GroupInterval):
@@ -284,13 +287,12 @@ func TestAggrGroup(t *testing.T) {
 
 	case batch := <-alertsCh:
 		if s := time.Since(last); s < opts.GroupInterval {
-			t.Fatalf("received batch to early after %v", s)
+			t.Fatalf("received batch too early after %v", s)
 		}
-		exp := types.AlertSlice{a1, a2, a3}
 		sort.Sort(batch)
 
-		if !reflect.DeepEqual(batch, exp) {
-			t.Fatalf("expected alerts %v but got %v", exp, batch)
+		if !reflect.DeepEqual(batch, resolved) {
+			t.Fatalf("expected alerts %v but got %v", resolved, batch)
 		}
 
 		if !ag.empty() {

test/acceptance.go

@@ -167,6 +167,8 @@ func (t *AcceptanceTest) Run() {
 		defer func(am *Alertmanager) {
 			am.Terminate()
 			am.cleanup()
+			t.Logf("stdout:\n%v", am.cmd.Stdout)
+			t.Logf("stderr:\n%v", am.cmd.Stderr)
 		}(am)
 	}
 
@@ -192,11 +194,6 @@ func (t *AcceptanceTest) Run() {
 		report := coll.check()
 		t.Log(report)
 	}
-
-	for _, am := range t.ams {
-		t.Logf("stdout:\n%v", am.cmd.Stdout)
-		t.Logf("stderr:\n%v", am.cmd.Stderr)
-	}
 }
 
 // runActions performs the stored actions at the defined times.
@@ -219,6 +216,23 @@ func (t *AcceptanceTest) runActions() {
 	wg.Wait()
 }
 
+type buffer struct {
+	b   bytes.Buffer
+	mtx sync.Mutex
+}
+
+func (b *buffer) Write(p []byte) (int, error) {
+	b.mtx.Lock()
+	defer b.mtx.Unlock()
+	return b.b.Write(p)
+}
+
+func (b *buffer) String() string {
+	b.mtx.Lock()
+	defer b.mtx.Unlock()
+	return b.b.String()
+}
+
 // Alertmanager encapsulates an Alertmanager process and allows
 // declaring alerts being pushed to it at fixed points in time.
 type Alertmanager struct {
@@ -246,7 +260,7 @@ func (am *Alertmanager) Start() {
 	)
 
 	if am.cmd == nil {
-		var outb, errb bytes.Buffer
+		var outb, errb buffer
 		cmd.Stdout = &outb
 		cmd.Stderr = &errb
 	} else {
@@ -344,14 +358,14 @@ func (am *Alertmanager) SetSilence(at float64, sil *TestSilence) {
 			am.t.Errorf("error setting silence %v: %s", sil, err)
 			return
 		}
-		sil.ID = v.Data.SilenceID
+		sil.SetID(v.Data.SilenceID)
 	})
 }
 
 // DelSilence deletes the silence with the sid at the given time.
 func (am *Alertmanager) DelSilence(at float64, sil *TestSilence) {
 	am.t.Do(at, func() {
-		req, err := http.NewRequest("DELETE", fmt.Sprintf("http://%s/api/v1/silence/%s", am.apiAddr, sil.ID), nil)
+		req, err := http.NewRequest("DELETE", fmt.Sprintf("http://%s/api/v1/silence/%s", am.apiAddr, sil.ID()), nil)
 		if err != nil {
 			am.t.Errorf("Error deleting silence %v: %s", sil, err)
 			return

test/collector.go

@@ -15,6 +15,7 @@ package test
 
 import (
 	"fmt"
+	"sync"
 	"testing"
 	"time"
 
@@ -30,6 +31,8 @@ type Collector struct {
 
 	collected map[float64][]model.Alerts
 	expected  map[Interval][]model.Alerts
+
+	mtx sync.RWMutex
 }
 
 func (c *Collector) String() string {
@@ -59,6 +62,8 @@ func batchesEqual(as, bs model.Alerts, opts *AcceptanceOpts) bool {
 // latest returns the latest relative point in time where a notification is
 // expected.
 func (c *Collector) latest() float64 {
+	c.mtx.RLock()
+	defer c.mtx.RUnlock()
 	var latest float64
 	for iv := range c.expected {
 		if iv.end > latest {
@@ -71,6 +76,8 @@ func (c *Collector) latest() float64 {
 // Want declares that the Collector expects to receive the given alerts
 // within the given time boundaries.
 func (c *Collector) Want(iv Interval, alerts ...*TestAlert) {
+	c.mtx.Lock()
+	defer c.mtx.Unlock()
 	var nas model.Alerts
 	for _, a := range alerts {
 		nas = append(nas, a.nativeAlert(c.opts))
@@ -81,6 +88,8 @@ func (c *Collector) Want(iv Interval, alerts ...*TestAlert) {
 
 // add the given alerts to the collected alerts.
 func (c *Collector) add(alerts ...*model.Alert) {
+	c.mtx.Lock()
+	defer c.mtx.Unlock()
 	arrival := c.opts.relativeTime(time.Now())
 
 	c.collected[arrival] = append(c.collected[arrival], model.Alerts(alerts))
@@ -89,6 +98,8 @@ func (c *Collector) add(alerts ...*model.Alert) {
 func (c *Collector) check() string {
 	report := fmt.Sprintf("\ncollector %q:\n\n", c)
 
+	c.mtx.RLock()
+	defer c.mtx.RUnlock()
 	for iv, expected := range c.expected {
 		report += fmt.Sprintf("interval %v\n", iv)
 

test/mock.go

@@ -19,6 +19,7 @@ import (
 	"net"
 	"net/http"
 	"reflect"
+	"sync"
 	"time"
 
 	"github.com/prometheus/common/model"
@@ -53,10 +54,12 @@ func Between(start, end float64) Interval {
 
 // TestSilence models a model.Silence with relative times.
 type TestSilence struct {
-	ID               string
+	id               string
 	match            []string
 	matchRE          []string
 	startsAt, endsAt float64
+
+	mtx sync.RWMutex
 }
 
 // Silence creates a new TestSilence active for the relative interval given
@@ -83,6 +86,20 @@ func (s *TestSilence) MatchRE(v ...string) *TestSilence {
 	return s
 }
 
+// SetID sets the silence ID.
+func (s *TestSilence) SetID(ID string) {
+	s.mtx.Lock()
+	defer s.mtx.Unlock()
+	s.id = ID
+}
+
+// ID gets the silence ID.
+func (s *TestSilence) ID() string {
+	s.mtx.RLock()
+	defer s.mtx.RUnlock()
+	return s.id
+}
+
 // nativeSilence converts the declared test silence into a regular
 // silence with resolved times.
 func (s *TestSilence) nativeSilence(opts *AcceptanceOpts) *types.Silence {