Skip to content

Commit

Permalink
linters: ci/use now checks for CI configurations at the Git root of…
Browse files Browse the repository at this point in the history
… the given project, fixing false negative on this rule when analysing project in subfolder of a monorepo. Implements git.GetGitRoot() to retrieve the root of the Git repo. Refactors the CI linter's tests to be based on files generated in a temp dir, rather than in this repo.
  • Loading branch information
bvobart committed May 28, 2021
1 parent addf81b commit 12832b6
Show file tree
Hide file tree
Showing 15 changed files with 153 additions and 29 deletions.
2 changes: 1 addition & 1 deletion linters/ci/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (l *CILinter) LintProject(project api.Project) (api.Report, error) {
for _, provider := range providers {
// if the repo is not tracking the CI config file, then they're not really using CI,
// they're merely trying to define it, which is at least a step in the right direction.
if !git.IsTracking(project.Dir, provider.ConfigFile()) {
if !git.IsTracking(project.Dir, provider.ConfigFile(project.Dir)) {
report.Scores[RuleUseCI] = 25
}
}
Expand Down
61 changes: 52 additions & 9 deletions linters/ci/linter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,29 @@ import (
"github.com/bvobart/mllint/api"
"github.com/bvobart/mllint/linters/ci"
"github.com/bvobart/mllint/setools/ciproviders"
"github.com/bvobart/mllint/utils/exec"
)

const tmpkey = "mllint-tests-ci"

func createTestGitDir(t *testing.T, name string) string {
dir, err := ioutil.TempDir(os.TempDir(), tmpkey+"-"+name)
require.NoError(t, err)
_, err = exec.CommandOutput(dir, "git", "init")
require.NoError(t, err)
return dir
}

func TestCILinter(t *testing.T) {
linter := ci.NewLinter()
require.Equal(t, "Continuous Integration (CI)", linter.Name())
require.Equal(t, []*api.Rule{&ci.RuleUseCI}, linter.Rules())

t.Run("None", func(t *testing.T) {
project := api.Project{Dir: "test-resources/none"}
dir := createTestGitDir(t, "")
project := api.Project{Dir: dir}
defer os.RemoveAll(dir)

report, err := linter.LintProject(project)
require.NoError(t, err)

Expand All @@ -29,40 +43,69 @@ func TestCILinter(t *testing.T) {
})

t.Run("Azure", func(t *testing.T) {
project := api.Project{Dir: "test-resources/azure"}
dir := createTestGitDir(t, "azure")
project := api.Project{Dir: dir}
defer os.RemoveAll(dir)

require.NoError(t, ioutil.WriteFile(ciproviders.Azure{}.ConfigFile(dir), []byte("\n"), 0644))
_, err := exec.CommandOutput(dir, "git", "add", ".")
require.NoError(t, err)

report, err := linter.LintProject(project)
require.NoError(t, err)
require.EqualValues(t, 100, report.Scores[ci.RuleUseCI])
})

t.Run("GitHubActions", func(t *testing.T) {
project := api.Project{Dir: "test-resources/ghactions"}
dir := createTestGitDir(t, "ghactions")
project := api.Project{Dir: dir}
defer os.RemoveAll(dir)

require.NoError(t, os.MkdirAll(ciproviders.GHActions{}.ConfigFile(dir), 0755))
require.NoError(t, ioutil.WriteFile(path.Join(ciproviders.GHActions{}.ConfigFile(dir), "workflow.yml"), []byte("\n"), 0644))
_, err := exec.CommandOutput(dir, "git", "add", ".")
require.NoError(t, err)

report, err := linter.LintProject(project)
require.NoError(t, err)
require.EqualValues(t, 100, report.Scores[ci.RuleUseCI])
})

t.Run("GitlabCI", func(t *testing.T) {
project := api.Project{Dir: "test-resources/gitlab"}
dir := createTestGitDir(t, "gitlab")
project := api.Project{Dir: dir}
defer os.RemoveAll(dir)

require.NoError(t, ioutil.WriteFile(ciproviders.Gitlab{}.ConfigFile(dir), []byte("\n"), 0644))
_, err := exec.CommandOutput(dir, "git", "add", ".")
require.NoError(t, err)

report, err := linter.LintProject(project)
require.NoError(t, err)
require.EqualValues(t, 100, report.Scores[ci.RuleUseCI])
})

t.Run("Travis", func(t *testing.T) {
project := api.Project{Dir: "test-resources/travis"}
dir := createTestGitDir(t, "travis")
project := api.Project{Dir: dir}
defer os.RemoveAll(dir)

require.NoError(t, ioutil.WriteFile(ciproviders.Travis{}.ConfigFile(dir), []byte("\n"), 0644))
_, err := exec.CommandOutput(dir, "git", "add", ".")
require.NoError(t, err)

report, err := linter.LintProject(project)
require.NoError(t, err)
require.EqualValues(t, 100, report.Scores[ci.RuleUseCI])
})

t.Run("GitlabUntracked", func(t *testing.T) {
dir, err := ioutil.TempDir("test-resources", "gitlab-untracked")
require.NoError(t, err)
require.NoError(t, ioutil.WriteFile(path.Join(dir, ciproviders.Gitlab{}.ConfigFile()), []byte("\n"), 0644))
dir := createTestGitDir(t, "gitlab-untracked")
project := api.Project{Dir: dir}
defer os.RemoveAll(dir)

project := api.Project{Dir: dir}
require.NoError(t, ioutil.WriteFile(ciproviders.Gitlab{}.ConfigFile(dir), []byte("\n"), 0644))

report, err := linter.LintProject(project)
require.NoError(t, err)
require.EqualValues(t, 25, report.Scores[ci.RuleUseCI])
Expand Down
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
7 changes: 4 additions & 3 deletions setools/ciproviders/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@ package ciproviders
import (
"path"

"github.com/bvobart/mllint/setools/git"
"github.com/bvobart/mllint/utils"
)

const azureFile = "azure-pipelines.yml"

type Azure struct{}

func (_ Azure) ConfigFile() string {
return azureFile
func (_ Azure) ConfigFile(projectdir string) string {
return path.Join(git.GetGitRoot(projectdir), azureFile)
}

func (_ Azure) Detect(projectdir string) bool {
return utils.FileExists(path.Join(projectdir, azureFile))
return utils.FileExists(path.Join(git.GetGitRoot(projectdir), azureFile))
}

func (_ Azure) Type() ProviderType {
Expand Down
13 changes: 5 additions & 8 deletions setools/ciproviders/ghactions.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,26 @@ package ciproviders
import (
"path"

"github.com/bvobart/mllint/setools/git"
"github.com/bvobart/mllint/utils"
)

const ghactionsFolder = ".github/workflows"

type GHActions struct{}

func (_ GHActions) ConfigFile() string {
return ghactionsFolder
func (_ GHActions) ConfigFile(projectdir string) string {
return path.Join(git.GetGitRoot(projectdir), ghactionsFolder)
}

func (_ GHActions) Detect(projectdir string) bool {
workflowsdir := path.Join(projectdir, ghactionsFolder)
workflowsdir := path.Join(git.GetGitRoot(projectdir), ghactionsFolder)
if !utils.FolderExists(workflowsdir) {
return false
}

isEmpty, err := utils.FolderIsEmpty(workflowsdir)
if err != nil {
return false
}

return !isEmpty
return err == nil && !isEmpty
}

func (_ GHActions) Type() ProviderType {
Expand Down
7 changes: 4 additions & 3 deletions setools/ciproviders/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@ package ciproviders
import (
"path"

"github.com/bvobart/mllint/setools/git"
"github.com/bvobart/mllint/utils"
)

const gitlabFile = ".gitlab-ci.yml"

type Gitlab struct{}

func (_ Gitlab) ConfigFile() string {
return gitlabFile
func (_ Gitlab) ConfigFile(projectdir string) string {
return path.Join(git.GetGitRoot(projectdir), gitlabFile)
}

func (_ Gitlab) Detect(projectdir string) bool {
return utils.FileExists(path.Join(projectdir, gitlabFile))
return utils.FileExists(path.Join(git.GetGitRoot(projectdir), gitlabFile))
}

func (_ Gitlab) Type() ProviderType {
Expand Down
7 changes: 5 additions & 2 deletions setools/ciproviders/providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ var all = []Provider{azure, ghActions, gitlab, travis}

type ProviderType string
type Provider interface {
// ConfigFile returns the location of the CI provider's configuration file, relative to the project's root.
ConfigFile() string
// ConfigFile returns the location of the CI provider's configuration file in a project.
ConfigFile(projectdir string) string

// Detects whether the project at the given location uses this provider. Checking for config file existance should be enough
Detect(projectdir string) bool
Expand All @@ -28,6 +28,9 @@ type Provider interface {
Type() ProviderType
}

// Detect detects CI providers in the root of the Git repository that the given folder is in.
// Often the Git root dir and the given folder will be the same, but not in the case of monorepo style repos.
// When the dir is not in a Git repo, then it will simply just check the dir.
func Detect(projectdir string) []Provider {
providers := []Provider{}
for _, p := range all {
Expand Down
28 changes: 28 additions & 0 deletions setools/ciproviders/providers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package ciproviders_test

import (
"os"
"path"
"testing"

"github.com/bvobart/mllint/setools/ciproviders"
"github.com/bvobart/mllint/setools/git"
"github.com/stretchr/testify/require"
)

func TestDetect(t *testing.T) {
dir := "."
providers := ciproviders.Detect(dir)
require.Equal(t, []ciproviders.Provider{ciproviders.GHActions{}}, providers)

dir = os.TempDir()
providers = ciproviders.Detect(dir)
require.Equal(t, []ciproviders.Provider{}, providers)
}

func TestConfigFile(t *testing.T) {
dir := "."
provider := ciproviders.GHActions{}
configDir := provider.ConfigFile(dir)
require.Equal(t, path.Join(git.GetGitRoot(dir), ".github", "workflows"), configDir)
}
7 changes: 4 additions & 3 deletions setools/ciproviders/travis.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@ package ciproviders
import (
"path"

"github.com/bvobart/mllint/setools/git"
"github.com/bvobart/mllint/utils"
)

const travisFile = ".travis.yml"

type Travis struct{}

func (_ Travis) ConfigFile() string {
return travisFile
func (_ Travis) ConfigFile(projectdir string) string {
return path.Join(git.GetGitRoot(projectdir), travisFile)
}

func (_ Travis) Detect(projectdir string) bool {
return utils.FileExists(path.Join(projectdir, travisFile))
return utils.FileExists(path.Join(git.GetGitRoot(projectdir), travisFile))
}

func (_ Travis) Type() ProviderType {
Expand Down
19 changes: 19 additions & 0 deletions setools/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package git

import (
"fmt"
"path"
"sort"
"strconv"
"strings"
Expand All @@ -17,6 +18,22 @@ func Detect(dir string) bool {
return err == nil
}

// If the given directory is in a Git repository,
// GetGitRoot checks whether the given folder is in a Git repository,
// then returns the absolute path to the root folder of that Git repository,
// or the dir given as argument when it is not a Git repo.
func GetGitRoot(dir string) string {
gitDir, err := exec.CommandOutput(dir, "git", "rev-parse", "--path-format=absolute", "--git-dir")
if err != nil {
return dir
}

rootDir := path.Dir(string(gitDir))
return rootDir
}

// MakeGitInfo creates a description of the Git repository in the given directory.
// Returns an empty api.GitInfo{} when the dir is not a Git repo.
func MakeGitInfo(dir string) api.GitInfo {
if !Detect(dir) {
return api.GitInfo{}
Expand All @@ -29,6 +46,7 @@ func MakeGitInfo(dir string) api.GitInfo {
return api.GitInfo{RemoteURL: remote, Commit: commit, Branch: branch, Dirty: dirty}
}

// GetRemoteURL returns the URL of the `origin` Git remote.
func GetRemoteURL(dir string) (string, error) {
output, err := exec.CommandOutput(dir, "git", "remote", "get-url", "origin")
return strings.TrimSpace(string(output)), err
Expand All @@ -44,6 +62,7 @@ func GetCurrentBranch(dir string) (string, error) {
return strings.TrimSpace(string(output)), err
}

// IsDirty returns true when there are changed files in the repository.
func IsDirty(dir string) bool {
_, err := exec.CommandOutput(dir, "git", "diff", "--no-ext-diff", "--quiet")
return err != nil
Expand Down
31 changes: 31 additions & 0 deletions setools/git/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@ import (
"io/ioutil"
"math"
"os"
"strings"
"testing"

"github.com/stretchr/testify/require"

"github.com/bvobart/mllint/api"
"github.com/bvobart/mllint/setools/git"
"github.com/bvobart/mllint/utils"
"github.com/bvobart/mllint/utils/exec"
)

Expand All @@ -23,6 +26,34 @@ func TestDetect(t *testing.T) {
require.False(t, git.Detect(dir))
}

func TestMakeGitInfo(t *testing.T) {
dir := "."
info := git.MakeGitInfo(dir)
require.Equal(t, "[email protected]:bvobart/mllint.git", info.RemoteURL)

dir = os.TempDir()
require.Equal(t, api.GitInfo{}, git.MakeGitInfo(dir))
}

func TestGetGitRoot(t *testing.T) {
dir := "."
rootDir := git.GetGitRoot(dir)
require.True(t, strings.HasPrefix(utils.AbsolutePath(dir), rootDir))

dir = os.TempDir()
rootDir = git.GetGitRoot(dir)
require.Equal(t, dir, rootDir)

// create Git repo in temp dir, expect that temp dir
dir, err := ioutil.TempDir(os.TempDir(), "mllint-tests-git-root")
require.NoError(t, err)
_, err = exec.CommandOutput(dir, "git", "init")
require.NoError(t, err)

rootDir = git.GetGitRoot(dir)
require.Equal(t, dir, rootDir)
}

func TestIsTracking(t *testing.T) {
dir := "."
require.True(t, git.IsTracking(dir, "git_test.go"))
Expand Down

0 comments on commit 12832b6

Please sign in to comment.