diff --git a/check.go b/check.go index 2942369..00ad9d1 100644 --- a/check.go +++ b/check.go @@ -48,15 +48,57 @@ func Check(request CheckRequest, manager Github) (CheckResponse, error) { log.Println("ignore paths configured:", iPaths) log.Println("changed files found:", p.Files) - switch { // if `paths` is configured && NONE of the changed files match `paths` pattern/s - case pullrequest.Patterns(paths)(p) && !pullrequest.Files(paths, false)(p): - log.Println("paths excluded pull") - continue + if pullrequest.Patterns(paths)(p) { + matches, err := pullrequest.Files(paths)(p.Files) + if err != nil { + log.Println("error identifying matching paths") + continue + } + + if len(matches) == 0 { + log.Println("paths excluded pull") + continue + } + } + // if `ignore_paths` is configured && ALL of the changed files match `ignore_paths` pattern/s - case pullrequest.Patterns(iPaths)(p) && pullrequest.Files(iPaths, true)(p): - log.Println("ignore paths excluded pull") - continue + if pullrequest.Patterns(iPaths)(p) { + matches, err := pullrequest.Files(iPaths)(p.Files) + if err != nil { + log.Println("error identifying matching ignore_paths") + continue + } + + if len(matches) == len(p.Files) { + log.Println("ignore paths excluded pull") + continue + } + } + + // Both `paths` and `ignore_paths` are defined, it is possible for the pull request + // to contain files outside of `paths` + if pullrequest.Patterns(paths)(p) && pullrequest.Patterns(iPaths)(p) { + log.Println("paths and ignore_paths both defined") + matches, err := pullrequest.Files(paths)(p.Files) + if err != nil { + log.Println("error identifying matching paths when both paths and ignore_paths are defined") + continue + } + + if len(matches) > 0 { + ignoreMatches, err := pullrequest.Files(iPaths)(matches) + if err != nil { + log.Println("error identifying matching ignore_paths when both paths and ignore_paths are defined") + continue + } + + matches = diff(matches, ignoreMatches) + } + + if len(matches) == 0 { + continue + } } } @@ -83,6 +125,30 @@ func Check(request CheckRequest, manager Github) (CheckResponse, error) { return response, nil } +// diff returns elements in match that are not found in ignoreMatch +func diff(match []string, ignoreMatch []string) []string { + found := make(map[string]bool) + + for _, f := range match { + found[f] = true + } + + for _, f := range ignoreMatch { + if _, ok := found[f]; ok { + found[f] = false + } + } + + var diff []string + for file, ret := range found { + if ret { + diff = append(diff, file) + } + } + + return diff +} + func newVersion(r CheckRequest, p pullrequest.PullRequest) bool { switch { // negative filters diff --git a/pullrequest/filter.go b/pullrequest/filter.go index 2f38026..4cbb2cc 100644 --- a/pullrequest/filter.go +++ b/pullrequest/filter.go @@ -3,6 +3,7 @@ package pullrequest import ( "log" "regexp" + "strings" "time" glob "github.com/sabhiram/go-gitignore" @@ -191,37 +192,30 @@ func Patterns(patterns []string) Filter { } } -// Files matches the PRs changed files against a set of glob patterns: -// (invert == false) returns true if patterns empty OR any of the changed files match the patterns -// (invert == true) returns true if all of the changed files match the patterns to ignore -func Files(patterns []string, invert bool) Filter { - return func(p PullRequest) bool { - matched := make([]int8, 0) +// Files returns the files that match against a set of glob patterns: +func Files(patterns []string) func([]string) ([]string, error) { + return func(files []string) ([]string, error) { + matches := make([]string, 0) + gc, err := glob.CompileIgnoreLines(patterns...) if err != nil { - return false + return matches, err } - for _, f := range p.Files { + for _, f := range files { log.Println("comparing patterns to changed file:", f) - if gc.MatchesPath("/" + f) { - if !invert { - log.Println("paths: true -", "matched:", f) - return true - } - - matched = append(matched, 1) - log.Println("matched:", f) + fn := f + if !strings.HasPrefix(f, "/") { + fn = "/" + f } - } - log.Println("invert:", invert, "- matched:", len(matched), "- files:", len(p.Files)) - if invert && len(matched) == len(p.Files) { - log.Println("ignore paths: true") - return true + if gc.MatchesPath(fn) { + matches = append(matches, f) + } } - return false + log.Printf("found %d matching files", len(matches)) + return matches, nil } } diff --git a/pullrequest/functional_test.go b/pullrequest/functional_test.go index cd31eca..74d76d5 100644 --- a/pullrequest/functional_test.go +++ b/pullrequest/functional_test.go @@ -692,125 +692,166 @@ func TestFiles(t *testing.T) { tests := []struct { description string patterns []string - invert bool - pull pullrequest.PullRequest - expect bool + files []string + expect []string }{ { description: "match txt files @ root level", patterns: []string{"*.txt"}, - invert: false, - pull: pullrequest.PullRequest{ - Files: []string{ - "file1.txt", - }, + files: []string{ + "file1.txt", + }, + expect: []string{ + "file1.txt", }, - expect: true, }, { description: "match txt files at any level", patterns: []string{"**/*.txt"}, - invert: false, - pull: pullrequest.PullRequest{ - Files: []string{ - "test/file2.txt", - "test/testing/file2.txt", - "test/testing/tested/file2.txt", - }, + files: []string{ + "test/file2.txt", + "test/testing/file2.txt", + "test/testing/tested/file2.txt", + }, + expect: []string{ + "test/file2.txt", + "test/testing/file2.txt", + "test/testing/tested/file2.txt", }, - expect: true, }, { description: "match any file in test dir", patterns: []string{"test/*"}, - invert: false, - pull: pullrequest.PullRequest{ - Files: []string{ - "file1.txt", - "test/file2.txt", - }, + files: []string{ + "file1.txt", + "test/file2.txt", + }, + expect: []string{ + "test/file2.txt", }, - expect: true, }, { description: "match any file recursively in test dir", patterns: []string{"test/**"}, - invert: false, - pull: pullrequest.PullRequest{ - Files: []string{ - "file1.txt", - "test/testing/file2.txt", - "test/testing/tested/file2.txt", - }, + files: []string{ + "file1.txt", + "test/testing/file2.txt", + "test/testing/tested/file2.txt", + }, + expect: []string{ + "test/testing/file2.txt", + "test/testing/tested/file2.txt", + }, + }, + // { + // description: "no match in test subdirectory", + // patterns: []string{"test/**"}, + // files: []string{ + // "file1.txt", + // "testing/test/file2.txt", + // "testing/test/tested/file2.txt", + // }, + // expect: []string{}, + // }, + { + description: "No match in test root directory", + patterns: []string{"/test/**"}, + files: []string{ + "file1.txt", + "testing/test/file2.txt", + "testing/test/tested/file2.txt", + }, + expect: []string{}, + }, + { + description: "Match in test root", + patterns: []string{"/test/*"}, + files: []string{ + "file1.txt", + "testing/test/file2.txt", + "testing/test/tested/file2.txt", + "/test/file3.txt", + }, + expect: []string{ + "/test/file3.txt", }, - expect: true, }, { description: "match multiple files", patterns: []string{"ci/dockerfiles/**/*", "ci/dockerfiles/*", "ci/tasks/build-image.yml", "ci/pipelines/images.yml"}, - invert: false, - pull: pullrequest.PullRequest{ - Files: []string{ - "ci/Makefile", - "ci/pipelines/images.yml", - "terraform/Makefile", - }, + files: []string{ + "ci/Makefile", + "ci/pipelines/images.yml", + "terraform/Makefile", + }, + expect: []string{ + "ci/pipelines/images.yml", }, - expect: true, }, { description: "no match /**/*", patterns: []string{"/ci/**/*"}, - invert: false, - pull: pullrequest.PullRequest{ - Files: []string{ - "src/teams/myteam/README.md", - "src/teams/myteam/ci/pipeline.yml", - }, + files: []string{ + "src/teams/myteam/README.md", + "src/teams/myteam/ci/pipeline.yml", }, - expect: false, + expect: []string{}, }, { description: "match /**/*", patterns: []string{"/ci/**/*"}, - invert: false, - pull: pullrequest.PullRequest{ - Files: []string{ - "ci/README.md", - "ci/pipeline.yml", - }, + files: []string{ + "ci/README.md", + "ci/pipeline.yml", + }, + expect: []string{ + "ci/README.md", + "ci/pipeline.yml", }, - expect: true, }, { description: "match **/*", patterns: []string{"src/teams/myteam/**/*"}, - invert: false, - pull: pullrequest.PullRequest{ - Files: []string{ - "src/teams/myteam/README.md", - "src/teams/myteam/ci/pipeline.yml", - }, + files: []string{ + "src/teams/myteam/README.md", + "src/teams/myteam/ci/pipeline.yml", + }, + expect: []string{ + "src/teams/myteam/README.md", + "src/teams/myteam/ci/pipeline.yml", }, - expect: true, }, { description: "match multiple files 2", patterns: []string{"src/teams/myteam/**/*", "src/teams/myteam/*"}, - invert: false, - pull: pullrequest.PullRequest{ - Files: []string{ - "src/teams/myteam/README.md", - "src/teams/myteam/ci/pipeline.yml", - }, + files: []string{ + "src/teams/myteam/README.md", + "src/teams/myteam/ci/pipeline.yml", + }, + expect: []string{ + "src/teams/myteam/README.md", + "src/teams/myteam/ci/pipeline.yml", + }, + }, + { + description: "match paths ending in file separator", + patterns: []string{"src/teams/myteam/Dockerfile", "src/teams/myteam/ci/", "src/teams/myteam/app/"}, + files: []string{ + "src/teams/myteam/ci/README.md", + "src/teams/myteam/ci/main.go", + "src/teams/yourteam/temp.go", + }, + expect: []string{ + "src/teams/myteam/ci/README.md", + "src/teams/myteam/ci/main.go", }, - expect: true, }, } for _, tc := range tests { t.Run(tc.description, func(t *testing.T) { - out := pullrequest.Files(tc.patterns, tc.invert)(tc.pull) + out, err := pullrequest.Files(tc.patterns)(tc.files) + assert.Nil(t, err) assert.Equal(t, tc.expect, out) }) }