Refactor and mock slack.go, fix reviewed changes

server/events/apply_executor.go

@@ -95,7 +95,6 @@ func (a *ApplyExecutor) apply(ctx *CommandContext, repoDir string, plan models.P
 	tfApplyCmd := append(append(append([]string{"apply", "-no-color"}, applyExtraArgs...), ctx.Command.Flags...), plan.LocalPath)
 	output, err := a.Terraform.RunCommandWithVersion(ctx.Log, absolutePath, tfApplyCmd, terraformVersion, env)
 
-	// Send webhooks.
 	a.Webhooks.Send(ctx.Log, webhooks.ApplyResult{
 		Environment: env,
 		User:        ctx.User,

server/events/webhooks/mocks/mock_slack.go

@@ -0,0 +1,151 @@
+// Automatically generated by pegomock. DO NOT EDIT!
+// Source: slack.go
+
+package mocks
+
+import (
+	"reflect"
+
+	pegomock "github.com/petergtz/pegomock"
+)
+
+type MockSlackClient struct {
+	fail func(message string, callerSkip ...int)
+}
+
+func NewMockSlackClient() *MockSlackClient {
+	return &MockSlackClient{fail: pegomock.GlobalFailHandler}
+}
+
+func (mock *MockSlackClient) AuthTest() error {
+	params := []pegomock.Param{}
+	result := pegomock.GetGenericMockFrom(mock).Invoke("AuthTest", params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()})
+	var ret0 error
+	if len(result) != 0 {
+		if result[0] != nil {
+			ret0 = result[0].(error)
+		}
+	}
+	return ret0
+}
+
+func (mock *MockSlackClient) ChannelExist(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()})
+	var ret0 bool
+	var ret1 error
+	if len(result) != 0 {
+		if result[0] != nil {
+			ret0 = result[0].(bool)
+		}
+		if result[1] != nil {
+			ret1 = result[1].(error)
+		}
+	}
+	return ret0, ret1
+}
+
+func (mock *MockSlackClient) PostMessage(channel string, result ApplyResult) error {
+	params := []pegomock.Param{channel, result}
+	result := pegomock.GetGenericMockFrom(mock).Invoke("PostMessage", params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()})
+	var ret0 error
+	if len(result) != 0 {
+		if result[0] != nil {
+			ret0 = result[0].(error)
+		}
+	}
+	return ret0
+}
+
+func (mock *MockSlackClient) VerifyWasCalledOnce() *VerifierSlackClient {
+	return &VerifierSlackClient{mock, pegomock.Times(1), nil}
+}
+
+func (mock *MockSlackClient) VerifyWasCalled(invocationCountMatcher pegomock.Matcher) *VerifierSlackClient {
+	return &VerifierSlackClient{mock, invocationCountMatcher, nil}
+}
+
+func (mock *MockSlackClient) VerifyWasCalledInOrder(invocationCountMatcher pegomock.Matcher, inOrderContext *pegomock.InOrderContext) *VerifierSlackClient {
+	return &VerifierSlackClient{mock, invocationCountMatcher, inOrderContext}
+}
+
+type VerifierSlackClient struct {
+	mock                   *MockSlackClient
+	invocationCountMatcher pegomock.Matcher
+	inOrderContext         *pegomock.InOrderContext
+}
+
+func (verifier *VerifierSlackClient) AuthTest() *SlackClient_AuthTest_OngoingVerification {
+	params := []pegomock.Param{}
+	methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "AuthTest", params)
+	return &SlackClient_AuthTest_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations}
+}
+
+type SlackClient_AuthTest_OngoingVerification struct {
+	mock              *MockSlackClient
+	methodInvocations []pegomock.MethodInvocation
+}
+
+func (c *SlackClient_AuthTest_OngoingVerification) GetCapturedArguments() {
+}
+
+func (c *SlackClient_AuthTest_OngoingVerification) GetAllCapturedArguments() {
+}
+
+func (verifier *VerifierSlackClient) ChannelExist(channelName string) *SlackClient_ChannelExist_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}
+}
+
+type SlackClient_ChannelExist_OngoingVerification struct {
+	mock              *MockSlackClient
+	methodInvocations []pegomock.MethodInvocation
+}
+
+func (c *SlackClient_ChannelExist_OngoingVerification) GetCapturedArguments() string {
+	channelName := c.GetAllCapturedArguments()
+	return channelName[len(channelName)-1]
+}
+
+func (c *SlackClient_ChannelExist_OngoingVerification) GetAllCapturedArguments() (_param0 []string) {
+	params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations)
+	if len(params) > 0 {
+		_param0 = make([]string, len(params[0]))
+		for u, param := range params[0] {
+			_param0[u] = param.(string)
+		}
+	}
+	return
+}
+
+func (verifier *VerifierSlackClient) PostMessage(channel string, result ApplyResult) *SlackClient_PostMessage_OngoingVerification {
+	params := []pegomock.Param{channel, result}
+	methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "PostMessage", params)
+	return &SlackClient_PostMessage_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations}
+}
+
+type SlackClient_PostMessage_OngoingVerification struct {
+	mock              *MockSlackClient
+	methodInvocations []pegomock.MethodInvocation
+}
+
+func (c *SlackClient_PostMessage_OngoingVerification) GetCapturedArguments() (string, ApplyResult) {
+	channel, result := c.GetAllCapturedArguments()
+	return channel[len(channel)-1], result[len(result)-1]
+}
+
+func (c *SlackClient_PostMessage_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []ApplyResult) {
+	params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations)
+	if len(params) > 0 {
+		_param0 = make([]string, len(params[0]))
+		for u, param := range params[0] {
+			_param0[u] = param.(string)
+		}
+		_param1 = make([]ApplyResult, len(params[1]))
+		for u, param := range params[1] {
+			_param1[u] = param.(ApplyResult)
+		}
+	}
+	return
+}

