From 8278c7bf7736a14398febc71dfa26497442d9710 Mon Sep 17 00:00:00 2001
From: Ganesh Vernekar <15064823+codesome@users.noreply.github.com>
Date: Fri, 28 May 2021 15:24:18 +0530
Subject: [PATCH] Alerting: Fix flaky TestNotificationChannels take 2 (#34880)

* Alerting: Fix flaky TestNotificationChannels take 2

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Fix nits

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>

* Fix lint

Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
---
 .../alerting/api_notification_channel_test.go | 72 ++++++++++++-------
 1 file changed, 46 insertions(+), 26 deletions(-)

diff --git a/pkg/tests/api/alerting/api_notification_channel_test.go b/pkg/tests/api/alerting/api_notification_channel_test.go
index 98c516d9891e7..11806c824ab0e 100644
--- a/pkg/tests/api/alerting/api_notification_channel_test.go
+++ b/pkg/tests/api/alerting/api_notification_channel_test.go
@@ -1,9 +1,13 @@
 package alerting
 
 import (
+	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
+	"io"
 	"io/ioutil"
+	"mime/multipart"
 	"net/http"
 	"regexp"
 	"strings"
@@ -12,7 +16,6 @@ import (
 	"time"
 
 	"github.com/prometheus/common/model"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/grafana/pkg/bus"
@@ -104,10 +107,11 @@ func TestNotificationChannels(t *testing.T) {
 	// Eventually, we'll get all the desired alerts.
 	// nolint:gosec
 	require.Eventually(t, func() bool {
-		return mockChannel.totalNotifications() == len(alertNames) &&
-			mockChannel.matchesExpNotifications(expNotifications)
+		return mockChannel.totalNotifications() == len(alertNames)
 	}, 30*time.Second, 1*time.Second)
 
+	mockChannel.matchesExpNotifications(t, expNotifications)
+
 	require.NoError(t, mockChannel.Close())
 }
 
@@ -234,20 +238,17 @@ func (nc *mockNotificationChannel) totalNotifications() int {
 	return total
 }
 
-func (nc *mockNotificationChannel) matchesExpNotifications(exp map[string][]string) bool {
-	nc.t.Helper()
+func (nc *mockNotificationChannel) matchesExpNotifications(t *testing.T, exp map[string][]string) {
+	t.Helper()
 	nc.receivedNotificationsMtx.Lock()
 	defer nc.receivedNotificationsMtx.Unlock()
 
-	if len(nc.receivedNotifications) != len(exp) {
-		return false
-	}
+	require.Len(t, nc.receivedNotifications, len(exp))
 
 	for expKey, expVals := range exp {
 		actVals, ok := nc.receivedNotifications[expKey]
-		if !ok || len(actVals) != len(expVals) {
-			return false
-		}
+		require.True(t, ok)
+		require.Len(t, actVals, len(expVals))
 		for i := range expVals {
 			expVal := expVals[i]
 			var r1, r2 *regexp.Regexp
@@ -279,11 +280,11 @@ func (nc *mockNotificationChannel) matchesExpNotifications(exp map[string][]stri
 			}
 			if r1 != nil {
 				parts := r1.FindStringSubmatch(actVals[i])
-				require.Equal(nc.t, 2, len(parts))
+				require.Len(t, parts, 2)
 				if expKey == "v1/alerts" {
 					// 2 fields for Prometheus Alertmanager.
 					parts2 := r2.FindStringSubmatch(actVals[i])
-					require.Equal(nc.t, 2, len(parts2))
+					require.Len(t, parts2, 2)
 					expVal = fmt.Sprintf(expVal, parts[1], parts2[1])
 				} else {
 					expVal = fmt.Sprintf(expVal, parts[1])
@@ -291,24 +292,43 @@ func (nc *mockNotificationChannel) matchesExpNotifications(exp map[string][]stri
 			}
 
 			switch expKey {
-			case "pushover_recv/pushover_test", "telegram_recv/bot6sh027hs034h",
-				"line_recv/line_test", "threema_recv/threema_test":
-				// Multipart data or POST parameters.
-				if expVal != actVals[i] {
-					return false
-				}
+			case "line_recv/line_test", "threema_recv/threema_test":
+				// POST parameters.
+				require.Equal(t, expVal, actVals[i])
+			case "pushover_recv/pushover_test", "telegram_recv/bot6sh027hs034h":
+				// Multipart data.
+				multipartEqual(t, expVal, actVals[i])
 			default:
-				var expJson, actJson interface{}
-				require.NoError(nc.t, json.Unmarshal([]byte(expVal), &expJson))
-				require.NoError(nc.t, json.Unmarshal([]byte(actVals[i]), &actJson))
-				if !assert.ObjectsAreEqual(expJson, actJson) {
-					return false
-				}
+				require.JSONEq(t, expVal, actVals[i])
 			}
 		}
 	}
+}
+
+func multipartEqual(t *testing.T, exp, act string) {
+	t.Helper()
+
+	fillMap := func(r *multipart.Reader, m map[string]string) {
+		for {
+			part, err := r.NextPart()
+			if part == nil || errors.Is(err, io.EOF) {
+				break
+			}
+			require.NoError(t, err)
+			buf := new(bytes.Buffer)
+			_, err = buf.ReadFrom(part)
+			require.NoError(t, err)
+			m[part.FormName()] = buf.String()
+		}
+	}
+
+	expReader := multipart.NewReader(strings.NewReader(exp), channels.GetBoundary())
+	actReader := multipart.NewReader(strings.NewReader(act), channels.GetBoundary())
+	expMap, actMap := make(map[string]string), make(map[string]string)
+	fillMap(expReader, expMap)
+	fillMap(actReader, actMap)
 
-	return true
+	require.Equal(t, expMap, actMap)
 }
 
 func (nc *mockNotificationChannel) Close() error {