From e423ce1e6483bad53a44c440352e39c95e088d13 Mon Sep 17 00:00:00 2001 From: Daniel James Date: Tue, 12 May 2020 16:18:18 -0700 Subject: [PATCH] (+semver: fix) Fix broken version calculation and improve test coverage (#29) * (+semver: fix) Fix egregious bug that counts the same commit several times * (+semver: feature) Support 2+ parents on merge commits * Update go.mod/go.sum * (+semver: fix) Consolitated tag string parsing * (+semver: patch) Rewrote tests * (+semver: fix) Ran formatter --- go.mod | 16 +- go.sum | 6 + pkg/git/branchWalker.go | 38 ++-- pkg/git/git.go | 4 +- pkg/git/git_test.go | 388 +++++++++++++++++++++++++++------------- 5 files changed, 302 insertions(+), 150 deletions(-) diff --git a/go.mod b/go.mod index 22d49be..6839ee1 100644 --- a/go.mod +++ b/go.mod @@ -4,23 +4,11 @@ go 1.13 require ( github.com/coreos/go-semver v0.2.0 - github.com/emirpasic/gods v1.9.0 - github.com/inconshreveable/mousetrap v1.0.0 - github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 - github.com/kevinburke/ssh_config v0.0.0-20180830205328-81db2a75821e - github.com/mitchellh/go-homedir v1.0.0 - github.com/pelletier/go-buffruneio v0.2.0 + github.com/inconshreveable/mousetrap v1.0.0 // indirect github.com/pkg/errors v0.8.0 - github.com/sergi/go-diff v1.0.0 github.com/spf13/cobra v0.0.3 - github.com/spf13/pflag v1.0.1 - github.com/src-d/gcfg v1.3.0 + github.com/spf13/pflag v1.0.1 // indirect github.com/stretchr/testify v1.2.2 - github.com/xanzy/ssh-agent v0.2.0 - golang.org/x/crypto v0.0.0-20180904163835-0709b304e793 - golang.org/x/net v0.0.0-20180906233101-161cd47e91fd - golang.org/x/sys v0.0.0-20180903190138-2b024373dcd9 - golang.org/x/text v0.3.0 gopkg.in/src-d/go-billy.v4 v4.2.1 gopkg.in/src-d/go-git.v4 v4.7.1 gopkg.in/yaml.v2 v2.2.1 diff --git a/go.sum b/go.sum index 7179092..98ee0d9 100644 --- a/go.sum +++ b/go.sum @@ -1,4 +1,6 @@ +github.com/alcortesm/tgz v0.0.0-20161220082320-9c5fe88206d7 h1:uSoVVbwJiQipAclBbw+8quDsfcvFjOpI5iCf4p/cqCs= github.com/alcortesm/tgz v0.0.0-20161220082320-9c5fe88206d7/go.mod h1:6zEj6s6u/ghQa61ZWa/C2Aw3RkjiTBOix7dkqa1VLIs= +github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239 h1:kFOfPq6dUM1hTo4JG6LR5AXSUEsOjtdm0kw0FtQtMJA= github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239/go.mod h1:2FmKhYUyUczH0OGQWaF5ceTx0UBShxjsH6f8oGKYe2c= github.com/coreos/go-semver v0.2.0 h1:3Jm3tLmsgAYcjC+4Up7hJrFBPr+n7rAqYeSw/SZazuY= github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk= @@ -6,8 +8,11 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/emirpasic/gods v1.9.0 h1:rUF4PuzEjMChMiNsVjdI+SyLu7rEqpQ5reNFnhC7oFo= github.com/emirpasic/gods v1.9.0/go.mod h1:YfzfFFoVP/catgzJb4IKIqXjX78Ha8FMSDh3ymbK86o= +github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568 h1:BHsljHzVlRcyQhjrss6TZTdY2VfCqZPbv5k3iBFa2ZQ= github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568/go.mod h1:xEzjJPgXI435gkrCt3MPfRiAkVrwSbHsst4LCFVfpJc= +github.com/gliderlabs/ssh v0.1.1 h1:j3L6gSLQalDETeEg/Jg0mGY0/y/N6zI2xX1978P0Uqw= github.com/gliderlabs/ssh v0.1.1/go.mod h1:U7qILu1NlMHj9FlMhZLlkCdDnU1DBEAqr0aevW3Awn0= +github.com/google/go-cmp v0.2.0 h1:+dTQ8DZQJz0Mb/HjFlkptS1FeQ4cWSnN941F8aEG4SQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= @@ -54,6 +59,7 @@ gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/src-d/go-billy.v4 v4.2.1 h1:omN5CrMrMcQ+4I8bJ0wEhOBPanIRWzFC953IiXKdYzo= gopkg.in/src-d/go-billy.v4 v4.2.1/go.mod h1:tm33zBoOwxjYHZIE+OV8bxTWFMJLrconzFMd38aARFk= +gopkg.in/src-d/go-git-fixtures.v3 v3.1.1 h1:XWW/s5W18RaJpmo1l0IYGqXKuJITWRFuA45iOf1dKJs= gopkg.in/src-d/go-git-fixtures.v3 v3.1.1/go.mod h1:dLBcvytrw/TYZsNTWCnkNF2DSIlzWYqTe3rJR56Ac7g= gopkg.in/src-d/go-git.v4 v4.7.1 h1:phAV/kNULxfYEvyInGdPuq3U2MtPpJdgmtOUF3cghkQ= gopkg.in/src-d/go-git.v4 v4.7.1/go.mod h1:xrJH/YX8uSWewT6evfocf8qsivF18JgCN7/IMitOptY= diff --git a/pkg/git/branchWalker.go b/pkg/git/branchWalker.go index 41b780d..3e78d4f 100644 --- a/pkg/git/branchWalker.go +++ b/pkg/git/branchWalker.go @@ -72,6 +72,7 @@ func (b *branchWalker) GetVersion() (*semver.Version, error) { } for ; index >= 0; index-- { + v := versionMap[index] switch { case v.MajorBump: baseVersion.BumpMajor() @@ -121,8 +122,7 @@ func (b *branchWalker) walkVersion(ref *object.Commit, version *versionHolder, t tag, ok := b.tagMap[ref.Hash.String()] if ok { - ft := strings.TrimPrefix(tag, "v") - tagVersion, err := semver.NewVersion(ft) + tagVersion, err := parseTag(tag) if err != nil { return err } @@ -189,27 +189,29 @@ func (b *branchWalker) checkWalkParent(ref *object.Commit, version *versionHolde } func (b *branchWalker) reconcileCommit(hash string, version *gitVersion) error { - versionMap := versionHolder{ - versionMap: []*gitVersion{}, - } - commit, err := b.repository.CommitObject(plumbing.NewHash(hash)) if err != nil { return errors.Wrap(err, "failed to get commit in reconcile") } - if commit.NumParents() <= 1 { + numParents := commit.NumParents() + if numParents <= 1 { return nil } - parentToWalk, err := commit.Parent(1) - if err != nil { - return errors.Wrap(err, "failed to get parent in reconcile") + versionMap := versionHolder{ + versionMap: []*gitVersion{}, } + for i := 1; i < numParents; i++ { + parentToWalk, err := commit.Parent(i) + if err != nil { + return errors.Wrap(err, "failed to get parent in reconcile") + } - err = b.walkVersion(parentToWalk, &versionMap, true) - if err != nil { - return err + err = b.walkVersion(parentToWalk, &versionMap, true) + if err != nil { + return err + } } var hasMajor, hasMinor bool @@ -232,3 +234,13 @@ func (b *branchWalker) reconcileCommit(hash string, version *gitVersion) error { return nil } + +func parseTag(tag string) (*semver.Version, error) { // TODO: Support ignoring invalid semver tags + trimmedTag := strings.TrimPrefix(tag, "v") + version, err := semver.NewVersion(trimmedTag) + if err != nil { + return nil, err + } + + return version, nil +} diff --git a/pkg/git/git.go b/pkg/git/git.go index 2bbbdd8..628f6e7 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -35,7 +35,7 @@ type gitVersion struct { func GetCurrentVersion(r *git.Repository, settings *Settings, branchSettings *BranchSettings, verbose bool) (version string, err error) { tag, ok := os.LookupEnv("TRAVIS_TAG") if !branchSettings.IgnoreEnvVars && ok && tag != "" { // If this is a tagged build in travis shortcircuit here - version, err := semver.NewVersion(tag) + version, err := parseTag(tag) if err != nil { return "", err } @@ -117,7 +117,7 @@ func getVersion(r *git.Repository, h *plumbing.Reference, tagMap map[string]stri masterHead, err := r.Reference("refs/heads/master", false) if err != nil { - masterHead, err = r.Reference("refs/remotes/origin/master", false) + masterHead, err = r.Reference("refs/remotes/origin/master", false) // TODO: This needs test coverage if err != nil { return nil, errors.Wrap(err, "failed to get master branch at 'refs/heads/master, 'refs/remotes/origin/master'") } diff --git a/pkg/git/git_test.go b/pkg/git/git_test.go index fef6d39..bfb41ad 100644 --- a/pkg/git/git_test.go +++ b/pkg/git/git_test.go @@ -2,7 +2,7 @@ package git_test import ( "fmt" - "path" + "os" "testing" "time" @@ -18,118 +18,302 @@ import ( igit "github.com/syncromatics/gogitver/pkg/git" ) -func TestUseLightweightTagForVersionAnchor(t *testing.T) { - fs := memfs.New() - storage := memory.NewStorage() - - r, err := git.Init(storage, fs) - if err != nil { - t.Error(err) - t.FailNow() +func Test_ShouldCalculateVersionFromCommitsInMaster(t *testing.T) { + // Arrange + repository, worktree := initRepository(t) + + commitMultiple(t, worktree, + "(+semver: breaking) This is a major commit\n", + "(+semver: major) This is also a major commit\n", + "(+semver: feature) This is a minor commit\n", + "(+semver: minor) This is also a minor commit\n", + "(+semver: fix) This is a patch commit\n", + "(+semver: patch) This is also a patch commit\n", + "This is also a patch commit\n", + ) + + settings := igit.GetDefaultSettings() + branchSettings := &igit.BranchSettings{ + IgnoreEnvVars: true, } - w, err := r.Worktree() - util.WriteFile(fs, "foo", []byte("foo"), 0644) + // Act + version, err := igit.GetCurrentVersion(repository, settings, branchSettings, false) + assert.Nil(t, err) - _, err = w.Add("foo") - if err != nil { - t.Error(err) - t.FailNow() - } + // Assert + assert.Equal(t, "2.2.3", version) +} - hash, err := w.Commit("foo\n", &git.CommitOptions{Author: defaultSignature()}) - if err != nil { - t.Error(err) - t.FailNow() - } +func Test_ShouldCalculateVersionFromCommitsInBranch(t *testing.T) { + // Arrange + repository, worktree := initRepository(t) - util.WriteFile(fs, "foo2", []byte("foo"), 0644) + commitMultiple(t, worktree, "Initial commit") - _, err = w.Add("foo2") - if err != nil { - t.Error(err) - t.FailNow() - } + err := worktree.Checkout(&git.CheckoutOptions{ + Create: true, + Branch: plumbing.ReferenceName("refs/heads/a-branch"), + }) + assert.Nil(t, err) - hash, err = w.Commit("(+semver: major) This is a major commit\n", &git.CommitOptions{Author: defaultSignature()}) - if err != nil { - t.Error(err) - t.FailNow() - } + hash := commitMultiple(t, worktree, + "(+semver: major)\n", + "(+semver: minor)\n", + "(+semver: patch)\n", + "some text\n", + ) - _, err = createTag(r, hash, "1.5.0", false) - if err != nil { - t.Error(err) - t.FailNow() + settings := igit.GetDefaultSettings() + branchSettings := &igit.BranchSettings{ + IgnoreEnvVars: true, } - s := igit.GetDefaultSettings() - version, err := igit.GetCurrentVersion(r, s, &igit.BranchSettings{ + // Act + version, err := igit.GetCurrentVersion(repository, settings, branchSettings, false) + assert.Nil(t, err) + + // Assert + shortHash := hash.String()[0:4] + expected := fmt.Sprintf("1.1.1-a-branch-3-%s", shortHash) + assert.Equal(t, expected, version) +} + +func Test_ShouldCalculateVersionFromCommitsInMasterWithMergeCommits(t *testing.T) { + // Arrange + repository, worktree := initRepository(t) + + masterHash := commitMultiple(t, worktree, "Initial commit") + + err := worktree.Checkout(&git.CheckoutOptions{ + Create: true, + Branch: plumbing.ReferenceName("refs/heads/a-branch"), + }) + assert.Nil(t, err) + + branchHash := commitMultiple(t, worktree, + "(+semver: major)\n", + "(+semver: minor)\n", + "(+semver: patch)\n", + "some text\n", + ) + + err = worktree.Checkout(&git.CheckoutOptions{ + Branch: plumbing.ReferenceName("refs/heads/master"), + }) + assert.Nil(t, err) + + masterHash, err = worktree.Commit("merged a-branch\n", &git.CommitOptions{ + Author: defaultSignature(), + Parents: []plumbing.Hash{ + masterHash, + branchHash, + }, + }) + assert.Nil(t, err) + + err = worktree.Checkout(&git.CheckoutOptions{ + Create: true, + Branch: plumbing.ReferenceName("refs/heads/another-branch"), + }) + assert.Nil(t, err) + + branchHash = commitMultiple(t, worktree, + "(+semver: minor)\n", + "(+semver: patch)\n", + ) + + err = worktree.Checkout(&git.CheckoutOptions{ + Branch: plumbing.ReferenceName("refs/heads/master"), + }) + assert.Nil(t, err) + + masterHash, err = worktree.Commit("merged another-branch\n", &git.CommitOptions{ + Author: defaultSignature(), + Parents: []plumbing.Hash{ + masterHash, + branchHash, + }, + }) + assert.Nil(t, err) + + err = worktree.Checkout(&git.CheckoutOptions{ + Create: true, + Branch: plumbing.ReferenceName("refs/heads/yet-another-branch"), + }) + assert.Nil(t, err) + + branchHash = commitMultiple(t, worktree, + "(+semver: patch)\n", + "(+semver: patch)\n", + "(+semver: patch)\n", + ) + + err = worktree.Checkout(&git.CheckoutOptions{ + Branch: plumbing.ReferenceName("refs/heads/master"), + }) + assert.Nil(t, err) + + _, err = worktree.Commit("merged yet-another-branch\n", &git.CommitOptions{ + Author: defaultSignature(), + Parents: []plumbing.Hash{ + masterHash, + branchHash, + }, + }) + assert.Nil(t, err) + + settings := igit.GetDefaultSettings() + branchSettings := &igit.BranchSettings{ IgnoreEnvVars: true, - }, false) - if err != nil { - t.Error(err) - t.FailNow() } - assert.Equal(t, "1.5.0", version) + // Act + version, err := igit.GetCurrentVersion(repository, settings, branchSettings, false) + assert.Nil(t, err) + + // Assert + assert.Equal(t, "1.1.1", version) } -func TestUseAnnotatedTagForVersionAnchor(t *testing.T) { - fs := memfs.New() - storage := memory.NewStorage() +func Test_ShouldCalculateVersionFromLightweightTag(t *testing.T) { + // Arrange + repository, worktree := initRepository(t) - r, err := git.Init(storage, fs) - if err != nil { - t.Error(err) - t.FailNow() - } + hash := commitMultiple(t, worktree, + "(+semver: breaking)\n", + "(+semver: breaking)\n", + ) - w, err := r.Worktree() - util.WriteFile(fs, "foo", []byte("foo"), 0644) + ref := plumbing.NewHashReference(plumbing.ReferenceName("refs/tags/v1.2.3"), hash) + err := repository.Storer.SetReference(ref) + assert.Nil(t, err) - _, err = w.Add("foo") - if err != nil { - t.Error(err) - t.FailNow() - } + commitMultiple(t, worktree, + "(+semver: minor)\n", + "(+semver: patch)\n", + ) - hash, err := w.Commit("foo\n", &git.CommitOptions{Author: defaultSignature()}) - if err != nil { - t.Error(err) - t.FailNow() + settings := igit.GetDefaultSettings() + branchSettings := &igit.BranchSettings{ + IgnoreEnvVars: true, } - util.WriteFile(fs, "foo2", []byte("foo"), 0644) + // Act + version, err := igit.GetCurrentVersion(repository, settings, branchSettings, false) + assert.Nil(t, err) - _, err = w.Add("foo2") - if err != nil { - t.Error(err) - t.FailNow() - } + // Assert + assert.Equal(t, "1.3.1", version) +} - hash, err = w.Commit("(+semver: major) This is a major commit\n", &git.CommitOptions{Author: defaultSignature()}) - if err != nil { - t.Error(err) - t.FailNow() +func Test_ShouldFailToCalculateVersionFromImproperlyNamedLightweightTag(t *testing.T) { // TODO: Submit a PR that allows this condition to exist; ignore unparsable tags + // Arrange + repository, worktree := initRepository(t) + + hash := commitMultiple(t, worktree, "Initial commit") + + ref := plumbing.NewHashReference(plumbing.ReferenceName("refs/tags/an-arbitrary-tag-name"), hash) + err := repository.Storer.SetReference(ref) + assert.Nil(t, err) + + settings := igit.GetDefaultSettings() + branchSettings := &igit.BranchSettings{ + IgnoreEnvVars: true, } - _, err = createTag(r, hash, "1.5.0", true) - if err != nil { - t.Error(err) - t.FailNow() + // Act + _, err = igit.GetCurrentVersion(repository, settings, branchSettings, false) + + // Assert + assert.NotNil(t, err) +} + +func Test_ShouldCalculateVersionFromAnnotatedTag(t *testing.T) { + // Arrange + repository, worktree := initRepository(t) + + hash := commitMultiple(t, worktree, + "(+semver: breaking)\n", + "(+semver: breaking)\n", + ) + + commitObj, err := object.GetObject(repository.Storer, hash) + tag := object.Tag{ + Name: "5.6.7", + Message: "not important", + TargetType: commitObj.Type(), + Target: hash, } + tagObj := repository.Storer.NewEncodedObject() + err = tag.Encode(tagObj) + assert.Nil(t, err) - s := igit.GetDefaultSettings() - version, err := igit.GetCurrentVersion(r, s, &igit.BranchSettings{ + target, err := repository.Storer.SetEncodedObject(tagObj) + assert.Nil(t, err) + + ref := plumbing.NewHashReference(plumbing.ReferenceName("refs/tags/5.6.7"), target) + err = repository.Storer.SetReference(ref) + assert.Nil(t, err) + + commitMultiple(t, worktree, + "(+semver: minor)\n", + "(+semver: patch)\n", + ) + + settings := igit.GetDefaultSettings() + branchSettings := &igit.BranchSettings{ IgnoreEnvVars: true, - }, false) - if err != nil { - t.Error(err) - t.FailNow() } - assert.Equal(t, "1.5.0", version) + // Act + version, err := igit.GetCurrentVersion(repository, settings, branchSettings, false) + assert.Nil(t, err) + + // Assert + assert.Equal(t, "5.7.1", version) +} + +func Test_ShouldCalculateVersionFromTravisTag(t *testing.T) { + // Arrange + repository, worktree := initRepository(t) + + commitMultiple(t, worktree, "Initial commit") + + settings := igit.GetDefaultSettings() + branchSettings := &igit.BranchSettings{} + os.Setenv("TRAVIS_TAG", "v1.2.3") + + // Act + version, err := igit.GetCurrentVersion(repository, settings, branchSettings, false) + assert.Nil(t, err) + + // Assert + assert.Equal(t, "1.2.3", version) +} + +func initRepository(t *testing.T) (*git.Repository, *git.Worktree) { + fs := memfs.New() + storage := memory.NewStorage() + + repository, err := git.Init(storage, fs) + assert.Nil(t, err) + + worktree, err := repository.Worktree() + assert.Nil(t, err) + + return repository, worktree +} + +func commitMultiple(t *testing.T, worktree *git.Worktree, messages ...string) plumbing.Hash { + var hash plumbing.Hash + var err error + for _, msg := range messages { + hash, err = worktree.Commit(msg, &git.CommitOptions{Author: defaultSignature()}) + assert.Nil(t, err) + } + + return hash } func TestTrimBranchPrefix(t *testing.T) { @@ -220,41 +404,3 @@ func defaultSignature() *object.Signature { When: when, } } - -// this method will be available in go-git after 4.7.0 -func createTag(r *git.Repository, h plumbing.Hash, name string, annotated bool) (plumbing.Hash, error) { - rawobj, err := object.GetObject(r.Storer, h) - if err != nil { - return plumbing.ZeroHash, err - } - - rname := plumbing.ReferenceName(path.Join("refs", "tags", name)) - - var target plumbing.Hash - if annotated { - tag := object.Tag{ - Name: name, - Message: "", - TargetType: rawobj.Type(), - Target: h, - } - obj := r.Storer.NewEncodedObject() - if err := tag.Encode(obj); err != nil { - return plumbing.ZeroHash, err - } - - target, err = r.Storer.SetEncodedObject(obj) - if err := tag.Encode(obj); err != nil { - return plumbing.ZeroHash, err - } - } else { - target = h - } - - ref := plumbing.NewHashReference(rname, target) - if err = r.Storer.SetReference(ref); err != nil { - return plumbing.ZeroHash, err - } - - return ref.Hash(), nil -}