From 46ec927f3a2e8710a92fbbaa7b92c7d31a4f010b Mon Sep 17 00:00:00 2001 From: usmonster Date: Mon, 1 Mar 2021 10:09:56 +0100 Subject: [PATCH] [github] Don't consider PR body in CI skip logic GitHub pull request edits were not triggering CI when the PR body/description contained the `[ ci skip ]`/`[ skip ci ]` pattern. This was an issue, for example, in Dependabot-generated PRs that include commit messages from upstream dependencies that do include the pattern. Furthermore, no other CI service or vendor considers the PR body for the "ci skip" mechanism. This corrects the behavior so that only commit messages or PR titles are considered when a PR is edited. --- service/hook/github/github.go | 14 +++++++++++--- service/hook/github/github_test.go | 31 +++++++----------------------- 2 files changed, 18 insertions(+), 27 deletions(-) diff --git a/service/hook/github/github.go b/service/hook/github/github.go index 6c6ead3d..28392578 100644 --- a/service/hook/github/github.go +++ b/service/hook/github/github.go @@ -196,11 +196,19 @@ func transformPullRequestEvent(pullRequest PullRequestEventModel) hookCommon.Tra } } if pullRequest.Action == "edited" { - // skip it if only title / description changed, and the previous pattern did not include a [skip ci] pattern + // skip it if only title / description changed, and the current title did not remove a [skip ci] pattern if pullRequest.Changes.Base == nil { - if !hookCommon.IsSkipBuildByCommitMessage(pullRequest.Changes.Title.From) && !hookCommon.IsSkipBuildByCommitMessage(pullRequest.Changes.Body.From) { + // only description changed + if pullRequest.Changes.Title.From == "" { return hookCommon.TransformResultModel{ - Error: errors.New("Pull Request edit doesn't require a build: only title and/or description was changed, and previous one was not skipped"), + Error: errors.New("Pull Request edit doesn't require a build: only body/description was changed"), + ShouldSkip: true, + } + } + // title changed without removing any [skip ci] pattern + if !hookCommon.IsSkipBuildByCommitMessage(pullRequest.Changes.Title.From) { + return hookCommon.TransformResultModel{ + Error: errors.New("Pull Request edit doesn't require a build: only title was changed, and previous one was not skipped"), ShouldSkip: true, } } diff --git a/service/hook/github/github_test.go b/service/hook/github/github_test.go index 19a3d9c6..1820ed0f 100644 --- a/service/hook/github/github_test.go +++ b/service/hook/github/github_test.go @@ -828,7 +828,7 @@ func Test_transformPullRequestEvent(t *testing.T) { }, } hookTransformResult := transformPullRequestEvent(pullRequest) - require.EqualError(t, hookTransformResult.Error, "Pull Request edit doesn't require a build: only title and/or description was changed, and previous one was not skipped") + require.EqualError(t, hookTransformResult.Error, "Pull Request edit doesn't require a build: only title was changed, and previous one was not skipped") require.True(t, hookTransformResult.ShouldSkip) require.Equal(t, []bitriseapi.TriggerAPIParamsModel(nil), hookTransformResult.TriggerAPIParams) require.Equal(t, false, hookTransformResult.DontWaitForTriggerResponse) @@ -897,7 +897,7 @@ func Test_transformPullRequestEvent(t *testing.T) { require.Equal(t, false, hookTransformResult.DontWaitForTriggerResponse) } - t.Log("Pull Request - edited - only body/description change - no skip ci in previous - no build") + t.Log("Pull Request - edited - only body/description change - no build") { pullRequest := PullRequestEventModel{ Action: "edited", @@ -937,13 +937,13 @@ func Test_transformPullRequestEvent(t *testing.T) { }, } hookTransformResult := transformPullRequestEvent(pullRequest) - require.EqualError(t, hookTransformResult.Error, "Pull Request edit doesn't require a build: only title and/or description was changed, and previous one was not skipped") + require.EqualError(t, hookTransformResult.Error, "Pull Request edit doesn't require a build: only body/description was changed") require.True(t, hookTransformResult.ShouldSkip) require.Equal(t, []bitriseapi.TriggerAPIParamsModel(nil), hookTransformResult.TriggerAPIParams) require.Equal(t, false, hookTransformResult.DontWaitForTriggerResponse) } - t.Log("Pull Request - edited - only body/description change - BUT the previous title included a skip CI pattern - should build") + t.Log("Pull Request - edited - only body/description change - the previous body included a skip CI pattern - still shouldn't build") { pullRequest := PullRequestEventModel{ Action: "edited", @@ -983,26 +983,9 @@ func Test_transformPullRequestEvent(t *testing.T) { }, } hookTransformResult := transformPullRequestEvent(pullRequest) - require.NoError(t, hookTransformResult.Error) - require.False(t, hookTransformResult.ShouldSkip) - require.Equal(t, []bitriseapi.TriggerAPIParamsModel{ - { - BuildParams: bitriseapi.BuildParamsModel{ - CommitHash: "83b86e5f286f546dc5a4a58db66ceef44460c85e", - CommitMessage: "PR test\n\nPR text body", - Branch: "feature/github-pr", - BranchDest: "develop", - PullRequestID: pointers.NewIntPtr(12), - PullRequestRepositoryURL: "https://github.com/bitrise-io/bitrise-webhooks.git", - BaseRepositoryURL: "https://github.com/bitrise-io/bitrise-webhooks.git", - HeadRepositoryURL: "https://github.com/bitrise-io/bitrise-webhooks.git", - PullRequestMergeBranch: "pull/12/merge", - PullRequestHeadBranch: "pull/12/head", - Environments: make([]bitriseapi.EnvironmentItem, 0), - }, - TriggeredBy: "webhook-github/test_user", - }, - }, hookTransformResult.TriggerAPIParams) + require.EqualError(t, hookTransformResult.Error, "Pull Request edit doesn't require a build: only body/description was changed") + require.True(t, hookTransformResult.ShouldSkip) + require.Equal(t, []bitriseapi.TriggerAPIParamsModel(nil), hookTransformResult.TriggerAPIParams) require.Equal(t, false, hookTransformResult.DontWaitForTriggerResponse) } }