Skip to content

Commit 425bd7e

Browse files
GenPageGenPage
committed
refactor: add tests, clean up command_runner logic
1 parent 49f56d6 commit 425bd7e

12 files changed

+126
-84
lines changed

‎server/events/apply_command_runner.go

+23-25
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ func NewApplyCommandRunner(
1818
dbUpdater *DBUpdater,
1919
db *db.BoltDB,
2020
parallelPoolSize int,
21-
silenceNoProjects bool,
21+
SilenceNoProjects bool,
2222
) *ApplyCommandRunner {
2323
return &ApplyCommandRunner{
2424
vcsClient: vcsClient,
@@ -32,7 +32,7 @@ func NewApplyCommandRunner(
3232
dbUpdater: dbUpdater,
3333
DB: db,
3434
parallelPoolSize: parallelPoolSize,
35-
silenceNoProjects: silenceNoProjects,
35+
SilenceNoProjects: SilenceNoProjects,
3636
}
3737
}
3838

@@ -48,14 +48,34 @@ type ApplyCommandRunner struct {
4848
pullUpdater *PullUpdater
4949
dbUpdater *DBUpdater
5050
parallelPoolSize int
51-
silenceNoProjects bool
51+
// SilenceNoProjects is whether Atlantis should respond to PRs if no projects
52+
// are found
53+
SilenceNoProjects bool
5254
}
5355

5456
func (a *ApplyCommandRunner) Run(ctx *CommandContext, cmd *CommentCommand) {
5557
var err error
5658
baseRepo := ctx.Pull.BaseRepo
5759
pull := ctx.Pull
5860

61+
var projectCmds []models.ProjectCommandContext
62+
projectCmds, err = a.prjCmdBuilder.BuildApplyCommands(ctx, cmd)
63+
64+
if err != nil {
65+
if statusErr := a.commitStatusUpdater.UpdateCombined(ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, cmd.CommandName()); statusErr != nil && !a.SilenceNoProjects {
66+
ctx.Log.Warn("unable to update commit status: %s", statusErr)
67+
}
68+
a.pullUpdater.updatePull(ctx, cmd, CommandResult{Error: err})
69+
return
70+
}
71+
72+
// If there are no projects to apply, don't respond to the PR and ignore
73+
if len(projectCmds) == 0 && a.SilenceNoProjects {
74+
ctx.Log.Info("determined there was no project to run apply in.")
75+
return
76+
}
77+
78+
// At this point we are sure Atlantis has work to do, so set commit status to pending
5979
if a.DisableApply {
6080
ctx.Log.Info("ignoring apply command since apply disabled globally")
6181
if err := a.vcsClient.CreateComment(baseRepo, pull.Num, applyDisabledComment, models.ApplyCommand.String()); err != nil {
@@ -74,23 +94,6 @@ func (a *ApplyCommandRunner) Run(ctx *CommandContext, cmd *CommentCommand) {
7494
return
7595
}
7696

77-
var projectCmds []models.ProjectCommandContext
78-
projectCmds, err = a.prjCmdBuilder.BuildApplyCommands(ctx, cmd)
79-
80-
if err != nil {
81-
if statusErr := a.commitStatusUpdater.UpdateCombined(ctx.Pull.BaseRepo, ctx.Pull, models.FailedCommitStatus, cmd.CommandName()); statusErr != nil {
82-
ctx.Log.Warn("unable to update commit status: %s", statusErr)
83-
}
84-
a.pullUpdater.updatePull(ctx, cmd, CommandResult{Error: err})
85-
return
86-
}
87-
88-
// If there are no projects to apply, don't respond to the PR and ignore
89-
if len(projectCmds) == 0 && a.silenceNoProjects {
90-
ctx.Log.Info("determined there was no project to run apply in.")
91-
return
92-
}
93-
9497
// Get the mergeable status before we set any build statuses of our own.
9598
// We do this here because when we set a "Pending" status, if users have
9699
// required the Atlantis status checks to pass, then we've now changed
@@ -149,11 +152,6 @@ func (a *ApplyCommandRunner) isParallelEnabled(projectCmds []models.ProjectComma
149152
}
150153

151154
func (a *ApplyCommandRunner) updateCommitStatus(ctx *CommandContext, pullStatus models.PullStatus) {
152-
// Don't updateCommitStatus either!
153-
if a.silenceNoProjects {
154-
return
155-
}
156-
157155
var numSuccess int
158156
var numErrored int
159157
status := models.SuccessCommitStatus

‎server/events/approve_policies_command_runner.go

+6-9
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,15 @@ func NewApprovePoliciesCommandRunner(
1212
prjCommandRunner ProjectApprovePoliciesCommandRunner,
1313
pullUpdater *PullUpdater,
1414
dbUpdater *DBUpdater,
15-
silenceNoProjects bool,
15+
SilenceNoProjects bool,
1616
) *ApprovePoliciesCommandRunner {
1717
return &ApprovePoliciesCommandRunner{
1818
commitStatusUpdater: commitStatusUpdater,
1919
prjCmdBuilder: prjCommandBuilder,
2020
prjCmdRunner: prjCommandRunner,
2121
pullUpdater: pullUpdater,
2222
dbUpdater: dbUpdater,
23-
silenceNoProjects: silenceNoProjects,
23+
SilenceNoProjects: SilenceNoProjects,
2424
}
2525
}
2626

@@ -30,7 +30,9 @@ type ApprovePoliciesCommandRunner struct {
3030
dbUpdater *DBUpdater
3131
prjCmdBuilder ProjectApprovePoliciesCommandBuilder
3232
prjCmdRunner ProjectApprovePoliciesCommandRunner
33-
silenceNoProjects bool
33+
// SilenceNoProjects is whether Atlantis should respond to PRs if no projects
34+
// are found
35+
SilenceNoProjects bool
3436
}
3537

3638
func (a *ApprovePoliciesCommandRunner) Run(ctx *CommandContext, cmd *CommentCommand) {
@@ -50,7 +52,7 @@ func (a *ApprovePoliciesCommandRunner) Run(ctx *CommandContext, cmd *CommentComm
5052
return
5153
}
5254

53-
if len(projectCmds) == 0 && a.silenceNoProjects {
55+
if len(projectCmds) == 0 && a.SilenceNoProjects {
5456
ctx.Log.Info("determined there was no project to run approve_policies in")
5557
return
5658
}
@@ -92,11 +94,6 @@ func (a *ApprovePoliciesCommandRunner) buildApprovePolicyCommandResults(ctx *Com
9294
}
9395

9496
func (a *ApprovePoliciesCommandRunner) updateCommitStatus(ctx *CommandContext, pullStatus models.PullStatus) {
95-
// Don't updateCommitStatus either!
96-
if a.silenceNoProjects {
97-
return
98-
}
99-
10097
var numSuccess int
10198
var numErrored int
10299
status := models.SuccessCommitStatus

‎server/events/command_runner_test.go

+31-6
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
105105
}
106106

107107
parallelPoolSize := 1
108-
silenceNoProjects := false
108+
SilenceNoProjects := false
109109
policyCheckCommandRunner = events.NewPolicyCheckCommandRunner(
110110
dbUpdater,
111111
pullUpdater,
@@ -127,7 +127,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
127127
policyCheckCommandRunner,
128128
autoMerger,
129129
parallelPoolSize,
130-
silenceNoProjects,
130+
SilenceNoProjects,
131131
)
132132

133133
applyCommandRunner = events.NewApplyCommandRunner(
@@ -142,7 +142,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
142142
dbUpdater,
143143
defaultBoltDB,
144144
parallelPoolSize,
145-
silenceNoProjects,
145+
SilenceNoProjects,
146146
)
147147

148148
approvePoliciesCommandRunner = events.NewApprovePoliciesCommandRunner(
@@ -151,13 +151,13 @@ func setup(t *testing.T) *vcsmocks.MockClient {
151151
projectCommandRunner,
152152
pullUpdater,
153153
dbUpdater,
154-
silenceNoProjects,
154+
SilenceNoProjects,
155155
)
156156

157157
unlockCommandRunner = events.NewUnlockCommandRunner(
158158
deleteLockCommand,
159159
vcsClient,
160-
silenceNoProjects,
160+
SilenceNoProjects,
161161
)
162162

163163
commentCommandRunnerByCmd := map[models.CommandName]events.CommentCommandRunner{
@@ -281,6 +281,31 @@ func TestRunCommentCommand_ForkPRDisabled_SilenceEnabled(t *testing.T) {
281281
vcsClient.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), AnyString())
282282
}
283283

284+
func TestRunCommentCommand_NoProjects_SilenceEnabled(t *testing.T) {
285+
t.Log("if a command is run on a pull request and SilenceNoProjects is enabled and we are silencing all comments if the modified files don't have a matching project")
286+
vcsClient := setup(t)
287+
planCommandRunner.SilenceNoProjects = true
288+
applyCommandRunner.SilenceNoProjects = true
289+
approvePoliciesCommandRunner.SilenceNoProjects = true
290+
unlockCommandRunner.SilenceNoProjects = true
291+
var pull github.PullRequest
292+
modelPull := models.PullRequest{BaseRepo: fixtures.GithubRepo, State: models.OpenPullState}
293+
When(githubGetter.GetPullRequest(fixtures.GithubRepo, fixtures.Pull.Num)).ThenReturn(&pull, nil)
294+
When(eventParsing.ParseGithubPull(&pull)).ThenReturn(modelPull, modelPull.BaseRepo, fixtures.GithubRepo, nil)
295+
296+
ch.RunCommentCommand(fixtures.GithubRepo, nil, nil, fixtures.User, fixtures.Pull.Num, &events.CommentCommand{Name: models.PlanCommand})
297+
vcsClient.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), AnyString())
298+
299+
ch.RunCommentCommand(fixtures.GithubRepo, nil, nil, fixtures.User, fixtures.Pull.Num, &events.CommentCommand{Name: models.ApplyCommand})
300+
vcsClient.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), AnyString())
301+
302+
ch.RunCommentCommand(fixtures.GithubRepo, nil, nil, fixtures.User, fixtures.Pull.Num, &events.CommentCommand{Name: models.ApprovePoliciesCommand})
303+
vcsClient.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), AnyString())
304+
305+
ch.RunCommentCommand(fixtures.GithubRepo, nil, nil, fixtures.User, fixtures.Pull.Num, &events.CommentCommand{Name: models.UnlockCommand})
306+
vcsClient.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), AnyString())
307+
}
308+
284309
func TestRunCommentCommand_DisableApplyAllDisabled(t *testing.T) {
285310
t.Log("if \"atlantis apply\" is run and this is disabled atlantis should" +
286311
" comment saying that this is not allowed")
@@ -378,7 +403,7 @@ func TestRunUnlockCommandFail_VCSComment(t *testing.T) {
378403
modelPull := models.PullRequest{BaseRepo: fixtures.GithubRepo, State: models.OpenPullState, Num: fixtures.Pull.Num}
379404
When(githubGetter.GetPullRequest(fixtures.GithubRepo, fixtures.Pull.Num)).ThenReturn(pull, nil)
380405
When(eventParsing.ParseGithubPull(pull)).ThenReturn(modelPull, modelPull.BaseRepo, fixtures.GithubRepo, nil)
381-
When(deleteLockCommand.DeleteLocksByPull(fixtures.GithubRepo.FullName, fixtures.Pull.Num)).ThenReturn(errors.New("err"))
406+
When(deleteLockCommand.DeleteLocksByPull(fixtures.GithubRepo.FullName, fixtures.Pull.Num)).ThenReturn(0, errors.New("err"))
382407

383408
ch.RunCommentCommand(fixtures.GithubRepo, &fixtures.GithubRepo, nil, fixtures.User, fixtures.Pull.Num, &events.CommentCommand{Name: models.UnlockCommand})
384409

‎server/events/commit_status_updater.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,7 @@ type CommitStatusUpdater interface {
4141
type DefaultCommitStatusUpdater struct {
4242
Client vcs.Client
4343
// StatusName is the name used to identify Atlantis when creating PR statuses.
44-
StatusName string
45-
SilenceNoProjects bool
44+
StatusName string
4645
}
4746

4847
func (d *DefaultCommitStatusUpdater) UpdateCombined(repo models.Repo, pull models.PullRequest, status models.CommitStatus, command models.CommandName) error {

‎server/events/delete_lock_command.go

+8-7
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212
// DeleteLockCommand is the first step after a command request has been parsed.
1313
type DeleteLockCommand interface {
1414
DeleteLock(id string) (*models.ProjectLock, error)
15-
DeleteLocksByPull(repoFullName string, pullNum int) error
15+
DeleteLocksByPull(repoFullName string, pullNum int) (int, error)
1616
}
1717

1818
// DefaultDeleteLockCommand deletes a specific lock after a request from the LocksController.
@@ -39,22 +39,23 @@ func (l *DefaultDeleteLockCommand) DeleteLock(id string) (*models.ProjectLock, e
3939
}
4040

4141
// DeleteLocksByPull handles deleting all locks for the pull request
42-
func (l *DefaultDeleteLockCommand) DeleteLocksByPull(repoFullName string, pullNum int) error {
42+
func (l *DefaultDeleteLockCommand) DeleteLocksByPull(repoFullName string, pullNum int) (int, error) {
4343
locks, err := l.Locker.UnlockByPull(repoFullName, pullNum)
44+
numLocks := len(locks)
4445
if err != nil {
45-
return err
46+
return numLocks, err
4647
}
47-
if len(locks) == 0 {
48+
if numLocks == 0 {
4849
l.Logger.Debug("No locks found for pull")
49-
return nil
50+
return numLocks, nil
5051
}
5152

52-
for i := 0; i < len(locks); i++ {
53+
for i := 0; i < numLocks; i++ {
5354
lock := locks[i]
5455
l.deleteWorkingDir(lock)
5556
}
5657

57-
return nil
58+
return numLocks, nil
5859
}
5960

6061
func (l *DefaultDeleteLockCommand) deleteWorkingDir(lock models.ProjectLock) {

‎server/events/delete_lock_command_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ func TestDeleteLocksByPull_LockerErr(t *testing.T) {
100100
Locker: l,
101101
Logger: logging.NewNoopLogger(),
102102
}
103-
err := dlc.DeleteLocksByPull(repoName, pullNum)
103+
_, err := dlc.DeleteLocksByPull(repoName, pullNum)
104104
ErrEquals(t, "err", err)
105105
}
106106

@@ -115,7 +115,7 @@ func TestDeleteLocksByPull_None(t *testing.T) {
115115
Locker: l,
116116
Logger: logging.NewNoopLogger(),
117117
}
118-
err := dlc.DeleteLocksByPull(repoName, pullNum)
118+
_, err := dlc.DeleteLocksByPull(repoName, pullNum)
119119
Ok(t, err)
120120
}
121121

@@ -130,6 +130,6 @@ func TestDeleteLocksByPull_OldFormat(t *testing.T) {
130130
Locker: l,
131131
Logger: logging.NewNoopLogger(),
132132
}
133-
err := dlc.DeleteLocksByPull(repoName, pullNum)
133+
_, err := dlc.DeleteLocksByPull(repoName, pullNum)
134134
Ok(t, err)
135135
}

‎server/events/mocks/matchers/ptr_to_models_projectlock.go

+14-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎server/events/mocks/mock_delete_lock_command.go

+13-9
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)