From d4324307441ffc74002e1cb4f5c08b83f45a2781 Mon Sep 17 00:00:00 2001 From: Johan Lindell Date: Tue, 19 Jan 2021 20:44:01 +0100 Subject: [PATCH] fix: added the number of created pull requests (#56) --- internal/github/github.go | 13 +++-- internal/gitlab/gitlab.go | 18 +++++-- internal/multigitter/close.go | 2 +- internal/multigitter/merge.go | 2 +- internal/multigitter/print.go | 2 +- internal/multigitter/repocounter/counter.go | 18 ++++++- internal/multigitter/run.go | 53 +++++++++++++-------- tests/story_test.go | 18 +++---- tests/table_test.go | 6 +-- tests/vcmock/vcmock.go | 16 +++++-- 10 files changed, 100 insertions(+), 48 deletions(-) diff --git a/internal/github/github.go b/internal/github/github.go index 1a7cd97e..fd70828a 100755 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -240,19 +240,24 @@ func (g Github) getRepository(ctx context.Context, repoRef RepositoryReference) } // CreatePullRequest creates a pull request -func (g Github) CreatePullRequest(ctx context.Context, repo domain.Repository, newPR domain.NewPullRequest) error { +func (g Github) CreatePullRequest(ctx context.Context, repo domain.Repository, newPR domain.NewPullRequest) (domain.PullRequest, error) { r := repo.(repository) pr, err := g.createPullRequest(ctx, r, newPR) if err != nil { - return err + return nil, err } if err := g.addReviewers(ctx, r, newPR, pr); err != nil { - return err + return nil, err } - return nil + return pullRequest{ + ownerName: pr.GetBase().GetUser().GetLogin(), + repoName: pr.GetBase().GetRepo().GetName(), + branchName: pr.GetHead().GetRef(), + number: pr.GetNumber(), + }, nil } func (g Github) createPullRequest(ctx context.Context, repo repository, newPR domain.NewPullRequest) (*github.PullRequest, error) { diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go index a9db863d..3dd16c05 100644 --- a/internal/gitlab/gitlab.go +++ b/internal/gitlab/gitlab.go @@ -232,7 +232,7 @@ func (g *Gitlab) getUserProjects(ctx context.Context, username string) ([]*gitla } // CreatePullRequest creates a pull request -func (g *Gitlab) CreatePullRequest(ctx context.Context, repo domain.Repository, newPR domain.NewPullRequest) error { +func (g *Gitlab) CreatePullRequest(ctx context.Context, repo domain.Repository, newPR domain.NewPullRequest) (domain.PullRequest, error) { r := repo.(repository) // Convert from usernames to user ids @@ -241,18 +241,28 @@ func (g *Gitlab) CreatePullRequest(ctx context.Context, repo domain.Repository, var err error assigneeIDs, err = g.getUserIDs(ctx, newPR.Reviewers) if err != nil { - return err + return nil, err } } - _, _, err := g.glClient.MergeRequests.CreateMergeRequest(r.pid, &gitlab.CreateMergeRequestOptions{ + mr, _, err := g.glClient.MergeRequests.CreateMergeRequest(r.pid, &gitlab.CreateMergeRequestOptions{ Title: &newPR.Title, Description: &newPR.Body, SourceBranch: &newPR.Head, TargetBranch: &newPR.Base, AssigneeIDs: assigneeIDs, }) - return err + if err != nil { + return nil, err + } + + return pullRequest{ + repoName: r.name, + ownerName: r.ownerName, + pid: r.pid, + branchName: newPR.Head, + iid: mr.IID, + }, nil } func (g *Gitlab) getUserIDs(ctx context.Context, usernames []string) ([]int, error) { diff --git a/internal/multigitter/close.go b/internal/multigitter/close.go index b466c069..c9d826f0 100644 --- a/internal/multigitter/close.go +++ b/internal/multigitter/close.go @@ -31,7 +31,7 @@ func (s Closer) Close(ctx context.Context) error { log.Infof("Closing %d pull requests", len(openPRs)) for _, pr := range openPRs { - log.WithField("repo", pr.String()).Infof("Closing") + log.WithField("pr", pr.String()).Infof("Closing") err := s.VersionController.ClosePullRequest(ctx, pr) if err != nil { return err diff --git a/internal/multigitter/merge.go b/internal/multigitter/merge.go index 6bf9986a..819ca7cc 100644 --- a/internal/multigitter/merge.go +++ b/internal/multigitter/merge.go @@ -32,7 +32,7 @@ func (s Merger) Merge(ctx context.Context) error { log.Infof("Merging %d pull requests", len(successPrs)) for _, pr := range successPrs { - log.WithField("repo", pr.String()).Infof("Merging") + log.WithField("pr", pr.String()).Infof("Merging") err := s.VersionController.MergePullRequest(ctx, pr) if err != nil { return err diff --git a/internal/multigitter/print.go b/internal/multigitter/print.go index 3d66ed0f..45ac952d 100755 --- a/internal/multigitter/print.go +++ b/internal/multigitter/print.go @@ -56,7 +56,7 @@ func (r Printer) Print(ctx context.Context) error { return } - rc.AddSuccess(repos[i]) + rc.AddSuccessRepositories(repos[i]) }, len(repos), r.Concurrent) return nil diff --git a/internal/multigitter/repocounter/counter.go b/internal/multigitter/repocounter/counter.go index d90edea0..5ebe1d8d 100644 --- a/internal/multigitter/repocounter/counter.go +++ b/internal/multigitter/repocounter/counter.go @@ -10,6 +10,7 @@ import ( // Counter keeps track of succeeded and failed repositories type Counter struct { + successPullRequests []domain.PullRequest successRepositories []domain.Repository errorRepositories map[string][]domain.Repository lock sync.RWMutex @@ -32,13 +33,21 @@ func (r *Counter) AddError(err error, repo domain.Repository) { } // AddSuccess adds a repository that succeeded -func (r *Counter) AddSuccess(repo domain.Repository) { +func (r *Counter) AddSuccessRepositories(repo domain.Repository) { defer r.lock.Unlock() r.lock.Lock() r.successRepositories = append(r.successRepositories, repo) } +// AddSuccess adds a pullrequest that succeeded +func (r *Counter) AddSuccessPullRequest(repo domain.PullRequest) { + defer r.lock.Unlock() + r.lock.Lock() + + r.successPullRequests = append(r.successPullRequests, repo) +} + // Info returns a formated string about all repositories func (r *Counter) Info() string { defer r.lock.RUnlock() @@ -53,6 +62,13 @@ func (r *Counter) Info() string { } } + if len(r.successPullRequests) > 0 { + exitInfo += "Repositories with a successful run:\n" + for _, pr := range r.successPullRequests { + exitInfo += fmt.Sprintf(" %s\n", pr.String()) + } + } + if len(r.successRepositories) > 0 { exitInfo += "Repositories with a successful run:\n" for _, repo := range r.successRepositories { diff --git a/internal/multigitter/run.go b/internal/multigitter/run.go index 42a44a12..15b59dad 100755 --- a/internal/multigitter/run.go +++ b/internal/multigitter/run.go @@ -22,7 +22,7 @@ import ( // VersionController fetches repositories type VersionController interface { GetRepositories(ctx context.Context) ([]domain.Repository, error) - CreatePullRequest(ctx context.Context, repo domain.Repository, newPR domain.NewPullRequest) error + CreatePullRequest(ctx context.Context, repo domain.Repository, newPR domain.NewPullRequest) (domain.PullRequest, error) GetPullRequestStatuses(ctx context.Context, branchName string) ([]domain.PullRequest, error) MergePullRequest(ctx context.Context, pr domain.PullRequest) error ClosePullRequest(ctx context.Context, pr domain.PullRequest) error @@ -53,6 +53,19 @@ type Runner struct { var errAborted = errors.New("run was never started because of aborted execution") +type dryRunPullRequest struct { + status domain.PullRequestStatus + Repository domain.Repository +} + +func (pr dryRunPullRequest) Status() domain.PullRequestStatus { + return pr.status +} + +func (pr dryRunPullRequest) String() string { + return fmt.Sprintf("%s #0", pr.Repository.FullName()) +} + // Run runs a script for multiple repositories and creates PRs with the changes made func (r Runner) Run(ctx context.Context) error { repos, err := r.VersionController.GetRepositories(ctx) @@ -71,7 +84,7 @@ func (r Runner) Run(ctx context.Context) error { runInParallel(func(i int) { logger := log.WithField("repo", repos[i].FullName()) - err := r.runSingleRepo(ctx, repos[i]) + pr, err := r.runSingleRepo(ctx, repos[i]) if err != nil { if err != errAborted { logger.Info(err) @@ -80,7 +93,7 @@ func (r Runner) Run(ctx context.Context) error { return } - rc.AddSuccess(repos[i]) + rc.AddSuccessPullRequest(pr) }, len(repos), r.Concurrent) return nil @@ -111,9 +124,9 @@ func getReviewers(reviewers []string, maxReviewers int) []string { return reviewers[0:maxReviewers] } -func (r Runner) runSingleRepo(ctx context.Context, repo domain.Repository) error { +func (r Runner) runSingleRepo(ctx context.Context, repo domain.Repository) (domain.PullRequest, error) { if ctx.Err() != nil { - return errAborted + return nil, errAborted } log := log.WithField("repo", repo.FullName()) @@ -121,7 +134,7 @@ func (r Runner) runSingleRepo(ctx context.Context, repo domain.Repository) error tmpDir, err := ioutil.TempDir(os.TempDir(), "multi-git-changer-") if err != nil { - return err + return nil, err } defer os.RemoveAll(tmpDir) @@ -137,19 +150,19 @@ func (r Runner) runSingleRepo(ctx context.Context, repo domain.Repository) error err = sourceController.Clone(baseBranch, r.FeatureBranch) if err != nil { - return err + return nil, err } featureBranchExist, err := sourceController.BranchExist(r.FeatureBranch) if err != nil { - return errors.Wrap(err, "could not verify if branch already exist") + return nil, errors.Wrap(err, "could not verify if branch already exist") } else if featureBranchExist { - return domain.BranchExistError + return nil, domain.BranchExistError } err = sourceController.ChangeBranch(r.FeatureBranch) if err != nil { - return err + return nil, err } // Run the command that might or might not change the content of the repo @@ -167,32 +180,34 @@ func (r Runner) runSingleRepo(ctx context.Context, repo domain.Repository) error err = cmd.Run() if err != nil { - return err + return nil, err } if changed, err := sourceController.Changes(); err != nil { - return err + return nil, err } else if !changed { - return domain.NoChangeError + return nil, domain.NoChangeError } err = sourceController.Commit(r.CommitAuthor, r.CommitMessage) if err != nil { - return err + return nil, err } if r.DryRun { log.Info("Skipping pushing changes because of dry run") - return nil + return dryRunPullRequest{ + Repository: repo, + }, nil } err = sourceController.Push() if err != nil { - return errors.Wrap(err, "could not push changes") + return nil, errors.Wrap(err, "could not push changes") } log.Info("Change done, creating pull request") - err = r.VersionController.CreatePullRequest(ctx, repo, domain.NewPullRequest{ + pr, err := r.VersionController.CreatePullRequest(ctx, repo, domain.NewPullRequest{ Title: r.PullRequestTitle, Body: r.PullRequestBody, Head: r.FeatureBranch, @@ -200,8 +215,8 @@ func (r Runner) runSingleRepo(ctx context.Context, repo domain.Repository) error Reviewers: getReviewers(r.Reviewers, r.MaxReviewers), }) if err != nil { - return err + return nil, err } - return nil + return pr, nil } diff --git a/tests/story_test.go b/tests/story_test.go index 6391cbd1..2fad1a37 100644 --- a/tests/story_test.go +++ b/tests/story_test.go @@ -58,12 +58,12 @@ func TestStory(t *testing.T) { // Verify that the output was correct runOutData, err := ioutil.ReadFile(runOutFile) require.NoError(t, err) - assert.Equal(t, string(runOutData), `No data was changed: + assert.Equal(t, `No data was changed: should-not-change Repositories with a successful run: - should-change - should-change-2 -`) + should-change #1 + should-change-2 #2 +`, string(runOutData)) // // Status @@ -81,7 +81,7 @@ Repositories with a successful run: // Verify that the output was correct statusOutData, err := ioutil.ReadFile(statusOutFile) require.NoError(t, err) - assert.Equal(t, "should-change/XX: Pending\nshould-change-2/XX: Pending\n", string(statusOutData)) + assert.Equal(t, "should-change #1: Pending\nshould-change-2 #2: Pending\n", string(statusOutData)) // One of the created PRs is set to succeeded vcMock.SetPRStatus("should-change", "custom-branch-name", domain.PullRequestStatusSuccess) @@ -103,7 +103,7 @@ Repositories with a successful run: mergeLogData, err := ioutil.ReadFile(mergeLogFile) require.NoError(t, err) assert.Contains(t, string(mergeLogData), "Merging 1 pull requests") - assert.Contains(t, string(mergeLogData), "Merging repo=should-change/XX") + assert.Contains(t, string(mergeLogData), "Merging pr=\"should-change #1\"") // // After Merge Status @@ -121,7 +121,7 @@ Repositories with a successful run: // Verify that the output was correct afterMergeStatusOutData, err := ioutil.ReadFile(afterMergeStatusOutFile) require.NoError(t, err) - assert.Equal(t, "should-change/XX: Merged\nshould-change-2/XX: Pending\n", string(afterMergeStatusOutData)) + assert.Equal(t, "should-change #1: Merged\nshould-change-2 #2: Pending\n", string(afterMergeStatusOutData)) // // Close @@ -140,7 +140,7 @@ Repositories with a successful run: closeLogData, err := ioutil.ReadFile(closeLogFile) require.NoError(t, err) assert.Contains(t, string(closeLogData), "Closing 1 pull request") - assert.Contains(t, string(closeLogData), "Closing repo=should-change-2/XX") + assert.Contains(t, string(closeLogData), "Closing pr=\"should-change-2 #2\"") // // After Close Status @@ -158,5 +158,5 @@ Repositories with a successful run: // Verify that the output was correct afterCloseStatusOutData, err := ioutil.ReadFile(afterCloseStatusOutFile) require.NoError(t, err) - assert.Equal(t, "should-change/XX: Merged\nshould-change-2/XX: Closed\n", string(afterCloseStatusOutData)) + assert.Equal(t, "should-change #1: Merged\nshould-change-2 #2: Closed\n", string(afterCloseStatusOutData)) } diff --git a/tests/table_test.go b/tests/table_test.go index 0dbf50d2..74d86b74 100644 --- a/tests/table_test.go +++ b/tests/table_test.go @@ -60,7 +60,7 @@ func TestTable(t *testing.T) { assert.Contains(t, runData.logOut, "Change done, creating pull request") assert.Equal(t, `Repositories with a successful run: - should-change + should-change #1 `, runData.out) }, }, @@ -90,7 +90,7 @@ func TestTable(t *testing.T) { assert.Contains(t, runData.logOut, "Change done, creating pull request") assert.Equal(t, `Repositories with a successful run: - should-change + should-change #1 `, runData.out) }, }, @@ -275,7 +275,7 @@ func TestTable(t *testing.T) { assert.Equal(t, `The new branch does already exist: already-existing-branch Repositories with a successful run: - should-change + should-change #1 `, runData.out) }, }, diff --git a/tests/vcmock/vcmock.go b/tests/vcmock/vcmock.go index e3a515f1..a8542c69 100644 --- a/tests/vcmock/vcmock.go +++ b/tests/vcmock/vcmock.go @@ -5,12 +5,14 @@ package vcmock import ( "context" "errors" + "fmt" "github.com/lindell/multi-gitter/internal/domain" ) // VersionController is a mock of an version controller (Github/Gitlab/etc.) type VersionController struct { + PRNumber int Repositories []Repository PullRequests []PullRequest } @@ -25,16 +27,19 @@ func (vc *VersionController) GetRepositories(ctx context.Context) ([]domain.Repo } // CreatePullRequest stores a mock pull request -func (vc *VersionController) CreatePullRequest(ctx context.Context, repo domain.Repository, newPR domain.NewPullRequest) error { +func (vc *VersionController) CreatePullRequest(ctx context.Context, repo domain.Repository, newPR domain.NewPullRequest) (domain.PullRequest, error) { repository := repo.(Repository) - vc.PullRequests = append(vc.PullRequests, PullRequest{ + vc.PRNumber = vc.PRNumber + 1 + pr := PullRequest{ PRStatus: domain.PullRequestStatusPending, + PRNumber: vc.PRNumber, Repository: repository, NewPullRequest: newPR, - }) + } + vc.PullRequests = append(vc.PullRequests, pr) - return nil + return pr, nil } // GetPullRequestStatuses gets mock pull request statuses @@ -89,6 +94,7 @@ func (vc *VersionController) SetPRStatus(repoName string, branchName string, new // PullRequest is a mock pr type PullRequest struct { PRStatus domain.PullRequestStatus + PRNumber int Merged bool Repository @@ -102,7 +108,7 @@ func (pr PullRequest) Status() domain.PullRequestStatus { // String return a description of the pr func (pr PullRequest) String() string { - return pr.Repository.Name + "/XX" + return fmt.Sprintf("%s #%d", pr.Repository.Name, pr.PRNumber) } // Repository is a mock repository