Split slack.go into slack_client.go, add new mocks & tests

Also renamed Concrete -> Default, Exist -> Exists

server/events/webhooks/mocks/mock_slack.go → server/events/webhooks/mocks/mock_slack_client.go

@@ -29,9 +29,9 @@ func (mock *MockSlackClient) AuthTest() error {
 	return ret0
 }
 
-func (mock *MockSlackClient) ChannelExist(channelName string) (bool, error) {
+func (mock *MockSlackClient) ChannelExists(channelName string) (bool, error) {
 	params := []pegomock.Param{channelName}
-	result := pegomock.GetGenericMockFrom(mock).Invoke("ChannelExist", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()})
+	result := pegomock.GetGenericMockFrom(mock).Invoke("ChannelExists", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()})
 	var ret0 bool
 	var ret1 error
 	if len(result) != 0 {
@@ -92,23 +92,23 @@ func (c *SlackClient_AuthTest_OngoingVerification) GetCapturedArguments() {
 func (c *SlackClient_AuthTest_OngoingVerification) GetAllCapturedArguments() {
 }
 
-func (verifier *VerifierSlackClient) ChannelExist(channelName string) *SlackClient_ChannelExist_OngoingVerification {
+func (verifier *VerifierSlackClient) ChannelExists(channelName string) *SlackClient_ChannelExists_OngoingVerification {
 	params := []pegomock.Param{channelName}
-	methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "ChannelExist", params)
-	return &SlackClient_ChannelExist_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations}
+	methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "ChannelExists", params)
+	return &SlackClient_ChannelExists_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations}
 }
 
