Repository: gembaadvantage/uplift
Pull request #312 (2023-01-17 21:23:03)

fix: changelog include incorrectly compresses log while filtering

codecov-commenter: # [Codecov](https://codecov.io/gh/gembaadvantage/uplift/pull/312?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gembaadvantage) Report
> Merging [#312](https://codecov.io/gh/gembaadvantage/uplift/pull/312?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gembaadvantage) (e52ccf3) into [main](https://codecov.io/gh/gembaadvantage/uplift/commit/b0023f5f7b5f7315448b0434613c74ce67695e8f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gembaadvantage) (b0023f5) will **decrease** coverage by `0.12%`.
> The diff coverage is `85.18%`.

```diff
@@            Coverage Diff             @@
##             main     #312      +/-   ##
==========================================
- Coverage   83.03%   82.92%   -0.12%     
==========================================
  Files          43       43              
  Lines        2199     2202       +3     
==========================================
  Hits         1826     1826              
- Misses        297      299       +2     
- Partials       76       77       +1     
```


| [Impacted Files](https://codecov.io/gh/gembaadvantage/uplift/pull/312?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gembaadvantage) | Coverage Δ | |
|---|---|---|
| [internal/task/changelog/changelog.go](https://codecov.io/gh/gembaadvantage/uplift/pull/312?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gembaadvantage#diff-aW50ZXJuYWwvdGFzay9jaGFuZ2Vsb2cvY2hhbmdlbG9nLmdv) | `67.64% <85.18%> (-0.87%)` | :arrow_down: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gembaadvantage). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gembaadvantage)


Patch:
diff --git a/internal/task/changelog/changelog.go b/internal/task/changelog/changelog.go
index 553b2ae..6fd8837 100644
--- a/internal/task/changelog/changelog.go
+++ b/internal/task/changelog/changelog.go
@@ -76,9 +76,6 @@ type release struct {
 	Changes []git.LogEntry
 }
 
-// Define functions for filtering commits
-type commitFilter func([]git.LogEntry, *regexp.Regexp) []git.LogEntry
-
 // Task that generates a changelog for the current repository
 type Task struct{}
 
@@ -175,7 +172,7 @@ func changelogRelease(ctx *context.Context) ([]release, error) {
 
 	if len(ctx.Changelog.Include) > 0 {
 		log.Info("cherry-picking commits based on include list")
-		ents, err = filterCommits(ents, ctx.Changelog.Include, includeCommits)
+		ents, err = includeCommits(ents, ctx.Changelog.Include)
 		if err != nil {
 			return []release{}, err
 		}
@@ -183,7 +180,7 @@ func changelogRelease(ctx *context.Context) ([]release, error) {
 
 	if len(ctx.Changelog.Exclude) > 0 {
 		log.Info("removing commits based on exclude list")
-		ents, err = filterCommits(ents, ctx.Changelog.Exclude, excludeCommits)
+		ents, err = excludeCommits(ents, ctx.Changelog.Exclude)
 		if err != nil {
 			return []release{}, err
 		}
@@ -247,7 +244,7 @@ func changelogReleases(ctx *context.Context) ([]release, error) {
 
 		if len(ctx.Changelog.Include) > 0 {
 			log.Info("cherry-picking commits based on include list")
-			ents, err = filterCommits(ents, ctx.Changelog.Include, includeCommits)
+			ents, err = includeCommits(ents, ctx.Changelog.Include)
 			if err != nil {
 				return []release{}, err
 			}
@@ -255,7 +252,7 @@ func changelogReleases(ctx *context.Context) ([]release, error) {
 
 		if len(ctx.Changelog.Exclude) > 0 {
 			log.Info("removing commits based on exclude list")
-			ents, err = filterCommits(ents, ctx.Changelog.Exclude, excludeCommits)
+			ents, err = excludeCommits(ents, ctx.Changelog.Exclude)
 			if err != nil {
 				return []release{}, err
 			}
@@ -364,35 +361,44 @@ func reverse(ents []git.LogEntry) {
 	}
 }
 
-func filterCommits(commits []git.LogEntry, regexes []string, filter commitFilter) ([]git.LogEntry, error) {
-	filteredCommits := commits
+func includeCommits(commits []git.LogEntry, regexes []string) ([]git.LogEntry, error) {
+	filtered := []git.LogEntry{}
 	for _, regex := range regexes {
-		rgx, err := regexp.Compile(regex)
+		includeRgx, err := regexp.Compile(regex)
 		if err != nil {
-			return filteredCommits, err
+			return filtered, err
+		}
+
+		// Iterate over the entire list of log entries for each regex and
+		// append any match to the filtered list
+		for _, commit := range commits {
+			if includeRgx.MatchString(commit.Message) {
+				filtered = append(filtered, commit)
+			}
 		}
-		filteredCommits = filter(filteredCommits, rgx)
 	}
 
-	return filteredCommits, nil
+	return filtered, nil
 }
 
-func includeCommits(commits []git.LogEntry, rgx *regexp.Regexp) []git.LogEntry {
-	filteredCommits := []git.LogEntry{}
-	for _, commit := range commits {
-		if rgx.MatchString(commit.Message) {
-			filteredCommits = append(filteredCommits, commit)
+func excludeCommits(commits []git.LogEntry, regexes []string) ([]git.LogEntry, error) {
+	filtered := commits
+	for _, regex := range regexes {
+		excludeRgx, err := regexp.Compile(regex)
+		if err != nil {
+			return filtered, err
 		}
-	}
-	return filteredCommits
-}
 
-func excludeCommits(commits []git.LogEntry, rgx *regexp.Regexp) []git.LogEntry {
-	filteredCommits := []git.LogEntry{}
-	for _, commit := range commits {
-		if !rgx.MatchString(commit.Message) {
-			filteredCommits = append(filteredCommits, commit)
+		// Repeat over the filtered list for every exclude, compressing the list
+		// of log entries on each iteration
+		filterPass := []git.LogEntry{}
+		for _, commit := range filtered {
+			if !excludeRgx.MatchString(commit.Message) {
+				filterPass = append(filterPass, commit)
+			}
 		}
+		filtered = filterPass
 	}
-	return filteredCommits
+
+	return filtered, nil
 }


Test patch:
diff --git a/internal/task/changelog/changelog_test.go b/internal/task/changelog/changelog_test.go
index 2dcd667..ac4ab28 100644
--- a/internal/task/changelog/changelog_test.go
+++ b/internal/task/changelog/changelog_test.go
@@ -590,17 +590,17 @@ func TestRun_IdentifiedSCM(t *testing.T) {
 	assert.Contains(t, buf.String(), expected)
 }
 
-func TestRun_WithIncludes(t *testing.T) {
+func TestRun_WithMultipleIncludes(t *testing.T) {
 	git.InitRepo(t)
 	git.Tag("1.0.0")
-	git.EmptyCommitsAndTag(t, "1.1.0", "ci: tweak", "fix(scope1): a fix", "feat(scope1): a feature", "fix(scope2): another fix")
+	git.EmptyCommitsAndTag(t, "1.1.0", "ci: tweak", "fix(scope1): a fix", "feat(scope1): a feature", "fix(common): another fix")
 
 	var buf bytes.Buffer
 	ctx := &context.Context{
 		Out: &buf,
 		Changelog: context.Changelog{
 			DiffOnly: true,
-			Include:  []string{`^.*\(scope1\)`},
+			Include:  []string{`^.*\(scope1\)`, `^.*\(common\)`},
 		},
 		CurrentVersion: semver.Version{
 			Raw: "1.0.0",
@@ -619,8 +619,8 @@ func TestRun_WithIncludes(t *testing.T) {
 	actual := buf.String()
 	assert.Contains(t, actual, "fix(scope1): a fix")
 	assert.Contains(t, actual, "feat(scope1): a feature")
+	assert.Contains(t, actual, "fix(common): another fix")
 	assert.NotContains(t, actual, "ci: tweak")
-	assert.NotContains(t, actual, "fix(scope2): another fix")
 }
 
 func TestRun_AllWithIncludes(t *testing.T) {
@@ -655,14 +655,14 @@ func TestRun_AllWithIncludes(t *testing.T) {
 func TestRun_CombinedIncludeAndExclude(t *testing.T) {
 	git.InitRepo(t)
 	git.Tag("1.0.0")
-	git.EmptyCommitsAndTag(t, "1.1.0", "ci: tweak", "fix(scope1): a fix", "feat(scope1): a feature", "fix(scope2): another fix")
+	git.EmptyCommitsAndTag(t, "1.1.0", "ci: tweak", "fix(scope1): a fix", "feat(scope1): a feature", "feat(scope2): another feature")
 
 	var buf bytes.Buffer
 	ctx := &context.Context{
 		Out: &buf,
 		Changelog: context.Changelog{
 			DiffOnly: true,
-			Include:  []string{`^.*\(scope1\)`},
+			Include:  []string{`^.*\(scope1\)`, `^.*\(scope2\)`},
 			Exclude:  []string{`^fix`},
 		},
 		CurrentVersion: semver.Version{
@@ -681,7 +681,7 @@ func TestRun_CombinedIncludeAndExclude(t *testing.T) {
 
 	actual := buf.String()
 	assert.Contains(t, actual, "feat(scope1): a feature")
+	assert.Contains(t, actual, "feat(scope2): another feature")
 	assert.NotContains(t, actual, "ci: tweak")
 	assert.NotContains(t, actual, "fix(scope1): a fix")
-	assert.NotContains(t, actual, "fix(scope2): another fix")
 }