From 9a1984c9ed69b2fc8aa042de4464b18efdf20a3e Mon Sep 17 00:00:00 2001 From: usmonster Date: Mon, 1 Mar 2021 10:09:56 +0100 Subject: [PATCH 1/3] [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 eea2de22..7bea7a2c 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 64a8cc0e..d571e10f 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) } } From 2e41ce101c085251f5d5925753fbe232b9c8b969 Mon Sep 17 00:00:00 2001 From: Usman Date: Mon, 27 Jan 2025 09:46:45 +0100 Subject: [PATCH 2/3] Update github_test.go --- service/hook/github/github_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/hook/github/github_test.go b/service/hook/github/github_test.go index 30d27a39..5ad646e8 100644 --- a/service/hook/github/github_test.go +++ b/service/hook/github/github_test.go @@ -1483,7 +1483,7 @@ func Test_transformPullRequestEvent(t *testing.T) { }, } hookTransformResult := transformPullRequestEvent(pullRequest) - require.EqualError(t, hookTransformResult.Error, "pull request edit doesn't require a build: only body/description was changed") + 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) From efdf09b78b1963ae905f94f1f62dd55b2d084787 Mon Sep 17 00:00:00 2001 From: Usman Date: Mon, 27 Jan 2025 09:47:31 +0100 Subject: [PATCH 3/3] Update github.go --- service/hook/github/github.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/service/hook/github/github.go b/service/hook/github/github.go index f7402ab9..f7199fd2 100644 --- a/service/hook/github/github.go +++ b/service/hook/github/github.go @@ -271,14 +271,14 @@ func transformPullRequestEvent(pullRequest PullRequestEventModel) hookCommon.Tra // only description changed if pullRequest.Changes.Title.From == "" { return hookCommon.TransformResultModel{ - Error: errors.New("pull request edit doesn't require a build: only body/description was changed"), + 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"), + Error: errors.New("pull Request edit doesn't require a build: only title was changed, and previous one was not skipped"), ShouldSkip: true, } }