server/events/webhooks/slack.go

@@ -8,67 +8,94 @@ import (
 	"github.com/pkg/errors"
 )
 
+const (
+	successColour = "good"
+	failureColour = "danger"
+)
+
+//go:generate pegomock generate --use-experimental-model-gen --package mocks -o mocks/mock_slack.go slack.go
+
+type SlackClient interface {
+	AuthTest() error
+	ChannelExist(channelName string) (bool, error)
+	PostMessage(channel string, result ApplyResult) error
+}
+
+type ConcreteSlackClient struct {
+	Slack *slack.Client
+}
+
 type SlackWebhook struct {
+	Client   SlackClient
 	EnvRegex *regexp.Regexp
 	Channel  string
-	Token    string
-	Client   *slack.Client
 }
 
-func NewSlack(r *regexp.Regexp, channel string, token string) (*SlackWebhook, error) {
-	slackClient := slack.New(token)
-	if _, err := slackClient.AuthTest(); err != nil {
+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")
 	}
 
-	// Make sure the slack channel exists.
-	channels, err := slackClient.GetChannels(true)
+	channelExist, err := client.ChannelExist(channel)
 	if err != nil {
 		return nil, err
 	}
-	channelExist := false
-	for _, c := range channels {
-		if c.Name == channel {
-			channelExist = true
-			break
-		}
-	}
 	if !channelExist {
 		return nil, errors.Errorf("slack channel %q doesn't exist", channel)
 	}
 
 	return &SlackWebhook{
-		Client:   slackClient,
+		Client:   client,
 		EnvRegex: r,
 		Channel:  channel,
-		Token:    token,
 	}, nil
 }
 
