test: reduce flakiness of acceptance tests

The CI environment isn't as performant as local machines: the time
needed to fully initialize the test environment can be significant and
skew the verification. Rather than setting the "virtual" clock used to
measure alert timings at the beginning of the acceptance test, it is
better to wait for the test bed to be ready.

Signed-off-by: Simon Pasquier <[email protected]>

Author: simonpasquier

test/cli/acceptance.go

@@ -92,10 +92,6 @@ func NewAcceptanceTest(t *testing.T, opts *AcceptanceOpts) *AcceptanceTest {
 		opts:    opts,
 		actions: map[float64][]func(){},
 	}
-	// TODO: Should this really be set during creation time? Why not do this
-	// during Run() time, maybe there is something else long happening between
-	// creation and running.
-	opts.baseTime = time.Now()
 
 	return test
 }
@@ -206,6 +202,10 @@ func (t *AcceptanceTest) Run() {
 		t.T.Fatal(err)
 	}
 
+	// Set the reference time right before running the test actions to avoid
+	// test failures due to slow setup of the test environment.
+	t.opts.baseTime = time.Now()
+
 	go t.runActions()
 
 	var latest float64

test/with_api_v1/acceptance.go

@@ -80,7 +80,6 @@ func NewAcceptanceTest(t *testing.T, opts *AcceptanceOpts) *AcceptanceTest {
 		opts:    opts,
 		actions: map[float64][]func(){},
 	}
-	opts.baseTime = time.Now()
 
 	return test
 }
@@ -177,6 +176,10 @@ func (t *AcceptanceTest) Run() {
 		}(am)
 	}
 
+	// Set the reference time right before running the test actions to avoid
+	// test failures due to slow setup of the test environment.
+	t.opts.baseTime = time.Now()
+
 	go t.runActions()
 
 	var latest float64
@@ -333,28 +336,28 @@ func (am *Alertmanager) cleanup() {
 // Push declares alerts that are to be pushed to the Alertmanager
 // server at a relative point in time.
 func (am *Alertmanager) Push(at float64, alerts ...*TestAlert) {
-	var cas []APIV1Alert
-	for i := range alerts {
-		a := alerts[i].nativeAlert(am.opts)
-		al := APIV1Alert{
-			Labels:       LabelSet{},
-			Annotations:  LabelSet{},
-			StartsAt:     a.StartsAt,
-			EndsAt:       a.EndsAt,
-			GeneratorURL: a.GeneratorURL,
-		}
-		for n, v := range a.Labels {
-			al.Labels[LabelName(n)] = LabelValue(v)
-		}
-		for n, v := range a.Annotations {
-			al.Annotations[LabelName(n)] = LabelValue(v)
+	am.t.Do(at, func() {
+		var cas []APIV1Alert
+		for i := range alerts {
+			a := alerts[i].nativeAlert(am.opts)
+			al := APIV1Alert{
+				Labels:       LabelSet{},
+				Annotations:  LabelSet{},
+				StartsAt:     a.StartsAt,
+				EndsAt:       a.EndsAt,
+				GeneratorURL: a.GeneratorURL,
+			}
+			for n, v := range a.Labels {
+				al.Labels[LabelName(n)] = LabelValue(v)
+			}
+			for n, v := range a.Annotations {
+				al.Annotations[LabelName(n)] = LabelValue(v)
+			}
+			cas = append(cas, al)
 		}
-		cas = append(cas, al)
-	}
 
-	alertAPI := NewAlertAPI(am.client)
+		alertAPI := NewAlertAPI(am.client)
 
-	am.t.Do(at, func() {
 		if err := alertAPI.Push(context.Background(), cas...); err != nil {
 			am.t.Errorf("Error pushing %v: %s", cas, err)
 		}

test/with_api_v2/acceptance.go

@@ -83,11 +83,6 @@ func NewAcceptanceTest(t *testing.T, opts *AcceptanceOpts) *AcceptanceTest {
 		opts:    opts,
 		actions: map[float64][]func(){},
 	}
-	// TODO: Should this really be set during creation time? Why not do this
-	// during Run() time, maybe there is something else long happening between
-	// creation and running.
-	opts.baseTime = time.Now()
-
 	return test
 }
 
@@ -185,6 +180,10 @@ func (t *AcceptanceTest) Run() {
 		t.T.Fatal(err)
 	}
 
+	// Set the reference time right before running the test actions to avoid
+	// test failures due to slow setup of the test environment.
+	t.opts.baseTime = time.Now()
+
 	go t.runActions()
 
 	var latest float64
@@ -420,26 +419,26 @@ func (amc *AlertmanagerCluster) Push(at float64, alerts ...*TestAlert) {
 // Push declares alerts that are to be pushed to the Alertmanager
 // server at a relative point in time.
 func (am *Alertmanager) Push(at float64, alerts ...*TestAlert) {
-	var cas models.PostableAlerts
-	for i := range alerts {
-		a := alerts[i].nativeAlert(am.opts)
-		alert := &models.PostableAlert{
-			Alert: models.Alert{
-				Labels:       a.Labels,
-				GeneratorURL: a.GeneratorURL,
-			},
-			Annotations: a.Annotations,
-		}
-		if a.StartsAt != nil {
-			alert.StartsAt = *a.StartsAt
-		}
-		if a.EndsAt != nil {
-			alert.EndsAt = *a.EndsAt
+	am.t.Do(at, func() {
+		var cas models.PostableAlerts
+		for i := range alerts {
+			a := alerts[i].nativeAlert(am.opts)
+			alert := &models.PostableAlert{
+				Alert: models.Alert{
+					Labels:       a.Labels,
+					GeneratorURL: a.GeneratorURL,
+				},
+				Annotations: a.Annotations,
+			}
+			if a.StartsAt != nil {
+				alert.StartsAt = *a.StartsAt
+			}
+			if a.EndsAt != nil {
+				alert.EndsAt = *a.EndsAt
+			}
+			cas = append(cas, alert)
 		}
-		cas = append(cas, alert)
-	}
 
-	am.t.Do(at, func() {
 		params := alert.PostAlertsParams{}
 		params.WithContext(context.Background()).WithAlerts(cas)
 

test/with_api_v2/acceptance/cluster_test.go

@@ -77,15 +77,17 @@ receivers:
   - url: 'http://%s'
 `
 
-	acceptanceOpts := &a.AcceptanceOpts{
-		Tolerance: 2 * time.Second,
+	acceptanceOpts := func() *a.AcceptanceOpts {
+		return &a.AcceptanceOpts{
+			Tolerance: 2 * time.Second,
+		}
 	}
 
 	clusterSizes := []int{1, 3}
 
 	tests := []*a.AcceptanceTest{
-		a.NewAcceptanceTest(t, acceptanceOpts),
-		a.NewAcceptanceTest(t, acceptanceOpts),
+		a.NewAcceptanceTest(t, acceptanceOpts()),
+		a.NewAcceptanceTest(t, acceptanceOpts()),
 	}
 
 	collectors := []*a.Collector{}
@@ -118,7 +120,7 @@ receivers:
 
 	wg.Wait()
 
-	_, err := a.CompareCollectors(collectors[0], collectors[1], acceptanceOpts)
+	_, err := a.CompareCollectors(collectors[0], collectors[1], acceptanceOpts())
 	if err != nil {
 		t.Fatal(err)
 	}