-type SlackClient_ChannelExist_OngoingVerification struct {
+type SlackClient_ChannelExists_OngoingVerification struct {
 	mock              *MockSlackClient
 	methodInvocations []pegomock.MethodInvocation
 }
 
-func (c *SlackClient_ChannelExist_OngoingVerification) GetCapturedArguments() string {
+func (c *SlackClient_ChannelExists_OngoingVerification) GetCapturedArguments() string {
 	channelName := c.GetAllCapturedArguments()
 	return channelName[len(channelName)-1]
 }
 
-func (c *SlackClient_ChannelExist_OngoingVerification) GetAllCapturedArguments() (_param0 []string) {
+func (c *SlackClient_ChannelExists_OngoingVerification) GetAllCapturedArguments() (_param0 []string) {
 	params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations)
 	if len(params) > 0 {
 		_param0 = make([]string, len(params[0]))

server/events/webhooks/slack.go

@@ -1,52 +1,27 @@
 package webhooks
 
 import (
-	"fmt"
 	"regexp"
 
-	"github.com/nlopes/slack"
 	"github.com/pkg/errors"
 )
 
-const (
-	successColour = "good"
-	failureColour = "danger"
-)
-
-//go:generate pegomock generate --use-experimental-model-gen --package mocks -o mocks/mock_slack.go SlackClient
-
-type SlackClient interface {
-	AuthTest() error
-	ChannelExist(channelName string) (bool, error)
-	PostMessage(channel string, applyResult ApplyResult) error
-}
-
-type ConcreteSlackClient struct {
-	Slack *slack.Client
-}
-
 type SlackWebhook struct {
 	Client   SlackClient
 	EnvRegex *regexp.Regexp
 	Channel  string
 }
 
-func NewSlackClient(token string) SlackClient {
-	return &ConcreteSlackClient{
-		Slack: slack.New(token),
-	}
-}
-
 func NewSlack(r *regexp.Regexp, channel string, client SlackClient) (*SlackWebhook, error) {
 	if err := client.AuthTest(); err != nil {
 		return nil, errors.Wrap(err, "testing slack authentication")
 	}
 
-	channelExist, err := client.ChannelExist(channel)
+	channelExists, err := client.ChannelExists(channel)
 	if err != nil {
 		return nil, err
 	}
-	if !channelExist {
+	if !channelExists {
 		return nil, errors.Errorf("slack channel %q doesn't exist", channel)
 	}
 
@@ -57,62 +32,6 @@ func NewSlack(r *regexp.Regexp, channel string, client SlackClient) (*SlackWebho
 	}, nil
 }
 
-func (c *ConcreteSlackClient) AuthTest() error {
-	_, err := c.Slack.AuthTest()
-	return err
-}
-
-func (c *ConcreteSlackClient) ChannelExist(channelName string) (bool, error) {
-	channels, err := c.Slack.GetChannels(true)
-	if err != nil {
-		return false, err
-	}
-
-	for _, channel := range channels {
-		if channel.Name == channelName {
-			return true, nil
-		}
-	}
-	return false, nil
-}
-
-func (c *ConcreteSlackClient) PostMessage(channel string, applyResult ApplyResult) error {
-	params := slack.NewPostMessageParameters()
-	params.Attachments = c.createAttachments(applyResult)
-	params.AsUser = true
-	params.EscapeText = false
-	_, _, err := c.Slack.PostMessage(channel, "", params)
-	return err
-}
-
-func (c *ConcreteSlackClient) createAttachments(applyResult ApplyResult) []slack.Attachment {
-	var colour string
-	if applyResult.Success {
-		colour = successColour
-	} else {
-		colour = failureColour
-	}
-
-	text := fmt.Sprintf("Applied in <%s|%s>.", applyResult.Pull.URL, applyResult.Repo.FullName)
-	attachment := slack.Attachment{
-		Color: colour,
-		Text:  text,
-		Fields: []slack.AttachmentField{
-			slack.AttachmentField{
-				Title: "Environment",
-				Value: applyResult.Environment,
-				Short: true,
-			},
-			slack.AttachmentField{
-				Title: "User",
-				Value: applyResult.User.Username,
-				Short: true,
-			},
-		},
-	}
-	return []slack.Attachment{attachment}
-}
-
 func (s *SlackWebhook) Send(applyResult ApplyResult) error {
 	if !s.EnvRegex.MatchString(applyResult.Environment) {
 		return nil

server/events/webhooks/slack_client.go

@@ -0,0 +1,86 @@
+package webhooks
+
+import (
+	"fmt"
+
+	"github.com/nlopes/slack"
+)
+
+const (
+	slackSuccessColour = "good"
+	slackFailureColour = "danger"
+)
+
+//go:generate pegomock generate --use-experimental-model-gen --package mocks -o mocks/mock_slack_client.go SlackClient
+
+type SlackClient interface {
+	AuthTest() error
+	ChannelExists(channelName string) (bool, error)
+	PostMessage(channel string, applyResult ApplyResult) error
+}
+
+type DefaultSlackClient struct {
+	Slack *slack.Client
+}
+
+func NewSlackClient(token string) SlackClient {
+	return &DefaultSlackClient{
+		Slack: slack.New(token),
+	}
+}
+
+func (d *DefaultSlackClient) AuthTest() error {
+	_, err := d.Slack.AuthTest()
+	return err
+}
+
+func (d *DefaultSlackClient) ChannelExists(channelName string) (bool, error) {
+	channels, err := d.Slack.GetChannels(true)
+	if err != nil {
+		return false, err
+	}
+
+	for _, channel := range channels {
+		if channel.Name == channelName {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
+func (d *DefaultSlackClient) PostMessage(channel string, applyResult ApplyResult) error {
+	params := slack.NewPostMessageParameters()
+	params.Attachments = d.createAttachments(applyResult)
+	params.AsUser = true
+	params.EscapeText = false
+	_, _, err := d.Slack.PostMessage(channel, "", params)
+	return err
+}
+
+func (d *DefaultSlackClient) createAttachments(applyResult ApplyResult) []slack.Attachment {
+	var colour string
+	if applyResult.Success {
+		colour = slackSuccessColour
+	} else {
+		colour = slackFailureColour
+	}
+
+	text := fmt.Sprintf("Applied in <%s|%s>.", applyResult.Pull.URL, applyResult.Repo.FullName)
+	attachment := slack.Attachment{
+		Color: colour,
+		Text:  text,
+		Fields: []slack.AttachmentField{
+			slack.AttachmentField{
+				Title: "Environment",
+				Value: applyResult.Environment,
+				Short: true,
+			},
+			slack.AttachmentField{
+				Title: "User",
+				Value: applyResult.User.Username,
+				Short: true,
+			},
+		},
+	}
+	return []slack.Attachment{attachment}
+}

server/events/webhooks/slack_client_test.go

@@ -0,0 +1,65 @@
+package webhooks_test
+
+import (
+	"testing"
+
+	"github.com/hootsuite/atlantis/server/events/models"
+	"github.com/hootsuite/atlantis/server/events/webhooks"
+
+	. "github.com/hootsuite/atlantis/testing"
+)
+
+const invalidSlackToken = "invalidtoken"
+
+func TestNewSlackClient_Success(t *testing.T) {
+	t.Log("NewSlackClient should always return a non-nil client")
+	client := webhooks.NewSlackClient(invalidSlackToken)
+	Assert(t, client != nil, "SlackClient shouldn't be nil")
+
+	client = webhooks.NewSlackClient("")
+	Assert(t, client != nil, "SlackClient shouldn't be nil")
+}
+
+func TestAuthTest_Error(t *testing.T) {
+	t.Log("When a SlackClient is created with an invalid token, AuthTest should error")
+	client := webhooks.NewSlackClient(invalidSlackToken)
+	err := client.AuthTest()
+	Assert(t, err != nil, "expected error")
+}
+
+func TestChannelExists_Error(t *testing.T) {
+	t.Log("When a SlackClient is created with an invalid token, ChannelExists should error")
+	client := webhooks.NewSlackClient(invalidSlackToken)
+	_, err := client.ChannelExists("somechannel")
+	Assert(t, err != nil, "expected error")
+}
+
+func TestPostMessage_Error(t *testing.T) {
+	t.Log("When a SlackClient is created with an invalid token, PostMessage should error")
+	client := webhooks.NewSlackClient(invalidSlackToken)
+	// todo: ?make this ApplyResult a fixture
+	result := webhooks.ApplyResult{
+		Environment: "production",
+		Repo: models.Repo{
+			CloneURL:          "https://user:password@github.com/hootsuite/atlantis.git",
+			FullName:          "hootsuite/atlantis",
+			Owner:             "hootsuite",
+			SanitizedCloneURL: "https://github.com/hootsuite/atlantis.git",
+			Name:              "atlantis",
+		},
+		Pull: models.PullRequest{
+			Num:        1,
+			HeadCommit: "16ca62f65c18ff456c6ef4cacc8d4826e264bb17",
+			Branch:     "branch",
+			Author:     "lkysow",
+			URL:        "url",
+			BaseCommit: "8ed0280678d49d42cd286610aabcfceb5bb673c6",
+		},
+		User: models.User{
+			Username: "lkysow",
+		},
+		Success: true,
+	}
+	err := client.PostMessage("somechannel", result)
+	Assert(t, err != nil, "expected error")
+}

server/events/webhooks/slack_test.go

@@ -39,14 +39,16 @@ func TestSend_NoopSuccess(t *testing.T) {
 	regex, err := regexp.Compile("weirdemv")
 	Ok(t, err)
 
+	channel := "somechannel"
 	hook := webhooks.SlackWebhook{
 		Client:   client,
 		EnvRegex: regex,
-		Channel:  "somechannel",
+		Channel:  channel,
 	}
 	result := webhooks.ApplyResult{
 		Environment: "production",
 	}
 	err = hook.Send(result)
 	Ok(t, err)
+	client.VerifyWasCalled(Never()).PostMessage(channel, result)
 }

server/events/webhooks/webhooks_test.go

@@ -33,7 +33,7 @@ func TestNewWebhooksManager_InvalidRegex(t *testing.T) {
 	t.Log("When given an invalid regex in a config, an error is returned")
 	RegisterMockTestingT(t)
 	client := mocks.NewMockSlackClient()
-	When(client.ChannelExist(validChannel)).ThenReturn(true, nil)
+	When(client.ChannelExists(validChannel)).ThenReturn(true, nil)
 
 	invalidRegex := "("
 	configs := validConfigs()
@@ -47,7 +47,7 @@ func TestNewWebhooksManager_UnsupportedEvent(t *testing.T) {
 	t.Log("When given an unsupported event in a config, an error is returned")
 	RegisterMockTestingT(t)
 	client := mocks.NewMockSlackClient()
-	When(client.ChannelExist(validChannel)).ThenReturn(true, nil)
+	When(client.ChannelExists(validChannel)).ThenReturn(true, nil)
 
 	unsupportedEvent := "badevent"
 	configs := validConfigs()
@@ -61,7 +61,7 @@ func TestNewWebhooksManager_UnsupportedKind(t *testing.T) {
 	t.Log("When given an unsupported kind in a config, an error is returned")
 	RegisterMockTestingT(t)
 	client := mocks.NewMockSlackClient()
-	When(client.ChannelExist(validChannel)).ThenReturn(true, nil)
+	When(client.ChannelExists(validChannel)).ThenReturn(true, nil)
 
 	unsupportedKind := "badkind"
 	configs := validConfigs()
@@ -91,7 +91,7 @@ func TestNewWebhooksManager_SingleConfigSuccess(t *testing.T) {
 	t.Log("When there is one valid config, function should succeed")
 	RegisterMockTestingT(t)
 	client := mocks.NewMockSlackClient()
-	When(client.ChannelExist(validChannel)).ThenReturn(true, nil)
+	When(client.ChannelExists(validChannel)).ThenReturn(true, nil)
 
 	configs := validConfigs()
 	m, err := webhooks.NewWebhooksManager(configs, client)
@@ -104,7 +104,7 @@ func TestNewWebhooksManager_MultipleConfigSuccess(t *testing.T) {
 	t.Log("When there are multiple valid configs, function should succeed")
 	RegisterMockTestingT(t)
 	client := mocks.NewMockSlackClient()
-	When(client.ChannelExist(validChannel)).ThenReturn(true, nil)
+	When(client.ChannelExists(validChannel)).ThenReturn(true, nil)
 
 	var configs []webhooks.Config
 	nConfigs := 5