Repository: srvaroa/labeler (go)

Issue:
Ignore files in PR Size condition
Sometimes I want to be able to not include certain files in the calculation for number of changes. 

For example, when `yarn.lock` gets updated, it can typically include many changes that blows up the PR size condition. It would be awesome if somewhere I could list files that I don't want included in that calculation.

🙏🏻

Pull request:
size: allow excluding files
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.

fixes #61 

Signed-off-by: Galo Navarro <anglorvaroa@gmail.com>


Patch:
diff --git a/README.md b/README.md
index fd6712e..53a9aa5 100644
--- a/README.md
+++ b/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
diff --git a/cmd/action.go b/cmd/action.go
index 1921d9e..d6d900d 100644
--- a/cmd/action.go
+++ b/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
diff --git a/new_file b/new_file
new file mode 100644
index 0000000..ce01362
--- /dev/null
+++ b/new_file
@@ -0,0 +1,1 @@
+hello
diff --git a/pkg/condition_size.go b/pkg/condition_size.go
index 1e4d24e..847de88 100644
--- a/pkg/condition_size.go
+++ b/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
+}
diff --git a/pkg/labeler.go b/pkg/labeler.go
index 55192d2..38c07e9 100644
--- a/pkg/labeler.go
+++ b/pkg/labeler.go
@@ -1,7 +1,6 @@
 package labeler
 
 import (
-	"context"
 	"log"
 	"strings"
 
@@ -9,8 +8,9 @@ import (
 )
 
 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)


Test patch:
diff --git a/cmd/action_test.go b/cmd/action_test.go
index ae188f7..d2a2cb8 100644
--- a/cmd/action_test.go
+++ b/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",
 				},
 			},
 		},
diff --git a/pkg/labeler_test.go b/pkg/labeler_test.go
index e1b6fb8..e6792e6 100644
--- a/pkg/labeler_test.go
+++ b/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
+			},
+		},
 	}
 }
 
diff --git a/test_data/config_v1_composite_size.yml b/test_data/config_v1_composite_size.yml
index 57acf4f..3acf8f1 100644
--- a/test_data/config_v1_composite_size.yml
+++ b/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
diff --git a/test_data/diff_response b/test_data/diff_response
index 6873999..b1068e0 100644
--- a/test_data/diff_response
+++ b/test_data/diff_response
@@ -1,49 +1,71 @@
 diff --git a/README.md b/README.md
-index d8827a8..3ef9a3c 100644
+index fd6712e..6bc9d15 100644
 --- a/README.md
 +++ b/README.md
-@@ -30,7 +30,7 @@ jobs:
-         GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
- ```
-
--Then add a new file `./github/labeler.yml` with the configuration as
-+Then add a new file `.github/labeler.yml` with the configuration as
- described below in the `Configuration` section.
-
- This action will avoid failing in all cases, so if you're experiencing
-@@ -44,7 +44,7 @@ Typical errors are:
-
- ## Configuration
-
--Configuration can be stored at `./github/labeler.yml` as a plain list of
-+Configuration can be stored at `.github/labeler.yml` as a plain list of
+@@ -1,4 +1,4 @@
+-# Label manager for PRs and issues based on configurable conditions
++# Label manager for PRs and issues based on configurable conditions!!
+ 
+ [![labeler release (latest SemVer)](https://img.shields.io/github/v/release/srvaroa/labeler?sort=semver)](https://github.com/srvaroa/labeler/releases)  
+ 
+@@ -53,7 +53,7 @@ on:
+ jobs:
+   build:
+ 
+-    runs-on: ubuntu-latest
++    runs-on: ubuntu-latest!!
+ 
+     steps:
+     - uses: srvaroa/labeler@master
+@@ -104,7 +104,7 @@ errors are:
+   permissions to label the main repository ([issue for
+   solving this](https://github.com/srvaroa/labeler/issues/3))
+ 
+-## Configuring matching rules
++## Configuring matching ruleA!!s
+ 
+ Configuration can be stored at `.github/labeler.yml` as a plain list of
  label matchers, which consist of a label and a set of conditions for
- each.  When *all* conditions for a label match, then the Action will set
- the given label.  When *any* condition for a label does not match, then
-@@ -58,7 +58,7 @@ Here is an example of a matcher for label "Example":
- <condition_name>: <condition_parameters>
- ```
-
--For example, this `./github/labeler.yml` contains a single matcher with
-+For example, this `.github/labeler.yml` contains a single matcher with
- a single condition:
-
+@@ -379,4 +379,4 @@ This condition is satisfied when the title matches on the given regex.
+ 
  ```yaml
-@@ -76,7 +76,7 @@ Each label may combine multiple conditions.  The action combines all
- conditions with an AND operation.  That is, the label will be applied if
- *all* conditions are satisfied, removed otherwise.
-
--For example, given this `./github/labeler.yml`:
-+For example, given this `.github/labeler.yml`:
-
- ```yaml
- version: 1
-@@ -151,7 +151,7 @@ the PR is within given thresholds.
-  The number of changed lines is calculated as the sum of all `additions +
-  deletions` in the PR.
-
- -For example, given this `./github/labeler.yml`:
- +For example, given this `.github/labeler.yml`:
-
-  ```yaml
-  - label: "S"
\ No newline at end of file
+ title: "^WIP:.*"
+-```
++```!!
+diff --git a/dependabot.yml b/dependabot.yml
+deleted file mode 100644
+index f17f51b..0000000
+--- a/dependabot.yml
++++ /dev/null
+@@ -1,12 +0,0 @@
+-# To get started with Dependabot version updates, you'll need to specify which
+-# package ecosystems to update and where the package manifests are located.
+-# Please see the documentation for all configuration options:
+-# https://docs.github.com/github/administering-a-repository/configuration-options-for-dependency-updates
+-
+-version: 2
+-updates:
+-  - package-ecosystem: "" # See documentation for possible values
+-    directory: "/" # Location of package manifests
+-    schedule:
+-      interval: "weekly"
+-
+diff --git a/pkg/condition_title.go b/pkg/condition_title.go
+index 79886f4..2fa4ee1 100644
+--- a/pkg/condition_title.go
++++ b/pkg/condition_title.go
+@@ -18,6 +18,7 @@ func TitleCondition() Condition {
+ 			if len(matcher.Title) <= 0 {
+ 				return false, fmt.Errorf("title is not set in config")
+ 			}
++			log.Printf("A change")
+ 			log.Printf("Matching `%s` against: `%s`", matcher.Title, target.Title)
+ 			isMatched, _ := regexp.Match(matcher.Title, []byte(target.Title))
+ 			return isMatched, nil
+diff --git a/new_file b/new_file
+new file mode 100644
+index 0000000..ce01362
+--- /dev/null
++++ b/new_file
+@@ -0,0 +1 @@
++hello