size: allow excluding files (#80)

The `size` condition now allows excluding a set of files. Changes in
excluded files will not be taken into account when applying the `above`
and `below` thresholds.

Signed-off-by: Galo Navarro <[email protected]>

README.md

@@ -368,6 +368,20 @@ reported by the [GitHub API](https://developer.github.com/v3/pulls).
 |Second example|5|42|M|
 |Third example|68|148|L|
 
+You can exclude some files so that their changes are not taken into
+account for the overall count. This can be useful for `yarn.lock`,
+`go.sum` and such. Use `exclude-files`:
+
+``yaml`
+- label: "L"
+    size:
+        exclude-files: ["yarn.lock"]
+        above: 100
+``` 
+
+This condition will apply the `L` label if the diff is above 100 lines,
+but NOT taking into account changes in `yarn.lock`.
+
 **NOTICE** the old format for specifying size properties (`size-above`
 and `size-below`) has been deprecated. The action will continue
 supporting old configs for now, but users are encouraged to migrate to

cmd/action.go

@@ -152,6 +152,8 @@ func getEventPayload() *[]byte {
 }
 
 func newLabeler(gh *github.Client, config *labeler.LabelerConfigV1) *labeler.Labeler {
+	ctx := context.Background()
+
 	l := labeler.Labeler{
 
 		FetchRepoConfig: func() (*labeler.LabelerConfigV1, error) {
@@ -176,7 +178,24 @@ func newLabeler(gh *github.Client, config *labeler.LabelerConfigV1) *labeler.Lab
 			}
 			return labels, err
 		},
-		GitHub: gh,
+		GitHubFacade: &labeler.GitHubFacade{
+			GetRawDiff: func(owner, repo string, prNumber int) (string, error) {
+				diff, _, err := gh.PullRequests.GetRaw(ctx,
+					owner, repo, prNumber,
+					github.RawOptions{github.Diff})
+				return diff, err
+			},
+			ListIssuesByRepo: func(owner, repo string) ([]*github.Issue, error) {
+				issues, _, err := gh.Issues.ListByRepo(ctx,
+					owner, repo, &github.IssueListByRepoOptions{})
+				return issues, err
+			},
+			ListPRs: func(owner, repo string) ([]*github.PullRequest, error) {
+				prs, _, err := gh.PullRequests.List(ctx,
+					owner, repo, &github.PullRequestListOptions{})
+				return prs, err
+			},
+		},
 		Client: labeler.NewDefaultHttpClient(),
 	}
 	return &l

cmd/action_test.go

@@ -220,8 +220,9 @@ func TestGetLabelerConfigV1WithCompositeSize(t *testing.T) {
 			{
 				Label: "M",
 				Size: &labeler.SizeConfig{
-					Above: "9",
-					Below: "100",
+					ExcludeFiles: []string{"test.yaml"},
+					Above:        "9",
+					Below:        "100",
 				},
 			},
 		},

new_file

@@ -0,0 +1 @@
+hello

pkg/condition_size.go

@@ -5,9 +5,12 @@ import (
 	"log"
 	"math"
 	"strconv"
+	"strings"
+
+	gh "github.com/google/go-github/v50/github"
 )
 
-func SizeCondition() Condition {
+func SizeCondition(l *Labeler) Condition {
 	return Condition{
 		GetName: func() string {
 			return "Pull Request contains a number of changes"
@@ -48,7 +51,8 @@ func SizeCondition() Condition {
 				lowerBound = 0
 				log.Printf("Lower boundary set to 0 (config has invalid or empty value)")
 			}
-			totalChanges := int64(math.Abs(float64(target.ghPR.GetAdditions() + target.ghPR.GetDeletions())))
+
+			totalChanges, err := l.getModifiedLinesCount(target.ghPR, realMatcher.ExcludeFiles)
 			log.Printf("Matching %d changes in PR against bounds: (%d, %d)", totalChanges, lowerBound, upperBound)
 			isWithinBounds := totalChanges > lowerBound && totalChanges < upperBound
 			return isWithinBounds, nil
@@ -63,3 +67,62 @@ func isNewConfig(matcher LabelMatcher) bool {
 func isOldConfig(matcher LabelMatcher) bool {
 	return matcher.SizeAbove != "" || matcher.SizeBelow != ""
 }
+
+func (l *Labeler) getModifiedLinesCount(pr *gh.PullRequest, excludedFiles []string) (int64, error) {
+
+	if len(excludedFiles) == 0 {
+		// no exclusions so we can just rely on GH's summary which is
+		// more lightweight
+		return int64(math.Abs(float64(pr.GetAdditions() + pr.GetDeletions()))), nil
+	}
+
+	// Get the diff for the pull request
+	urlParts := strings.Split(pr.GetBase().GetRepo().GetHTMLURL(), "/")
+	owner := urlParts[len(urlParts)-2]
+	repo := urlParts[len(urlParts)-1]
+	diff, err := l.GitHubFacade.GetRawDiff(owner, repo, pr.GetNumber())
+	if err != nil {
+		return 0, err
+	}
+
+	// Count the number of lines that start with "+" or "-"
+	var count int64
+	var countFile = false
+	for _, line := range strings.Split(diff, "\n") {
+		if line == "+++ /dev/null" || line == "--- /dev/null" {
+			// ignore, these are removed or added files
+			continue
+		}
+		if strings.HasPrefix(line, "+++") || strings.HasPrefix(line, "---") {
+			// We're in a file's block
+			path := strings.TrimPrefix(line, "---")
+			path = strings.TrimPrefix(path, "+++")
+			path = strings.TrimSpace(path)
+			// Check if the file path matches any of the excluded files
+			countFile = !isFileExcluded(path, excludedFiles)
+			if countFile {
+				log.Printf("Counting changes in file %s", path)
+			} else {
+				log.Printf("Ignoring file %s", path)
+			}
+			continue
+		}
+		if countFile && (strings.HasPrefix(line, "+") || strings.HasPrefix(line, "-")) {
+			log.Printf("Count line %s", line)
+			count++
+		}
+	}
+
+	log.Printf("Total count %d", count)
+
+	return count, nil
+}
+
+func isFileExcluded(path string, excludedFiles []string) bool {
+	for _, excludedFile := range excludedFiles {
+		if strings.HasSuffix(path, excludedFile) {
+			return true
+		}
+	}
+	return false
+}

pkg/labeler.go

@@ -1,16 +1,16 @@
 package labeler
 
 import (
-	"context"
 	"log"
 	"strings"
 
 	gh "github.com/google/go-github/v50/github"
 )
 
 type SizeConfig struct {
-	Above string
-	Below string
+	ExcludeFiles []string `yaml:"exclude-files"`
+	Above        string
+	Below        string
 }
 
 type LabelMatcher struct {
@@ -55,11 +55,18 @@ type LabelUpdates struct {
 	set map[string]bool
 }
 
+// Just to make this mockable..
+type GitHubFacade struct {
+	GetRawDiff       func(owner, repo string, prNumber int) (string, error)
+	ListIssuesByRepo func(owner, repo string) ([]*gh.Issue, error)
+	ListPRs          func(owner, repo string) ([]*gh.PullRequest, error)
+}
+
 type Labeler struct {
 	FetchRepoConfig  func() (*LabelerConfigV1, error)
 	ReplaceLabels    func(target *Target, labels []string) error
 	GetCurrentLabels func(target *Target) ([]string, error)
-	GitHub           *gh.Client
+	GitHubFacade     *GitHubFacade
 	Client           HttpClient
 }
 
@@ -204,7 +211,7 @@ func (l *Labeler) findMatches(target *Target, config *LabelerConfigV1) (LabelUpd
 		FilesCondition(l),
 		IsDraftCondition(),
 		IsMergeableCondition(),
-		SizeCondition(),
+		SizeCondition(l),
 		TitleCondition(),
 	}
 
@@ -272,10 +279,7 @@ func (l *Labeler) ProcessAllIssues(owner, repo string) {
 			"process issues in the scheduled execution mode")
 	}
 
-	issues, _, err := l.GitHub.Issues.ListByRepo(
-		context.Background(),
-		owner, repo,
-		&gh.IssueListByRepoOptions{})
+	issues, err := l.GitHubFacade.ListIssuesByRepo(owner, repo)
 
 	if err != nil {
 		log.Printf("Unable to list issues in %s/%s: %+v", owner, repo, err)
@@ -290,10 +294,7 @@ func (l *Labeler) ProcessAllIssues(owner, repo string) {
 
 func (l *Labeler) ProcessAllPRs(owner, repo string) {
 
-	prs, _, err := l.GitHub.PullRequests.List(
-		context.Background(),
-		owner, repo,
-		&gh.PullRequestListOptions{})
+	prs, err := l.GitHubFacade.ListPRs(owner, repo)
 
 	if err != nil {
 		log.Printf("Unable to list pull requests in %s/%s: %+v", owner, repo, err)

pkg/labeler_test.go

@@ -293,6 +293,30 @@ func TestHandleEvent(t *testing.T) {
 			initialLabels:  []string{},
 			expectedLabels: []string{"L"},
 		},
+		{
+			event:    "pull_request",
+			payloads: []string{"big_pr"},
+			name:     "Test the size_above rule applying file exclusions",
+			config: LabelerConfigV1{
+				Version: 1,
+				Labels: []LabelMatcher{
+					{
+						Label: "L",
+						Size: &SizeConfig{
+							ExcludeFiles: []string{"README.md", "new_file", "dependabot.yml"},
+							// our test file has a diff in four files,
+							// including added/removed which have a
+							// slightly trickier diff.  Adding any of
+							// these will get us above 2 lines and fail
+							// the test.
+							Below: "2",
+						},
+					},
+				},
+			},
+			initialLabels:  []string{},
+			expectedLabels: []string{"L"},
+		},
 		{
 			event:    "pull_request",
 			payloads: []string{"small_pr"},
@@ -725,6 +749,17 @@ func NewTestLabeler(t *testing.T, tc TestCase) Labeler {
 				tc.name, tc.expectedLabels, labels)
 		},
 		Client: &FakeHttpClient{},
+		GitHubFacade: &GitHubFacade{
+			GetRawDiff: func(owner, repo string, prNumber int) (string, error) {
+				file, err := os.Open("../test_data/diff_response")
+				if err != nil {
+					return "", err
+				}
+
+				data, err := ioutil.ReadAll(file)
+				return string(data), nil
+			},
+		},
 	}
 }
 

test_data/config_v1_composite_size.yml

@@ -5,5 +5,6 @@ labels:
     size-above: 1
   - label: M
     size:
+      exclude-files: ["test.yaml"]
       above: 9
       below: 100