Split status checks

e2e/e2e.go

@@ -166,7 +166,7 @@ func getAtlantisStatus(t *E2ETester, branchName string) (string, error) {
 	}
 
 	for _, status := range combinedStatus.Statuses {
-		if status.GetContext() == "Atlantis" {
+		if status.GetContext() == "plan/atlantis" {
 			return status.GetState(), nil
 		}
 	}

server/events/command_runner.go

@@ -91,18 +91,26 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo
 	if !c.validateCtxAndComment(ctx) {
 		return
 	}
-	if err := c.CommitStatusUpdater.Update(ctx.BaseRepo, ctx.Pull, models.PendingCommitStatus, models.PlanCommand); err != nil {
+
+	if err := c.CommitStatusUpdater.UpdateCombined(ctx.BaseRepo, ctx.Pull, models.PendingCommitStatus, models.PlanCommand); err != nil {
 		ctx.Log.Warn("unable to update commit status: %s", err)
 	}
 
 	projectCmds, err := c.ProjectCommandBuilder.BuildAutoplanCommands(ctx)
 	if err != nil {
+		if statusErr := c.CommitStatusUpdater.UpdateCombined(ctx.BaseRepo, ctx.Pull, models.FailedCommitStatus, models.PlanCommand); statusErr != nil {
+			ctx.Log.Warn("unable to update commit status: %s", statusErr)
+		}
+
 		c.updatePull(ctx, AutoplanCommand{}, CommandResult{Error: err})
 		return
 	}
 	if len(projectCmds) == 0 {
 		log.Info("determined there was no project to run plan in")
-		if err := c.CommitStatusUpdater.Update(baseRepo, pull, models.SuccessCommitStatus, models.PlanCommand); err != nil {
+		// If there were no projects modified, we set a successful commit status
+		// with 0/0 projects planned successfully because we've already set an
+		// in-progress status and we don't want that to be "in progress" forever.
+		if err := c.CommitStatusUpdater.UpdateCombinedCount(baseRepo, pull, models.SuccessCommitStatus, models.PlanCommand, 0, 0); err != nil {
 			ctx.Log.Warn("unable to update commit status: %s", err)
 		}
 		return
@@ -115,10 +123,12 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo
 		result.PlansDeleted = true
 	}
 	c.updatePull(ctx, AutoplanCommand{}, result)
-	_, err = c.updateDB(ctx, ctx.Pull, result.ProjectResults)
+	pullStatus, err := c.updateDB(ctx, ctx.Pull, result.ProjectResults)
 	if err != nil {
 		c.Logger.Err("writing results: %s", err)
 	}
+
+	c.updateCommitStatus(ctx, models.PlanCommand, pullStatus)
 }
 
 // RunCommentCommand executes the command.
@@ -184,7 +194,7 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead
 		ctx.Log.Info("pull request mergeable status: %t", ctx.PullMergeable)
 	}
 
-	if err = c.CommitStatusUpdater.Update(baseRepo, pull, models.PendingCommitStatus, cmd.CommandName()); err != nil {
+	if err = c.CommitStatusUpdater.UpdateCombined(baseRepo, pull, models.PendingCommitStatus, cmd.CommandName()); err != nil {
 		ctx.Log.Warn("unable to update commit status: %s", err)
 	}
 
@@ -199,6 +209,9 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead
 		return
 	}
 	if err != nil {
+		if statusErr := c.CommitStatusUpdater.UpdateCombined(ctx.BaseRepo, ctx.Pull, models.FailedCommitStatus, cmd.CommandName()); statusErr != nil {
+			ctx.Log.Warn("unable to update commit status: %s", statusErr)
+		}
 		c.updatePull(ctx, cmd, CommandResult{Error: err})
 		return
 	}
@@ -220,12 +233,46 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead
 		return
 	}
 
+	c.updateCommitStatus(ctx, cmd.Name, pullStatus)
+
 	if cmd.Name == models.ApplyCommand && c.automergeEnabled(ctx, projectCmds) {
 		c.automerge(ctx, pullStatus)
 	}
 }
 