-func (s *SlackWebhook) Send(result ApplyResult) error {
-	if !s.EnvRegex.MatchString(result.Environment) {
-		return 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, result ApplyResult) error {
 	params := slack.NewPostMessageParameters()
-	params.Attachments = s.createAttachments(result)
+	params.Attachments = c.createAttachments(result)
 	params.AsUser = true
 	params.EscapeText = false
-	_, _, err := s.Client.PostMessage(s.Channel, "", params)
+	_, _, err := c.Slack.PostMessage(channel, "", params)
 	return err
 }
 
-func (s *SlackWebhook) createAttachments(result ApplyResult) []slack.Attachment {
-	var color string
+func (c *ConcreteSlackClient) createAttachments(result ApplyResult) []slack.Attachment {
+	var colour string
 	if result.Success {
-		color = "good"
+		colour = successColour
 	} else {
-		color = "danger"
+		colour = failureColour
 	}
 
 	text := fmt.Sprintf("Applied in <%s|%s>.", result.Pull.URL, result.Repo.FullName)
 	attachment := slack.Attachment{
-		Color: color,
+		Color: colour,
 		Text:  text,
 		Fields: []slack.AttachmentField{
 			slack.AttachmentField{
@@ -83,6 +110,12 @@ func (s *SlackWebhook) createAttachments(result ApplyResult) []slack.Attachment
 			},
 		},
 	}
-	var attachments []slack.Attachment
-	return append(attachments, attachment)
+	return []slack.Attachment{attachment}
+}
+
+func (s *SlackWebhook) Send(result ApplyResult) error {
+	if !s.EnvRegex.MatchString(result.Environment) {
+		return nil
+	}
+	return s.Client.PostMessage(s.Channel, result)
 }

server/events/webhooks/slack_test.go

@@ -1,4 +1,39 @@
 package webhooks_test
 
-// todo: actually test
-// purposefully empty to trigger coverage report
+import (
+	"regexp"
+	"strings"
+	"testing"
+
+	"github.com/hootsuite/atlantis/server/events/webhooks"
+	"github.com/hootsuite/atlantis/server/events/webhooks/mocks"
+)
+
+func TestNewWebhooksManager_InvalidToken(t *testing.T) {
+	t.Log("When given an empty slack token and there is a slack webhook config, an error is returned")
+	emptyToken := ""
+	_, err := webhooks.NewWebhooksManager(validConfigs(), webhooks.NewClient(emptyToken))
+	Assert(t, err != nil, "expected error")
+	// Equals(t, "for slack webhooks, slack-token must be set", err.Error())
+	Assert(t, strings.Contains(err.Error(), "testing slack authentication"), "expected auth error")
+}
+
+func TestSend_Success(t *testing.T) {
+	RegisterMockTestingT(t)
+	client := mocks.MockSlackClient()
+	regex, err := regexp.Compile(".*")
+	Ok(t, err)
+	hook := SlackWebhook{
+		Client:   client,
+		EnvRegex: regex,
+		Channel:  "somechannel",
+	}
+	result := webhooks.ApplyResult{
+		Environment: "production"
+	}
+	hook.Send(result)
+}
+
+func TestSend_Error(t *testing.T) {
+
+}

server/events/webhooks/webhooks.go

@@ -1,7 +1,6 @@
 package webhooks
 
 import (
-	"errors"
 	"fmt"
 	"regexp"
 
@@ -12,7 +11,7 @@ import (
 const SlackKind = "slack"
 const ApplyEvent = "apply"
 
-//go:generate pegomock generate --package mocks -o mocks/mock_slack.go slack.go
+//go:generate pegomock generate --use-experimental-model-gen --package mocks -o mocks/mock_webhooks.go webhooks.go
 
 type WebhooksSender interface {
 	Send(log *logging.SimpleLogger, result ApplyResult)
@@ -41,28 +40,26 @@ type Config struct {
 	Channel        string
 }
 
-func NewWebhooksManager(configs []Config, slackToken string) (*WebhooksManager, error) {
+func NewWebhooksManager(configs []Config, client SlackClient) (*WebhooksManager, error) {
 	var webhooks []WebhookSender
 	for _, c := range configs {
 		r, err := regexp.Compile(c.WorkspaceRegex)
 		if err != nil {
 			return nil, err
 		}
 		if c.Event != ApplyEvent {
-			return nil, fmt.Errorf("event: %s not supported. Only event: %s is supported right now", c.Kind, ApplyEvent)
+			return nil, fmt.Errorf("event: %s not supported. Only event: %s is supported right now", c.Event, ApplyEvent)
 		}
-		if c.Kind != SlackKind {
+		switch c.Kind {
+		case SlackKind:
+			slack, err := NewSlack(r, c.Channel, client)
+			if err != nil {
+				return nil, err
+			}
+			webhooks = append(webhooks, slack)
+		default:
 			return nil, fmt.Errorf("kind: %s not supported. Only kind: %s is supported right now", c.Kind, SlackKind)
 		}
-		if slackToken == "" {
-			return nil, errors.New("for slack webhooks, slack-token must be set")
-		}
-
-		slack, err := NewSlack(r, c.Channel, slackToken)
-		if err != nil {
-			return nil, err
-		}
-		webhooks = append(webhooks, slack)
 	}
 
 	return &WebhooksManager{