fix: add test and prevent issue if template is not defined (#55)

* add test and prevent issue if template is not defined Signed-off-by: pashavictorovich <pavel@codefresh.io> * better test coverage of msgoptions builder Signed-off-by: pashavictorovich <pavel@codefresh.io>
argoproj · Nov 17, 2021 · 0e1f1ed · 0e1f1ed
1 parent 31da4db
commit 0e1f1ed
Show file tree

Hide file tree

Showing 2 changed files with 91 additions and 27 deletions.
diff --git a/pkg/services/slack.go b/pkg/services/slack.go
@@ -89,42 +89,33 @@ func NewSlackService(opts SlackOptions) NotificationService {
 	return &slackService{opts: opts}
 }
 
-func (s *slackService) Send(notification Notification, dest Destination) error {
-	apiURL := slack.APIURL
-	if s.opts.ApiURL != "" {
-		apiURL = s.opts.ApiURL
-	}
-	transport := httputil.NewTransport(apiURL, s.opts.InsecureSkipVerify)
-	client := &http.Client{
-		Transport: httputil.NewLoggingRoundTripper(transport, log.WithField("service", "slack")),
-	}
-	sl := slack.New(s.opts.Token, slack.OptionHTTPClient(client), slack.OptionAPIURL(apiURL))
+func buildMessageOptions(notification Notification, dest Destination, opts SlackOptions) ([]slack.MsgOption, error) {
 	msgOptions := []slack.MsgOption{slack.MsgOptionText(notification.Message, false)}
-	if s.opts.Username != "" {
-		msgOptions = append(msgOptions, slack.MsgOptionUsername(s.opts.Username))
+	if opts.Username != "" {
+		msgOptions = append(msgOptions, slack.MsgOptionUsername(opts.Username))
 	}
-	if s.opts.Icon != "" {
-		if validIconEmoji.MatchString(s.opts.Icon) {
-			msgOptions = append(msgOptions, slack.MsgOptionIconEmoji(s.opts.Icon))
-		} else if isValidIconURL(s.opts.Icon) {
-			msgOptions = append(msgOptions, slack.MsgOptionIconURL(s.opts.Icon))
+	if opts.Icon != "" {
+		if validIconEmoji.MatchString(opts.Icon) {
+			msgOptions = append(msgOptions, slack.MsgOptionIconEmoji(opts.Icon))
+		} else if isValidIconURL(opts.Icon) {
+			msgOptions = append(msgOptions, slack.MsgOptionIconURL(opts.Icon))
 		} else {
-			log.Warnf("Icon reference '%v' is not a valid emoij or url", s.opts.Icon)
+			log.Warnf("Icon reference '%v' is not a valid emoij or url", opts.Icon)
 		}
 	}
 
 	if notification.Slack != nil {
 		attachments := make([]slack.Attachment, 0)
 		if notification.Slack.Attachments != "" {
 			if err := json.Unmarshal([]byte(notification.Slack.Attachments), &attachments); err != nil {
-				return fmt.Errorf("failed to unmarshal attachments '%s' : %v", notification.Slack.Attachments, err)
+				return nil, fmt.Errorf("failed to unmarshal attachments '%s' : %v", notification.Slack.Attachments, err)
 			}
 		}
 
 		blocks := slack.Blocks{}
 		if notification.Slack.Blocks != "" {
 			if err := json.Unmarshal([]byte(notification.Slack.Blocks), &blocks); err != nil {
-				return fmt.Errorf("failed to unmarshal blocks '%s' : %v", notification.Slack.Blocks, err)
+				return nil, fmt.Errorf("failed to unmarshal blocks '%s' : %v", notification.Slack.Blocks, err)
 			}
 		}
 		msgOptions = append(msgOptions, slack.MsgOptionAttachments(attachments...), slack.MsgOptionBlocks(blocks.BlockSet...))
@@ -134,16 +125,36 @@ func (s *slackService) Send(notification Notification, dest Destination) error {
 		threadTSs[dest.Recipient] = map[string]string{}
 	}
 
-	if notification.Slack.NotifyBroadcast {
-		msgOptions = append(msgOptions, slack.MsgOptionBroadcast())
+	if notification.Slack != nil {
+		if notification.Slack.NotifyBroadcast {
+			msgOptions = append(msgOptions, slack.MsgOptionBroadcast())
+		}
+
+		if lastTs, ok := threadTSs[dest.Recipient][notification.Slack.GroupingKey]; ok && lastTs != "" && notification.Slack.GroupingKey != "" {
+			msgOptions = append(msgOptions, slack.MsgOptionTS(lastTs))
+		}
+	}
+
+	return msgOptions, nil
+}
+
+func (s *slackService) Send(notification Notification, dest Destination) error {
+	apiURL := slack.APIURL
+	if s.opts.ApiURL != "" {
+		apiURL = s.opts.ApiURL
 	}
+	transport := httputil.NewTransport(apiURL, s.opts.InsecureSkipVerify)
+	client := &http.Client{
+		Transport: httputil.NewLoggingRoundTripper(transport, log.WithField("service", "slack")),
+	}
+	sl := slack.New(s.opts.Token, slack.OptionHTTPClient(client), slack.OptionAPIURL(apiURL))
 
-	if lastTs, ok := threadTSs[dest.Recipient][notification.Slack.GroupingKey]; ok && lastTs != "" && notification.Slack.GroupingKey != "" {
-		msgOptions = append(msgOptions, slack.MsgOptionTS(lastTs))
+	msgOptions, err := buildMessageOptions(notification, dest, s.opts)
+	if err != nil {
+		return err
 	}
 
 	ctx := context.TODO()
-	var err error
 	for {
 		err = rateLimiter.Wait(ctx)
 		if err != nil {
@@ -157,8 +168,10 @@ func (s *slackService) Send(notification Notification, dest Destination) error {
 				break
 			}
 		} else {
-			if lastTs, ok := threadTSs[dest.Recipient][notification.Slack.GroupingKey]; !ok || lastTs == "" {
-				threadTSs[dest.Recipient][notification.Slack.GroupingKey] = ts
+			if notification.Slack != nil {
+				if lastTs, ok := threadTSs[dest.Recipient][notification.Slack.GroupingKey]; !ok || lastTs == "" {
+					threadTSs[dest.Recipient][notification.Slack.GroupingKey] = ts
+				}
 			}
 			// No error, so remove rate limit
 			rateLimiter.SetLimit(rate.Inf)

diff --git a/pkg/services/slack_test.go b/pkg/services/slack_test.go
@@ -1,6 +1,8 @@
 package services
 
 import (
+	"reflect"
+	"runtime"
 	"testing"
 	"text/template"
 
@@ -51,3 +53,52 @@ func TestGetTemplater_Slack(t *testing.T) {
 	assert.Equal(t, "hello-world", notification.Slack.GroupingKey)
 	assert.Equal(t, true, notification.Slack.NotifyBroadcast)
 }
+
+func TestBuildMessageOptionsWithNonExistTemplate(t *testing.T) {
+	n := Notification{}
+
+	opts, err := buildMessageOptions(n, Destination{}, SlackOptions{})
+	assert.NoError(t, err)
+	assert.Len(t, opts, 1)
+}
+
+func TestBuildMessageOptionsUsername(t *testing.T) {
+	n := Notification{}
+
+	opts, err := buildMessageOptions(n, Destination{}, SlackOptions{Username: "test-username"})
+	assert.NoError(t, err)
+	assert.Len(t, opts, 2)
+
+	usernameOption := opts[1]
+
+	val := runtime.FuncForPC(reflect.ValueOf(usernameOption).Pointer()).Name()
+	assert.Contains(t, val, "MsgOptionUsername")
+}
+
+func TestBuildMessageOptionsIcon(t *testing.T) {
+	n := Notification{}
+
+	opts, err := buildMessageOptions(n, Destination{}, SlackOptions{Icon: ":+1:"})
+	assert.NoError(t, err)
+	assert.Len(t, opts, 2)
+
+	usernameOption := opts[1]
+
+	val := runtime.FuncForPC(reflect.ValueOf(usernameOption).Pointer()).Name()
+	assert.Contains(t, val, "MsgOptionIconEmoji")
+}
+
+func TestBuildMessageOptionsNotifyBroadcast(t *testing.T) {
+	n := Notification{Slack: &SlackNotification{
+		NotifyBroadcast: true,
+	}}
+
+	opts, err := buildMessageOptions(n, Destination{}, SlackOptions{})
+	assert.NoError(t, err)
+	assert.Len(t, opts, 4)
+
+	usernameOption := opts[3]
+
+	val := runtime.FuncForPC(reflect.ValueOf(usernameOption).Pointer()).Name()
+	assert.Contains(t, val, "MsgOptionBroadcast")
+}