-func (c *DefaultCommandRunner) automerge(ctx *CommandContext, pullStatus *models.PullStatus) {
+func (c *DefaultCommandRunner) updateCommitStatus(ctx *CommandContext, cmd models.CommandName, pullStatus models.PullStatus) {
+	var numSuccess int
+	var status models.CommitStatus
+
+	if cmd == models.PlanCommand {
+		// We consider anything that isn't a plan error as a plan success.
+		// For example, if there is an apply error, that means that at least a
+		// plan was generated successfully.
+		numSuccess = len(pullStatus.Projects) - pullStatus.StatusCount(models.ErroredPlanStatus)
+		status = models.SuccessCommitStatus
+		if numSuccess != len(pullStatus.Projects) {
+			status = models.FailedCommitStatus
+		}
+	} else {
+		numSuccess = pullStatus.StatusCount(models.AppliedPlanStatus)
+
+		numErrored := pullStatus.StatusCount(models.ErroredApplyStatus)
+		status = models.SuccessCommitStatus
+		if numErrored > 0 {
+			status = models.FailedCommitStatus
+		} else if numSuccess < len(pullStatus.Projects) {
+			// If there are plans that haven't been applied yet, we'll use a pending
+			// status.
+			status = models.PendingCommitStatus
+		}
+	}
+
+	if err := c.CommitStatusUpdater.UpdateCombinedCount(ctx.BaseRepo, ctx.Pull, status, cmd, numSuccess, len(pullStatus.Projects)); err != nil {
+		ctx.Log.Warn("unable to update commit status: %s", err)
+	}
+}
+
+func (c *DefaultCommandRunner) automerge(ctx *CommandContext, pullStatus models.PullStatus) {
 	// We only automerge if all projects have been successfully applied.
 	for _, p := range pullStatus.Projects {
 		if p.Status != models.AppliedPlanStatus {
@@ -328,10 +375,6 @@ func (c *DefaultCommandRunner) updatePull(ctx *CommandContext, command PullComma
 		ctx.Log.Warn(res.Failure)
 	}
 
-	// Update the pull request's status icon and comment back.
-	if err := c.CommitStatusUpdater.UpdateProjectResult(ctx, command.CommandName(), res); err != nil {
-		ctx.Log.Warn("unable to update commit status: %s", err)
-	}
 	comment := c.MarkdownRenderer.Render(res, command.CommandName(), ctx.Log.History.String(), command.IsVerbose(), ctx.BaseRepo.VCSHost.Type)
 	if err := c.VCSClient.CreateComment(ctx.BaseRepo, ctx.Pull.Num, comment); err != nil {
 		ctx.Log.Err("unable to comment: %s", err)
@@ -364,7 +407,7 @@ func (c *DefaultCommandRunner) deletePlans(ctx *CommandContext) {
 	}
 }
 
-func (c *DefaultCommandRunner) updateDB(ctx *CommandContext, pull models.PullRequest, results []models.ProjectResult) (*models.PullStatus, error) {
+func (c *DefaultCommandRunner) updateDB(ctx *CommandContext, pull models.PullRequest, results []models.ProjectResult) (models.PullStatus, error) {
 	// Filter out results that errored due to the directory not existing. We
 	// don't store these in the database because they would never be "applyable"
 	// and so the pull request would always have errors.

server/events/command_runner_internal_test.go

@@ -0,0 +1,145 @@
+package events
+
+import (
+	"github.com/runatlantis/atlantis/server/events/models"
+	. "github.com/runatlantis/atlantis/testing"
+	"testing"
+)
+
+func TestUpdateCommitStatus(t *testing.T) {
+	cases := map[string]struct {
+		cmd           models.CommandName
+		pullStatus    models.PullStatus
+		expStatus     models.CommitStatus
+		expNumSuccess int
+		expNumTotal   int
+	}{
+		"single plan success": {
+			cmd: models.PlanCommand,
+			pullStatus: models.PullStatus{
+				Projects: []models.ProjectStatus{
+					{
+						Status: models.PlannedPlanStatus,
+					},
+				},
+			},
+			expStatus:     models.SuccessCommitStatus,
+			expNumSuccess: 1,
+			expNumTotal:   1,
+		},
+		"one plan error, other errors": {
+			cmd: models.PlanCommand,
+			pullStatus: models.PullStatus{
+				Projects: []models.ProjectStatus{
+					{
+						Status: models.ErroredPlanStatus,
+					},
+					{
+						Status: models.PlannedPlanStatus,
+					},
+					{
+						Status: models.AppliedPlanStatus,
+					},
+					{
+						Status: models.ErroredApplyStatus,
+					},
+				},
+			},
+			expStatus:     models.FailedCommitStatus,
+			expNumSuccess: 3,
+			expNumTotal:   4,
+		},
+		"apply, one pending": {
+			cmd: models.ApplyCommand,
+			pullStatus: models.PullStatus{
+				Projects: []models.ProjectStatus{
+					{
+						Status: models.PlannedPlanStatus,
+					},
+					{
+						Status: models.AppliedPlanStatus,
+					},
+				},
+			},
+			expStatus:     models.PendingCommitStatus,
+			expNumSuccess: 1,
+			expNumTotal:   2,
+		},
+		"apply, all successful": {
+			cmd: models.ApplyCommand,
+			pullStatus: models.PullStatus{
+				Projects: []models.ProjectStatus{
+					{
+						Status: models.AppliedPlanStatus,
+					},
+					{
+						Status: models.AppliedPlanStatus,
+					},
+				},
+			},
+			expStatus:     models.SuccessCommitStatus,
+			expNumSuccess: 2,
+			expNumTotal:   2,
+		},
+		"apply, one errored, one pending": {
+			cmd: models.ApplyCommand,
+			pullStatus: models.PullStatus{
+				Projects: []models.ProjectStatus{
+					{
+						Status: models.AppliedPlanStatus,
+					},
+					{
+						Status: models.ErroredApplyStatus,
+					},
+					{
+						Status: models.PlannedPlanStatus,
+					},
+				},
+			},
+			expStatus:     models.FailedCommitStatus,
+			expNumSuccess: 1,
+			expNumTotal:   3,
+		},
+	}
+
+	for name, c := range cases {
+		t.Run(name, func(t *testing.T) {
+			csu := &MockCSU{}
+			cr := &DefaultCommandRunner{
+				CommitStatusUpdater: csu,
+			}
+			cr.updateCommitStatus(&CommandContext{}, c.cmd, c.pullStatus)
+			Equals(t, models.Repo{}, csu.CalledRepo)
+			Equals(t, models.PullRequest{}, csu.CalledPull)
+			Equals(t, c.expStatus, csu.CalledStatus)
+			Equals(t, c.cmd, csu.CalledCommand)
+			Equals(t, c.expNumSuccess, csu.CalledNumSuccess)
+			Equals(t, c.expNumTotal, csu.CalledNumTotal)
+		})
+	}
+}
+
+type MockCSU struct {
+	CalledRepo       models.Repo
+	CalledPull       models.PullRequest
+	CalledStatus     models.CommitStatus
+	CalledCommand    models.CommandName
+	CalledNumSuccess int
+	CalledNumTotal   int
+}
+
+func (m *MockCSU) UpdateCombinedCount(repo models.Repo, pull models.PullRequest, status models.CommitStatus, command models.CommandName, numSuccess int, numTotal int) error {
+	m.CalledRepo = repo
+	m.CalledPull = pull
+	m.CalledStatus = status
+	m.CalledCommand = command
+	m.CalledNumSuccess = numSuccess
+	m.CalledNumTotal = numTotal
+	return nil
+}
+func (m *MockCSU) UpdateCombined(repo models.Repo, pull models.PullRequest, status models.CommitStatus, command models.CommandName) error {
+	return nil
+}
+func (m *MockCSU) UpdateProject(ctx models.ProjectCommandContext, cmdName models.CommandName, status models.CommitStatus, url string) error {
+	return nil
+}

server/events/command_runner_test.go

@@ -36,7 +36,6 @@ import (
 var projectCommandBuilder *mocks.MockProjectCommandBuilder
 var projectCommandRunner *mocks.MockProjectCommandRunner
 var eventParsing *mocks.MockEventParsing
-var ghStatus *mocks.MockCommitStatusUpdater
 var githubGetter *mocks.MockGithubPullGetter
 var gitlabGetter *mocks.MockGitlabMergeRequestGetter
 var ch events.DefaultCommandRunner
@@ -48,7 +47,6 @@ func setup(t *testing.T) *vcsmocks.MockClient {
 	RegisterMockTestingT(t)
 	projectCommandBuilder = mocks.NewMockProjectCommandBuilder()
 	eventParsing = mocks.NewMockEventParsing()
-	ghStatus = mocks.NewMockCommitStatusUpdater()
 	vcsClient := vcsmocks.NewMockClient()
 	githubGetter = mocks.NewMockGithubPullGetter()
 	gitlabGetter = mocks.NewMockGitlabMergeRequestGetter()
@@ -62,7 +60,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
 		ThenReturn(pullLogger)
 	ch = events.DefaultCommandRunner{
 		VCSClient:                vcsClient,
-		CommitStatusUpdater:      ghStatus,
+		CommitStatusUpdater:      &events.DefaultCommitStatusUpdater{vcsClient},
 		EventParser:              eventParsing,
 		MarkdownRenderer:         &events.MarkdownRenderer{},
 		GithubPullGetter:         githubGetter,

server/events/commit_status_updater.go

@@ -26,12 +26,12 @@ import (
 // CommitStatusUpdater updates the status of a commit with the VCS host. We set
 // the status to signify whether the plan/apply succeeds.
 type CommitStatusUpdater interface {
-	// Update updates the status of the head commit of pull.
-	Update(repo models.Repo, pull models.PullRequest, status models.CommitStatus, command models.CommandName) error
-	// UpdateProjectResult updates the status of the head commit given the
-	// state of response.
-	// todo: rename this so it doesn't conflict with UpdateProject
-	UpdateProjectResult(ctx *CommandContext, commandName models.CommandName, res CommandResult) error
+	// UpdateCombined updates the combined status of the head commit of pull.
+	// A combined status represents all the projects modified in the pull.
+	UpdateCombined(repo models.Repo, pull models.PullRequest, status models.CommitStatus, command models.CommandName) error
+	// UpdateCombinedCount updates the combined status to reflect the
+	// numSuccess out of numTotal.
+	UpdateCombinedCount(repo models.Repo, pull models.PullRequest, status models.CommitStatus, command models.CommandName, numSuccess int, numTotal int) error
 	// UpdateProject sets the commit status for the project represented by
 	// ctx.
 	UpdateProject(ctx models.ProjectCommandContext, cmdName models.CommandName, status models.CommitStatus, url string) error
@@ -42,25 +42,28 @@ type DefaultCommitStatusUpdater struct {
 	Client vcs.Client
 }
 
-// Update updates the commit status.
-func (d *DefaultCommitStatusUpdater) Update(repo models.Repo, pull models.PullRequest, status models.CommitStatus, command models.CommandName) error {
-	description := fmt.Sprintf("%s %s", strings.Title(command.String()), strings.Title(status.String()))
-	return d.Client.UpdateStatus(repo, pull, status, "Atlantis", description, "")
+func (d *DefaultCommitStatusUpdater) UpdateCombined(repo models.Repo, pull models.PullRequest, status models.CommitStatus, command models.CommandName) error {
+	src := fmt.Sprintf("%s/atlantis", command.String())
+	var descripWords string
+	switch status {
+	case models.PendingCommitStatus:
+		descripWords = "in progress..."
+	case models.FailedCommitStatus:
+		descripWords = "failed."
+	case models.SuccessCommitStatus:
+		descripWords = "succeeded."
+	}
+	descrip := fmt.Sprintf("%s %s", strings.Title(command.String()), descripWords)
+	return d.Client.UpdateStatus(repo, pull, status, src, descrip, "")
 }
 
-// UpdateProjectResult updates the commit status based on the status of res.
-func (d *DefaultCommitStatusUpdater) UpdateProjectResult(ctx *CommandContext, commandName models.CommandName, res CommandResult) error {
-	var status models.CommitStatus
-	if res.Error != nil || res.Failure != "" {
-		status = models.FailedCommitStatus
-	} else {
-		var statuses []models.CommitStatus
-		for _, p := range res.ProjectResults {
-			statuses = append(statuses, p.Status())
-		}
-		status = d.worstStatus(statuses)
+func (d *DefaultCommitStatusUpdater) UpdateCombinedCount(repo models.Repo, pull models.PullRequest, status models.CommitStatus, command models.CommandName, numSuccess int, numTotal int) error {
+	src := fmt.Sprintf("%s/atlantis", command.String())
+	cmdVerb := "planned"
+	if command == models.ApplyCommand {
+		cmdVerb = "applied"
 	}
-	return d.Update(ctx.BaseRepo, ctx.Pull, status, commandName)
+	return d.Client.UpdateStatus(repo, pull, status, src, fmt.Sprintf("%d/%d projects %s successfully.", numSuccess, numTotal, cmdVerb), "")
 }
 
 func (d *DefaultCommitStatusUpdater) UpdateProject(ctx models.ProjectCommandContext, cmdName models.CommandName, status models.CommitStatus, url string) error {
@@ -81,12 +84,3 @@ func (d *DefaultCommitStatusUpdater) UpdateProject(ctx models.ProjectCommandCont
 	descrip := fmt.Sprintf("%s %s", strings.Title(cmdName.String()), descripWords)
 	return d.Client.UpdateStatus(ctx.BaseRepo, ctx.Pull, status, src, descrip, url)
 }
-
-func (d *DefaultCommitStatusUpdater) worstStatus(ss []models.CommitStatus) models.CommitStatus {
-	for _, s := range ss {
-		if s == models.FailedCommitStatus {
-			return models.FailedCommitStatus
-		}
-	}
-	return models.SuccessCommitStatus
-}