Skip to content

Commit

Permalink
[github] Don't consider PR body in CI skip logic
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
usmonster committed Jun 21, 2022
1 parent 454b8be commit 46ec927
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 27 deletions.
14 changes: 11 additions & 3 deletions service/hook/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
Expand Down
31 changes: 7 additions & 24 deletions service/hook/github/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)
}
}
Expand Down

0 comments on commit 46ec927

Please sign in to comment.