Skip to content

Commit

Permalink
refactor: split Clone into Clone and MergeAgain
Browse files Browse the repository at this point in the history
Clone is now a NOP if the PR has not changed, and loses its second
return value, the MergedAgain flag.

MergeAgain must be called explicitly in the only location that
cares about this flag, just before planning.

This cleans up the code for Clone and re-merging a bit.

Also regenerated mocks
  • Loading branch information
finnag committed Apr 25, 2024
1 parent 7969c67 commit 4d89a78
Show file tree
Hide file tree
Showing 19 changed files with 268 additions and 189 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 17 additions & 12 deletions server/core/runtime/mocks/mock_pull_approved_checker.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion server/events/github_app_working_dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type GithubAppWorkingDir struct {
}

// Clone writes a fresh token for Github App authentication
func (g *GithubAppWorkingDir) Clone(logger logging.SimpleLogging, headRepo models.Repo, p models.PullRequest, workspace string) (string, bool, error) {
func (g *GithubAppWorkingDir) Clone(logger logging.SimpleLogging, headRepo models.Repo, p models.PullRequest, workspace string) (string, error) {
baseRepo := &p.BaseRepo

// Realistically, this is a super brittle way of supporting clones using gh app installation tokens
Expand Down
8 changes: 4 additions & 4 deletions server/events/github_app_working_dir_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TestClone_GithubAppNoneExisting(t *testing.T) {
GithubHostname: testServer,
}

cloneDir, _, err := gwd.Clone(logger, models.Repo{}, models.PullRequest{
cloneDir, err := gwd.Clone(logger, models.Repo{}, models.PullRequest{
BaseRepo: models.Repo{},
HeadBranch: "branch",
}, "default")
Expand Down Expand Up @@ -90,11 +90,11 @@ func TestClone_GithubAppSetsCorrectUrl(t *testing.T) {

When(credentials.GetToken()).ThenReturn("token", nil)
When(workingDir.Clone(Any[logging.SimpleLogging](), Eq(modifiedBaseRepo), Eq(models.PullRequest{BaseRepo: modifiedBaseRepo}),
Eq("default"))).ThenReturn("", true, nil)
Eq("default"))).ThenReturn("", nil)

_, success, _ := ghAppWorkingDir.Clone(logger, headRepo, models.PullRequest{BaseRepo: baseRepo}, "default")
_, err := ghAppWorkingDir.Clone(logger, headRepo, models.PullRequest{BaseRepo: baseRepo}, "default")

workingDir.VerifyWasCalledOnce().Clone(logger, modifiedBaseRepo, models.PullRequest{BaseRepo: modifiedBaseRepo}, "default")

Assert(t, success == true, "clone url mutation error")
Ok(t, err)
}
67 changes: 48 additions & 19 deletions server/events/mock_workingdir_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion server/events/mocks/mock_event_parsing.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

67 changes: 48 additions & 19 deletions server/events/mocks/mock_working_dir.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion server/events/post_workflow_hooks_command_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (w *DefaultPostWorkflowHooksCommandRunner) RunPostHooks(ctx *command.Contex
ctx.Log.Debug("got workspace lock")
defer unlockFn()

repoDir, _, err := w.WorkingDir.Clone(ctx.Log, ctx.HeadRepo, ctx.Pull, DefaultWorkspace)
repoDir, err := w.WorkingDir.Clone(ctx.Log, ctx.HeadRepo, ctx.Pull, DefaultWorkspace)
if err != nil {
return err
}
Expand Down
Loading

0 comments on commit 4d89a78

Please sign in to comment.