Skip to content

Commit

Permalink
Add branch recommendations to checkout errors [ACT-912] (#129)
Browse files Browse the repository at this point in the history 
* refactor: Rename mapRecommendation to better state its purpose

* test: Add new test cases for branch recommendation

* feat: Implement new step error constructor

* test: Add tests for branch related error handling

* refactor: branch related error handling
  • Loading branch information
@zoltanszabo-bitrise
zoltanszabo-bitrise authored Nov 12, 2020
1 parent af8aa6f commit d15bd8e
Show file tree
Hide file tree
Showing 4 changed files with 282 additions and 59 deletions.
70 changes: 43 additions & 27 deletions gitclone/git.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"strconv"
"strings"

"github.com/bitrise-io/bitrise-init/errormapper"
"github.com/bitrise-io/bitrise-init/step"
"github.com/bitrise-io/go-utils/command"
"github.com/bitrise-io/go-utils/command/git"
Expand All @@ -27,7 +26,6 @@ import (
const (
checkoutFailedTag = "checkout_failed"
fetchFailedTag = "fetch_failed"
branchRecKey = "BranchRecommendation"
)

func isOriginPresent(gitCmd git.Git, dir, repoURL string) (bool, error) {
Expand Down Expand Up @@ -296,6 +294,22 @@ func manualMerge(gitCmd git.Git, repoURL, prRepoURL, branch, commit, branchDest
return nil
}

type getAvailableBranches func() (map[string][]string, error)

func listBranches(gitCmd git.Git) getAvailableBranches {
return func() (map[string][]string, error) {
if err := run(gitCmd.Fetch()); err != nil {
return nil, err
}
out, err := output(gitCmd.Branch("-r"))
if err != nil {
return nil, err
}

return parseListBranchesOutput(out), nil
}
}

func parseListBranchesOutput(output string) map[string][]string {
lines := strings.Split(output, "\n")
branchesByRemote := map[string][]string{}
Expand All @@ -315,16 +329,29 @@ func parseListBranchesOutput(output string) map[string][]string {
return branchesByRemote
}

func listBranches(gitCmd git.Git) (map[string][]string, error) {
if err := run(gitCmd.Fetch()); err != nil {
return nil, err
}
out, err := output(gitCmd.Branch("-r"))
if err != nil {
return nil, err
func handleCheckoutError(callback getAvailableBranches, tag string, err error, shortMsg string, branch string) *step.Error {
// We were checking out a branch (not tag or commit)
if branch != "" {
branchesByRemote, branchesErr := callback()
branches := branchesByRemote[defaultRemoteName]
// There was no error grabbing the available branches
// And the current branch is not present in the list
if branchesErr == nil && !sliceutil.IsStringInSlice(branch, branches) {
return newStepErrorWithBranchRecommendations(
tag,
err,
shortMsg,
branch,
branches,
)
}
}

return parseListBranchesOutput(out), nil
return newStepError(
tag,
err,
shortMsg,
)
}

func checkout(gitCmd git.Git, arg, branch string, depth int, isTag bool) *step.Error {
Expand All @@ -341,34 +368,23 @@ func checkout(gitCmd git.Git, arg, branch string, depth int, isTag bool) *step.E
}
return gitCmd.Fetch(opts...)
}); err != nil {
if branch != "" {
branchesByRemote, branchesErr := listBranches(gitCmd)
branches := branchesByRemote[defaultRemoteName]
if branchesErr == nil && !sliceutil.IsStringInSlice(branch, branches) {
return newStepErrorWithRecommendations(
fetchFailedTag,
fmt.Errorf("fetch failed: invalid branch selected: %s, available branches: %s: %v", branch, strings.Join(branches, ", "), err),
"Fetching repository has failed",
step.Recommendation{
branchRecKey: branches,
errormapper.DetailedErrorRecKey: newFetchFailedInvalidBranchDetailedError(branch),
},
)
}
}
return newStepError(
return handleCheckoutError(
listBranches(gitCmd),
fetchFailedTag,
fmt.Errorf("fetch failed, error: %v", err),
"Fetching repository has failed",
branch,
)
}

if err := run(gitCmd.Checkout(arg)); err != nil {
if depth == 0 {
return newStepError(
return handleCheckoutError(
listBranches(gitCmd),
checkoutFailedTag,
fmt.Errorf("checkout failed (%s), error: %v", arg, err),
"Checkout has failed",
branch,
)
}
log.Warnf("Checkout failed, error: %v\nUnshallow...", err)
Expand Down
152 changes: 152 additions & 0 deletions gitclone/git_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
package gitclone

import (
"errors"
"reflect"
"testing"

"github.com/bitrise-io/bitrise-init/errormapper"
"github.com/bitrise-io/bitrise-init/step"
)

func TestFetchArg(t *testing.T) {
Expand Down Expand Up @@ -96,3 +100,151 @@ func Test_parseListBranchesOutput(t *testing.T) {
})
}
}

func Test_handleCheckoutError(t *testing.T) {
type args struct {
callback func() (map[string][]string, error)
tag string
err error
shortMsg string
branch string
}
tests := []struct {
name string
args args
want *step.Error
}{
{
name: "handleCheckoutError: generic error without branch recommendation",
args: args{
callback: func() (map[string][]string, error) { return nil, nil },
tag: "fetch_failed",
err: errors.New("Something bad happened"),
shortMsg: "Fetching repository has failed",
branch: "",
},
want: &step.Error{
StepID: "git-clone",
Tag: "fetch_failed",
Err: errors.New("Something bad happened"),
ShortMsg: "Fetching repository has failed",
Recommendations: step.Recommendation{
errormapper.DetailedErrorRecKey: errormapper.DetailedError{
Title: "We couldn’t fetch your repository.",
Description: "Our auto-configurator returned the following error:\nSomething bad happened",
},
},
},
},
{
name: "handleCheckoutError: specific error with branch recommendations for default remote",
args: args{
callback: func() (map[string][]string, error) {
return map[string][]string{
defaultRemoteName: {"master", "develop"},
}, nil
},
tag: "checkout_failed",
err: errors.New("pathspec 'test' did not match any file(s) known to git"),
shortMsg: "Checkout has failed",
branch: "test",
},
want: &step.Error{
StepID: "git-clone",
Tag: "checkout_failed",
Err: errors.New("pathspec 'test' did not match any file(s) known to git"),
ShortMsg: "Checkout has failed",
Recommendations: step.Recommendation{
branchRecKey: []string{"master", "develop"},
errormapper.DetailedErrorRecKey: errormapper.DetailedError{
Title: "We couldn't find the branch 'test'.",
Description: "Please choose another branch and try again.",
},
},
},
},
{
name: "handleCheckoutError: specific error without branch recommendations due to error",
args: args{
callback: func() (map[string][]string, error) {
return nil, errors.New("No available branches")
},
tag: "checkout_failed",
err: errors.New("pathspec 'test' did not match any file(s) known to git"),
shortMsg: "Checkout has failed",
branch: "test",
},
want: &step.Error{
StepID: "git-clone",
Tag: "checkout_failed",
Err: errors.New("pathspec 'test' did not match any file(s) known to git"),
ShortMsg: "Checkout has failed",
Recommendations: step.Recommendation{
errormapper.DetailedErrorRecKey: errormapper.DetailedError{
Title: "We couldn't find the branch 'test'.",
Description: "Please choose another branch and try again.",
},
},
},
},
{
name: "handleCheckoutError: specific error without branch recommendations due correct branch",
args: args{
callback: func() (map[string][]string, error) {
return map[string][]string{
defaultRemoteName: {"master", "develop", "test"},
}, nil
},
tag: "checkout_failed",
err: errors.New("pathspec 'test' did not match any file(s) known to git"),
shortMsg: "Checkout has failed",
branch: "test",
},
want: &step.Error{
StepID: "git-clone",
Tag: "checkout_failed",
Err: errors.New("pathspec 'test' did not match any file(s) known to git"),
ShortMsg: "Checkout has failed",
Recommendations: step.Recommendation{
errormapper.DetailedErrorRecKey: errormapper.DetailedError{
Title: "We couldn't find the branch 'test'.",
Description: "Please choose another branch and try again.",
},
},
},
},
{
name: "handleCheckoutError: specific error without branch recommendations for default remote",
args: args{
callback: func() (map[string][]string, error) {
return map[string][]string{
"something": {"master", "develop"},
}, nil
},
tag: "checkout_failed",
err: errors.New("pathspec 'test' did not match any file(s) known to git"),
shortMsg: "Checkout has failed",
branch: "test",
},
want: &step.Error{
StepID: "git-clone",
Tag: "checkout_failed",
Err: errors.New("pathspec 'test' did not match any file(s) known to git"),
ShortMsg: "Checkout has failed",
Recommendations: step.Recommendation{
errormapper.DetailedErrorRecKey: errormapper.DetailedError{
Title: "We couldn't find the branch 'test'.",
Description: "Please choose another branch and try again.",
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := handleCheckoutError(tt.args.callback, tt.args.tag, tt.args.err, tt.args.shortMsg, tt.args.branch); !reflect.DeepEqual(got, tt.want) {
t.Errorf("handleCheckoutError() = %v, want %v", got, tt.want)
}
})
}
}
40 changes: 29 additions & 11 deletions gitclone/steperror.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,37 +7,55 @@ import (
"github.com/bitrise-io/bitrise-init/step"
)

func mapRecommendation(tag, errMsg string) step.Recommendation {
const (
branchRecKey = "BranchRecommendation"
)

func mapDetailedErrorRecommendation(tag, errMsg string) step.Recommendation {
switch tag {
case checkoutFailedTag:
matcher := newCheckoutFailedPatternErrorMatcher()
return matcher.Run(errMsg)
case updateSubmodelFailedTag: // update_submodule_failed could have the same errors as fetch
fallthrough
case fetchFailedTag:
fetchFailedMatcher := newFetchFailedPatternErrorMatcher()
return fetchFailedMatcher.Run(errMsg)
matcher := newFetchFailedPatternErrorMatcher()
return matcher.Run(errMsg)
}
return nil
}

func newStepError(tag string, err error, shortMsg string) *step.Error {
recommendation := mapRecommendation(tag, err.Error())
if recommendation != nil {
return step.NewErrorWithRecommendations("git-clone", tag, err, shortMsg, recommendation)
recommendations := mapDetailedErrorRecommendation(tag, err.Error())
if recommendations != nil {
return step.NewErrorWithRecommendations("git-clone", tag, err, shortMsg, recommendations)
}

return step.NewError("git-clone", tag, err, shortMsg)
}

func newStepErrorWithRecommendations(tag string, err error, shortMsg string, recommendations step.Recommendation) *step.Error {
return step.NewErrorWithRecommendations("git-clone", tag, err, shortMsg, recommendations)
func newStepErrorWithBranchRecommendations(tag string, err error, shortMsg, currentBranch string, availableBranches []string) *step.Error {
// First: Map the error messages
mappedError := newStepError(tag, err, shortMsg)

// Second: Extend recommendation with available branches, if has any
if len(availableBranches) > 0 {
rec := mappedError.Recommendations
if rec == nil {
rec = step.Recommendation{}
}
rec[branchRecKey] = availableBranches
}

return mappedError
}

func newCheckoutFailedPatternErrorMatcher() *errormapper.PatternErrorMatcher {
return &errormapper.PatternErrorMatcher{
DefaultBuilder: newCheckoutFailedGenericDetailedError,
PatternToBuilder: nil,
DefaultBuilder: newCheckoutFailedGenericDetailedError,
PatternToBuilder: errormapper.PatternToDetailedErrorBuilder{
`pathspec '(.+)' did not match any file\(s\) known to git`: newInvalidBranchDetailedError,
},
}
}

Expand Down Expand Up @@ -122,7 +140,7 @@ func newFetchFailedSamlSSOEnforcedDetailedError(...string) errormapper.DetailedE
}
}

func newFetchFailedInvalidBranchDetailedError(params ...string) errormapper.DetailedError {
func newInvalidBranchDetailedError(params ...string) errormapper.DetailedError {
branch := errormapper.GetParamAt(0, params)
return errormapper.DetailedError{
Title: fmt.Sprintf("We couldn't find the branch '%s'.", branch),
Expand Down
Loading

0 comments on commit d15bd8e

Please sign in to comment.