From 7708de8591a7eaa8a2f397a582990bd88df18f59 Mon Sep 17 00:00:00 2001 From: Matt Farina Date: Thu, 5 Jan 2017 10:36:38 -0500 Subject: [PATCH] Adding gometalinter testing Cleaned up code based on linter feedback --- .travis.yml | 6 ++++-- Makefile | 36 ++++++++++++++++++++++++++++++++++++ bzr.go | 23 ++++++++--------------- bzr_test.go | 16 ++++++++-------- errors.go | 2 +- git.go | 16 ++++------------ git_test.go | 16 ++++++++-------- hg.go | 14 +++----------- hg_test.go | 18 +++++++++--------- repo_test.go | 4 ++-- svn.go | 20 +++++++++----------- svn_test.go | 14 +++++++------- vcs_local_lookup.go | 10 +++++----- vcs_remote_lookup.go | 31 ------------------------------- vcs_remote_lookup_test.go | 10 +++++----- 15 files changed, 109 insertions(+), 127 deletions(-) create mode 100644 Makefile diff --git a/.travis.yml b/.travis.yml index d094718..b14aeb8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,6 @@ language: go go: - - 1.3 - - 1.4 - 1.5 - 1.6 - 1.7 @@ -18,6 +16,10 @@ before_script: # - http://docs.travis-ci.com/user/workers/standard-infrastructure/ sudo: false +script: + - GO15VENDOREXPERIMENT=1 make setup + - GO15VENDOREXPERIMENT=1 make test + notifications: webhooks: urls: diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..a7a1b4e --- /dev/null +++ b/Makefile @@ -0,0 +1,36 @@ +.PHONY: setup +setup: + go get -u gopkg.in/alecthomas/gometalinter.v1 + gometalinter.v1 --install + +.PHONY: test +test: validate lint + @echo "==> Running tests" + go test -v + +.PHONY: validate +validate: + @echo "==> Running static validations" + @gometalinter.v1 \ + --disable-all \ + --enable deadcode \ + --severity deadcode:error \ + --enable gofmt \ + --enable gosimple \ + --enable ineffassign \ + --enable misspell \ + --enable vet \ + --tests \ + --vendor \ + --deadline 60s \ + ./... || exit_code=1 + +.PHONY: lint +lint: + @echo "==> Running linters" + @gometalinter.v1 \ + --disable-all \ + --enable golint \ + --vendor \ + --deadline 60s \ + ./... || : diff --git a/bzr.go b/bzr.go index e8f55b6..04d7998 100644 --- a/bzr.go +++ b/bzr.go @@ -39,7 +39,7 @@ func NewBzrRepo(remote, local string) (*BzrRepo, error) { // http://bazaar.launchpad.net/~mattfarina/govcstestbzrrepo/trunk/. Notice // the change from https to http and the path chance. // Here we set the remote to be the local one if none is passed in. - if err == nil && r.CheckLocal() == true && remote == "" { + if err == nil && r.CheckLocal() && remote == "" { c := exec.Command("bzr", "info") c.Dir = local c.Env = envForDir(c.Dir) @@ -226,11 +226,7 @@ func (s *BzrRepo) Tags() ([]string, error) { // commit id or tag. func (s *BzrRepo) IsReference(r string) bool { _, err := s.RunFromDir("bzr", "revno", "-r", r) - if err == nil { - return true - } - - return false + return err == nil } // IsDirty returns if the checkout has been modified from the checked @@ -308,21 +304,14 @@ func (s *BzrRepo) Ping() bool { // an error is returned. Launchpad returns a 404 for a codebase that // does not exist. Otherwise it returns a JSON object describing it. _, er := get("https://api.launchpad.net/1.0/" + try) - if er == nil { - return true - } - return false + return er == nil } } // This is the same command that Go itself uses but it's not fast (or fast // enough by my standards). A faster method would be useful. _, err = s.run("bzr", "info", s.Remote()) - if err != nil { - return false - } - - return true + return err == nil } // ExportDir exports the current revision to the passed in directory. @@ -340,6 +329,10 @@ func (s *BzrRepo) ExportDir(dir string) error { // https://bazaar.launchpad.net/~bzr-pqm/bzr/bzr.dev/files/head:/po/ func (s *BzrRepo) isUnableToCreateDir(err error) bool { msg := err.Error() + + // Note, linting find adres as a spelling issue. This is a bug in the + // misspelling linter and the issue has been filed. + // https://github.com/client9/misspell/issues/99 if strings.HasPrefix(msg, fmt.Sprintf("Parent directory of %s does not exist.", s.LocalPath())) || strings.HasPrefix(msg, fmt.Sprintf("Nadřazený adresář %s neexistuje.", s.LocalPath())) || strings.HasPrefix(msg, fmt.Sprintf("El directorio padre de %s no existe.", s.LocalPath())) || diff --git a/bzr_test.go b/bzr_test.go index e12c6d3..385fd18 100644 --- a/bzr_test.go +++ b/bzr_test.go @@ -55,7 +55,7 @@ func TestBzr(t *testing.T) { } // Verify Bzr repo is a Bzr repo - if repo.CheckLocal() == false { + if !repo.CheckLocal() { t.Error("Problem checking out repo or Bzr CheckLocal is not working") } @@ -75,7 +75,7 @@ func TestBzr(t *testing.T) { t.Error(nrerr) } // Verify the right oject is returned. It will check the local repo type. - if nrepo.CheckLocal() == false { + if !nrepo.CheckLocal() { t.Error("Wrong version returned from NewRepo") } @@ -164,15 +164,15 @@ func TestBzr(t *testing.T) { t.Error("Bzr is incorrectly returning branches") } - if repo.IsReference("1.0.0") != true { + if !repo.IsReference("1.0.0") { t.Error("Bzr is reporting a reference is not one") } - if repo.IsReference("foo") == true { - t.Error("Bzr is reporting a non-existant reference is one") + if repo.IsReference("foo") { + t.Error("Bzr is reporting a non-existent reference is one") } - if repo.IsDirty() == true { + if repo.IsDirty() { t.Error("Bzr incorrectly reporting dirty") } @@ -227,7 +227,7 @@ func TestBzr(t *testing.T) { _, err = os.Stat(filepath.Join(exportDir, string(repo.Vcs()))) if err != nil { - if found := os.IsNotExist(err); found == false { + if found := os.IsNotExist(err); !found { t.Errorf("Error checking exported metadata in Bzr: %s", err) } } else { @@ -250,7 +250,7 @@ func TestBzrCheckLocal(t *testing.T) { }() repo, _ := NewBzrRepo("", tempDir) - if repo.CheckLocal() == true { + if repo.CheckLocal() { t.Error("Bzr CheckLocal does not identify non-Bzr location") } diff --git a/errors.go b/errors.go index ea8c5fc..be70970 100644 --- a/errors.go +++ b/errors.go @@ -5,7 +5,7 @@ import "errors" // The vcs package provides ways to work with errors that hide the underlying // implementation details but make them accessible if needed. For basic errors // that do not have underlying implementation specific details or the underlying -// details are likely not necessairy there are errors for comparison. +// details are not necessary there are errors for comparison. // // For example: // diff --git a/git.go b/git.go index f2577ea..dcf6278 100644 --- a/git.go +++ b/git.go @@ -33,7 +33,7 @@ func NewGitRepo(remote, local string) (*GitRepo, error) { // Make sure the local Git repo is configured the same as the remote when // A remote value was passed in. - if err == nil && r.CheckLocal() == true { + if err == nil && r.CheckLocal() { c := exec.Command("git", "config", "--get", "remote.origin.url") c.Dir = local c.Env = envForDir(c.Dir) @@ -143,7 +143,7 @@ func (s *GitRepo) Update() error { return NewLocalError("Unable to update repository", err, "") } - if detached == true { + if detached { return nil } @@ -280,11 +280,7 @@ func (s *GitRepo) IsReference(r string) bool { // not been checked out yet. This next step should pickup the other // possible references. _, err = s.RunFromDir("git", "show-ref", r) - if err == nil { - return true - } - - return false + return err == nil } // IsDirty returns if the checkout has been modified from the checked @@ -364,11 +360,7 @@ func (s *GitRepo) Ping() bool { // remote needs to be different. c.Env = mergeEnvLists([]string{"GIT_TERMINAL_PROMPT=0"}, os.Environ()) _, err := c.CombinedOutput() - if err != nil { - return false - } - - return true + return err == nil } // ExportDir exports the current revision to the passed in directory. diff --git a/git_test.go b/git_test.go index d92ed46..7c6e093 100644 --- a/git_test.go +++ b/git_test.go @@ -54,7 +54,7 @@ func TestGit(t *testing.T) { } // Verify Git repo is a Git repo - if repo.CheckLocal() == false { + if !repo.CheckLocal() { t.Error("Problem checking out repo or Git CheckLocal is not working") } @@ -74,7 +74,7 @@ func TestGit(t *testing.T) { t.Error(nrerr) } // Verify the right oject is returned. It will check the local repo type. - if nrepo.CheckLocal() == false { + if !nrepo.CheckLocal() { t.Error("Wrong version returned from NewRepo") } @@ -198,15 +198,15 @@ func TestGit(t *testing.T) { t.Error("Git is incorrectly returning branches") } - if repo.IsReference("1.0.0") != true { + if !repo.IsReference("1.0.0") { t.Error("Git is reporting a reference is not one") } - if repo.IsReference("foo") == true { - t.Error("Git is reporting a non-existant reference is one") + if repo.IsReference("foo") { + t.Error("Git is reporting a non-existent reference is one") } - if repo.IsDirty() == true { + if repo.IsDirty() { t.Error("Git incorrectly reporting dirty") } @@ -261,7 +261,7 @@ func TestGit(t *testing.T) { _, err = os.Stat(filepath.Join(exportDir, string(repo.Vcs()))) if err != nil { - if found := os.IsNotExist(err); found == false { + if found := os.IsNotExist(err); !found { t.Errorf("Error checking exported metadata in Git: %s", err) } } else { @@ -284,7 +284,7 @@ func TestGitCheckLocal(t *testing.T) { }() repo, _ := NewGitRepo("", tempDir) - if repo.CheckLocal() == true { + if repo.CheckLocal() { t.Error("Git CheckLocal does not identify non-Git location") } diff --git a/hg.go b/hg.go index df41cd6..5000a6d 100644 --- a/hg.go +++ b/hg.go @@ -32,7 +32,7 @@ func NewHgRepo(remote, local string) (*HgRepo, error) { // Make sure the local Hg repo is configured the same as the remote when // A remote value was passed in. - if err == nil && r.CheckLocal() == true { + if err == nil && r.CheckLocal() { // An Hg repo was found so test that the URL there matches // the repo passed in here. c := exec.Command("hg", "paths") @@ -207,11 +207,7 @@ func (s *HgRepo) Tags() ([]string, error) { // commit id, branch, or tag. func (s *HgRepo) IsReference(r string) bool { _, err := s.RunFromDir("hg", "log", "-r", r) - if err == nil { - return true - } - - return false + return err == nil } // IsDirty returns if the checkout has been modified from the checked @@ -305,11 +301,7 @@ func (s *HgRepo) TagsFromCommit(id string) ([]string, error) { // Ping returns if remote location is accessible. func (s *HgRepo) Ping() bool { _, err := s.run("hg", "identify", s.Remote()) - if err != nil { - return false - } - - return true + return err == nil } // ExportDir exports the current revision to the passed in directory. diff --git a/hg_test.go b/hg_test.go index b8aa39a..6b19f72 100644 --- a/hg_test.go +++ b/hg_test.go @@ -55,7 +55,7 @@ func TestHg(t *testing.T) { } // Verify Hg repo is a Hg repo - if repo.CheckLocal() == false { + if !repo.CheckLocal() { t.Error("Problem checking out repo or Hg CheckLocal is not working") } @@ -75,7 +75,7 @@ func TestHg(t *testing.T) { t.Error(nrerr) } // Verify the right oject is returned. It will check the local repo type. - if nrepo.CheckLocal() == false { + if !nrepo.CheckLocal() { t.Error("Wrong version returned from NewRepo") } @@ -166,19 +166,19 @@ func TestHg(t *testing.T) { t.Error("Hg is incorrectly returning branches") } - if repo.IsReference("1.0.0") != true { + if !repo.IsReference("1.0.0") { t.Error("Hg is reporting a reference is not one") } - if repo.IsReference("test") != true { + if !repo.IsReference("test") { t.Error("Hg is reporting a reference is not one") } - if repo.IsReference("foo") == true { - t.Error("Hg is reporting a non-existant reference is one") + if repo.IsReference("foo") { + t.Error("Hg is reporting a non-existent reference is one") } - if repo.IsDirty() == true { + if repo.IsDirty() { t.Error("Hg incorrectly reporting dirty") } @@ -231,7 +231,7 @@ func TestHg(t *testing.T) { _, err = os.Stat(filepath.Join(exportDir, string(repo.Vcs()))) if err != nil { - if found := os.IsNotExist(err); found == false { + if found := os.IsNotExist(err); !found { t.Errorf("Error checking exported metadata in Hg: %s", err) } } else { @@ -254,7 +254,7 @@ func TestHgCheckLocal(t *testing.T) { }() repo, _ := NewHgRepo("", tempDir) - if repo.CheckLocal() == true { + if repo.CheckLocal() { t.Error("Hg CheckLocal does not identify non-Hg location") } diff --git a/repo_test.go b/repo_test.go index d61f6cb..fef0bcb 100644 --- a/repo_test.go +++ b/repo_test.go @@ -63,12 +63,12 @@ func TestTypeSwitch(t *testing.T) { func TestDepInstalled(t *testing.T) { i := depInstalled("git") - if i != true { + if !i { t.Error("depInstalled not finding installed dep.") } i = depInstalled("thisreallyisntinstalled") - if i != false { + if i { t.Error("depInstalled finding not installed dep.") } } diff --git a/svn.go b/svn.go index 888ae09..895f93e 100644 --- a/svn.go +++ b/svn.go @@ -33,7 +33,7 @@ func NewSvnRepo(remote, local string) (*SvnRepo, error) { // Make sure the local SVN repo is configured the same as the remote when // A remote value was passed in. - if err == nil && r.CheckLocal() == true { + if err == nil && r.CheckLocal() { // An SVN repo was found so test that the URL there matches // the repo passed in here. out, err := exec.Command("svn", "info", local).CombinedOutput() @@ -139,6 +139,9 @@ func (s *SvnRepo) Version() (string, error) { } out, err := s.RunFromDir("svn", "info", "--xml") + if err != nil { + return "", NewLocalError("Unable to retrieve checked out version", err, string(out)) + } s.log(out) infos := &Info{} err = xml.Unmarshal(out, &infos) @@ -261,6 +264,9 @@ func (s *SvnRepo) CommitInfo(id string) (*CommitInfo, error) { } out, err := s.RunFromDir("svn", "info", "-r", id, "--xml") + if err != nil { + return nil, NewLocalError("Unable to retrieve commit information", err, string(out)) + } infos := &Info{} err = xml.Unmarshal(out, &infos) if err != nil { @@ -323,11 +329,7 @@ func (s *SvnRepo) TagsFromCommit(id string) ([]string, error) { // Ping returns if remote location is accessible. func (s *SvnRepo) Ping() bool { _, err := s.run("svn", "--non-interactive", "info", s.Remote()) - if err != nil { - return false - } - - return true + return err == nil } // ExportDir exports the current revision to the passed in directory. @@ -346,11 +348,7 @@ func (s *SvnRepo) ExportDir(dir string) error { // where the parent directory of the VCS local path doesn't exist. func (s *SvnRepo) isUnableToCreateDir(err error) bool { msg := err.Error() - if strings.HasPrefix(msg, "E000002") { - return true - } - - return false + return strings.HasPrefix(msg, "E000002") } // detectRemoteFromInfoCommand finds the remote url from the `svn info` diff --git a/svn_test.go b/svn_test.go index d242f3e..dfa0af8 100644 --- a/svn_test.go +++ b/svn_test.go @@ -54,7 +54,7 @@ func TestSvn(t *testing.T) { } // Verify SVN repo is a SVN repo - if repo.CheckLocal() == false { + if !repo.CheckLocal() { t.Error("Problem checking out repo or SVN CheckLocal is not working") } @@ -167,15 +167,15 @@ func TestSvn(t *testing.T) { t.Error("Svn is incorrectly returning branches") } - if repo.IsReference("r4") != true { + if !repo.IsReference("r4") { t.Error("Svn is reporting a reference is not one") } - if repo.IsReference("55") == true { - t.Error("Svn is reporting a non-existant reference is one") + if repo.IsReference("55") { + t.Error("Svn is reporting a non-existent reference is one") } - if repo.IsDirty() == true { + if repo.IsDirty() { t.Error("Svn incorrectly reporting dirty") } @@ -230,7 +230,7 @@ func TestSvn(t *testing.T) { _, err = os.Stat(filepath.Join(exportDir, string(repo.Vcs()))) if err != nil { - if found := os.IsNotExist(err); found == false { + if found := os.IsNotExist(err); !found { t.Errorf("Error checking exported metadata in Svn: %s", err) } } else { @@ -253,7 +253,7 @@ func TestSvnCheckLocal(t *testing.T) { }() repo, _ := NewSvnRepo("", tempDir) - if repo.CheckLocal() == true { + if repo.CheckLocal() { t.Error("SVN CheckLocal does not identify non-SVN location") } diff --git a/vcs_local_lookup.go b/vcs_local_lookup.go index f965132..0709f61 100644 --- a/vcs_local_lookup.go +++ b/vcs_local_lookup.go @@ -15,20 +15,20 @@ func DetectVcsFromFS(vcsPath string) (Type, error) { return "", ErrCannotDetectVCS } - seperator := string(os.PathSeparator) + separator := string(os.PathSeparator) // Walk through each of the different VCS types to see if // one can be detected. Do this is order of guessed popularity. - if _, err := os.Stat(vcsPath + seperator + ".git"); err == nil { + if _, err := os.Stat(vcsPath + separator + ".git"); err == nil { return Git, nil } - if _, err := os.Stat(vcsPath + seperator + ".svn"); err == nil { + if _, err := os.Stat(vcsPath + separator + ".svn"); err == nil { return Svn, nil } - if _, err := os.Stat(vcsPath + seperator + ".hg"); err == nil { + if _, err := os.Stat(vcsPath + separator + ".hg"); err == nil { return Hg, nil } - if _, err := os.Stat(vcsPath + seperator + ".bzr"); err == nil { + if _, err := os.Stat(vcsPath + separator + ".bzr"); err == nil { return Bzr, nil } diff --git a/vcs_remote_lookup.go b/vcs_remote_lookup.go index 1b2c691..6689f95 100644 --- a/vcs_remote_lookup.go +++ b/vcs_remote_lookup.go @@ -266,37 +266,6 @@ func checkBitbucket(i map[string]string, ul *url.URL) (Type, error) { } -// Google supports Git, Hg, and Svn. The SVN style is only -// supported through their legacy setup at .googlecode.com. -// I wonder if anyone is actually using SVN support. -func checkGoogle(i map[string]string, u *url.URL) (Type, error) { - - // To figure out which of the VCS types is used in Google Code you need - // to parse a web page and find it. Ugh. I mean... ugh. - var hack = regexp.MustCompile(`id="checkoutcmd">(hg|git|svn)`) - - d, err := get(expand(i, "https://code.google.com/p/{project}/source/checkout?repo={repo}")) - if err != nil { - return "", err - } - - if m := hack.FindSubmatch(d); m != nil { - if vcs := string(m[1]); vcs != "" { - if vcs == "svn" { - // While Google supports SVN it can only be used with the legacy - // urls of .googlecode.com. I considered creating a new - // error for this problem but Google Code is going away and there - // is support for the legacy structure. - return "", ErrCannotDetectVCS - } - - return Type(vcs), nil - } - } - - return "", ErrCannotDetectVCS -} - // Expect a type key on i with the exact type detected from the regex. func checkURL(i map[string]string, u *url.URL) (Type, error) { return Type(i["type"]), nil diff --git a/vcs_remote_lookup_test.go b/vcs_remote_lookup_test.go index a4cf6b0..395ee79 100644 --- a/vcs_remote_lookup_test.go +++ b/vcs_remote_lookup_test.go @@ -49,15 +49,15 @@ func TestVCSLookup(t *testing.T) { for u, c := range urlList { ty, _, err := detectVcsFromRemote(u) - if err == nil && c.work == false { + if err == nil && !c.work { t.Errorf("Error detecting VCS from URL(%s)", u) } - if err == ErrCannotDetectVCS && c.work == true { + if err == ErrCannotDetectVCS && c.work { t.Errorf("Error detecting VCS from URL(%s)", u) } - if err != nil && c.work == true { + if err != nil && c.work { t.Errorf("Error detecting VCS from URL(%s): %s", u, err) } @@ -65,11 +65,11 @@ func TestVCSLookup(t *testing.T) { err != ErrCannotDetectVCS && !strings.HasSuffix(err.Error(), "Not Found") && !strings.HasSuffix(err.Error(), "Access Denied") && - c.work == false { + !c.work { t.Errorf("Unexpected error returned (%s): %s", u, err) } - if c.work == true && ty != c.t { + if c.work && ty != c.t { t.Errorf("Incorrect VCS type returned(%s)", u) } }