From 66221cf33a10384839a84075fdc536e78e5cc23f Mon Sep 17 00:00:00 2001 From: Roberto Santalla Date: Thu, 2 Feb 2023 11:37:11 +0100 Subject: [PATCH 1/3] dependabot,renovate: handle revert commits dependabot,renovate: add integration test --- src/app/generate/generate_test.go | 66 +++++++++++++++++++ src/changelog/sources/dependabot/source.go | 19 ++++-- .../sources/dependabot/source_test.go | 5 ++ src/changelog/sources/renovate/source.go | 12 +++- src/changelog/sources/renovate/source_test.go | 15 ++++- 5 files changed, 110 insertions(+), 7 deletions(-) diff --git a/src/app/generate/generate_test.go b/src/app/generate/generate_test.go index ff9e2ee2..a11bb266 100644 --- a/src/app/generate/generate_test.go +++ b/src/app/generate/generate_test.go @@ -120,6 +120,72 @@ dependencies: commit: chore(deps): bump anotherdep from 0.0.1 to 0.0.2 (#69) `) + "\n", }, + { + name: "Markdown_Reverted_Dependency", + md: strings.TrimSpace(` +# Changelog +This is based on blah blah blah + +## Unreleased + +## v1.2.3 - 20YY-DD-MM + +### Enhancements +- This is in the past and should not be included + `) + "\n", + args: "", + author: "not a bot ", + commits: []string{ + "Revert \"chore(deps): bump thisdep from 1.7.0 to 1.10.1\"", + }, + expected: strings.TrimSpace(` +notes: "" +changes: [] +dependencies: + - name: thisdep + from: 1.10.1 + to: 1.7.0 + meta: + commit: Revert "chore(deps): bump thisdep from 1.7.0 to 1.10.1" + `) + "\n", + }, + { + // This test case does not represent the behavior we would like, but servers as an acknowledgement + // of the limitations of the current behavior. + name: "Markdown_Dependency_And_Revert", + md: strings.TrimSpace(` +# Changelog +This is based on blah blah blah + +## Unreleased + +## v1.2.3 - 20YY-DD-MM + +### Enhancements +- This is in the past and should not be included + `) + "\n", + args: "", + author: "dependabot ", + commits: []string{ + "chore(deps): bump thisdep from 1.7.0 to 1.10.1", + "Revert \"chore(deps): bump thisdep from 1.7.0 to 1.10.1\"", + }, + expected: strings.TrimSpace(` +notes: "" +changes: [] +dependencies: + - name: thisdep + from: 1.7.0 + to: 1.10.1 + meta: + commit: chore(deps): bump thisdep from 1.7.0 to 1.10.1 + - name: thisdep + from: 1.10.1 + to: 1.7.0 + meta: + commit: Revert "chore(deps): bump thisdep from 1.7.0 to 1.10.1" + `) + "\n", + }, { name: "Markdown_Dependabot", md: mdChangelog, diff --git a/src/changelog/sources/dependabot/source.go b/src/changelog/sources/dependabot/source.go index 10617ef0..452203c9 100644 --- a/src/changelog/sources/dependabot/source.go +++ b/src/changelog/sources/dependabot/source.go @@ -12,7 +12,7 @@ import ( log "github.com/sirupsen/logrus" ) -var commitRegex = regexp.MustCompile(`(?m)[Bb]ump (\S+)(?: from (\S+))?(?: to (\S+))?(?:.+\([#!](\d+)\)$)?`) +var commitRegex = regexp.MustCompile(`(?m)[Bb]ump (\S+)(?: from (\S+))?(?: to (\S+))?(?:.+\([#!](\d+)\)$)?(?:")?`) const dependabotAuthor = "dependabot" @@ -46,12 +46,19 @@ func (r Source) Changelog() (*changelog.Changelog, error) { for _, c := range gitCommits { commitLine := strings.Split(c.Message, "\n")[0] - if !strings.Contains(strings.ToLower(c.Author), dependabotAuthor) { - log.Debugf("skipping commit as it is not authored by dependabot\n> %q", commitLine) + isRevert := strings.HasPrefix(strings.ToLower(commitLine), "revert") + if !strings.Contains(strings.ToLower(c.Author), dependabotAuthor) && !isRevert { + log.Debugf("skipping commit as it is neither authored by dependabot nor a revert\n> %q", commitLine) continue } - capturingGroups := commitRegex.FindStringSubmatch(c.Message) + if isRevert { + // Revert commits usually surround the original commit line with double quotes. + // We could complicate the regex further, but stripping the quotes is allegedly easier. + commitLine = strings.Trim(commitLine, `"`) + } + + capturingGroups := commitRegex.FindStringSubmatch(commitLine) if len(capturingGroups) == 0 { log.Debugf("skipping commit %s as it does not match dependabot pattern and no information can be retrieved", c.Message) continue @@ -67,6 +74,10 @@ func (r Source) Changelog() (*changelog.Changelog, error) { log.Debugf("skipping dependency %q as it doesn't conform to semver %v", dependencyName, dependencyTo) } + if isRevert { + dependencyTo, dependencyFrom = dependencyFrom, dependencyTo + } + dependencies = append(dependencies, changelog.Dependency{ Name: dependencyName, From: dependencyFrom, diff --git a/src/changelog/sources/dependabot/source_test.go b/src/changelog/sources/dependabot/source_test.go index 082e21dd..5d2c6155 100644 --- a/src/changelog/sources/dependabot/source_test.go +++ b/src/changelog/sources/dependabot/source_test.go @@ -263,6 +263,11 @@ Bumps [actions/github-script](https://github.com/actions/github-script) from 4.0 errExpected: errRandomError, expected: nil, }, + { + name: "Revert_Matching_Commit_Single_Root_Minor", + commit: git.Commit{Message: "Revert \"chore(deps): bump my-dep from 1.2.3 to 2.0.0\"", Author: "Not-a-bot"}, + expected: []changelog.Dependency{{Name: "my-dep", From: semver.MustParse("2.0.0"), To: semver.MustParse("1.2.3")}}, + }, } { tc := tc if tc.name == "" { diff --git a/src/changelog/sources/renovate/source.go b/src/changelog/sources/renovate/source.go index 54915c8e..ad5e2a7c 100644 --- a/src/changelog/sources/renovate/source.go +++ b/src/changelog/sources/renovate/source.go @@ -47,8 +47,9 @@ func (r Source) Changelog() (*changelog.Changelog, error) { var commitDependencies []changelog.Dependency commitLine := strings.Split(c.Message, "\n")[0] - if !strings.Contains(strings.ToLower(c.Author), renovateAuthor) { - log.Debugf("skipping commit as it is not authored by renovate\n> %q", commitLine) + isRevert := strings.HasPrefix(strings.ToLower(commitLine), "revert") + if !strings.Contains(strings.ToLower(c.Author), renovateAuthor) && !isRevert { + log.Debugf("skipping commit as it is neither authored by renovate nor a revert\n> %q", commitLine) continue } @@ -66,6 +67,13 @@ func (r Source) Changelog() (*changelog.Changelog, error) { commitDependencies = append(commitDependencies, r.titleDependencies(commitLine)...) } + if isRevert { + //If this is a revert commit, swap to/from for all commitDependencies. + for i := range commitDependencies { + commitDependencies[i].From, commitDependencies[i].To = commitDependencies[i].To, commitDependencies[i].From + } + } + // Add commit hash and copy to dependencies for _, dep := range commitDependencies { dep.Meta.Commit = c.Hash diff --git a/src/changelog/sources/renovate/source_test.go b/src/changelog/sources/renovate/source_test.go index 96fed341..5e0c0e48 100644 --- a/src/changelog/sources/renovate/source_test.go +++ b/src/changelog/sources/renovate/source_test.go @@ -41,7 +41,7 @@ func (c *commitsGetterMock) Commits(_ string) ([]git.Commit, error) { return commits, nil } -//nolint:funlen +//nolint:funlen,maintidx func TestSource_Source(t *testing.T) { t.Parallel() for _, tc := range []struct { @@ -196,6 +196,19 @@ func TestSource_Source(t *testing.T) { }, }, }, + { + name: "Matches_Reverts", + defaultAuthor: "not-a-bot", + commitMessages: []git.Commit{ + {Message: "Revert \"update noprefix to v1.2.3\""}, + }, + expectedDependencies: []changelog.Dependency{ + { + Name: "noprefix", + From: semver.MustParse("v1.2.3"), + }, + }, + }, { name: "Error_No_Releases", defaultAuthor: "renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>", From 814860cc78c595ffbc64fa2b5e2f5afdcd21b6ba Mon Sep 17 00:00:00 2001 From: Roberto Santalla Date: Thu, 2 Feb 2023 11:43:17 +0100 Subject: [PATCH 2/3] dependabot,renovate: exclude cyclo linters from Changelog functions --- src/changelog/sources/dependabot/source.go | 1 + src/changelog/sources/renovate/source.go | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/changelog/sources/dependabot/source.go b/src/changelog/sources/dependabot/source.go index 452203c9..587b7740 100644 --- a/src/changelog/sources/dependabot/source.go +++ b/src/changelog/sources/dependabot/source.go @@ -28,6 +28,7 @@ func NewSource(tagsVersionGetter git.TagsVersionGetter, commitsGetter git.Commit } } +//nolint:gocyclo,cyclop func (r Source) Changelog() (*changelog.Changelog, error) { lastHash, err := r.tagsVersionGetter.LastVersionHash() if err != nil { diff --git a/src/changelog/sources/renovate/source.go b/src/changelog/sources/renovate/source.go index ad5e2a7c..5037acfe 100644 --- a/src/changelog/sources/renovate/source.go +++ b/src/changelog/sources/renovate/source.go @@ -26,6 +26,7 @@ func NewSource(tagsVersionGetter git.TagsVersionGetter, commitsGetter git.Commit } } +//nolint:gocyclo,cyclop func (r Source) Changelog() (*changelog.Changelog, error) { lastHash, err := r.tagsVersionGetter.LastVersionHash() if err != nil { @@ -68,7 +69,7 @@ func (r Source) Changelog() (*changelog.Changelog, error) { } if isRevert { - //If this is a revert commit, swap to/from for all commitDependencies. + // If this is a revert commit, swap to/from for all commitDependencies. for i := range commitDependencies { commitDependencies[i].From, commitDependencies[i].To = commitDependencies[i].To, commitDependencies[i].From } From 42df074c6f8a76d4e8b7b30cddf25a0f7bdffeed Mon Sep 17 00:00:00 2001 From: Roberto Santalla Date: Thu, 2 Feb 2023 11:56:52 +0100 Subject: [PATCH 3/3] app/generate: add nolint:maintidx to test --- src/app/generate/generate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/generate/generate_test.go b/src/app/generate/generate_test.go index a11bb266..5dbb623a 100644 --- a/src/app/generate/generate_test.go +++ b/src/app/generate/generate_test.go @@ -34,7 +34,7 @@ This is a release note - This is in the past and should not be included ` -//nolint:funlen,paralleltest +//nolint:funlen,maintidx,paralleltest func TestGenerate(t *testing.T) { for _, tc := range []struct { name string