search

jenkinsci / ghprb-plugin

github pull requests builder plugin for Jenkins
https://plugins.jenkins.io/ghprb/
MIT License
501 stars 608 forks source link

"Only use trigger phrase for build triggering" does not work when "trigger phrase" is common with "test phrase" or "add to whitelist" phrase #499

Open LinkMJB opened 7 years ago

LinkMJB commented 7 years ago

When enabling "Only use trigger phrase for build triggering" PR builds no longer work when the trigger phrase is provided. The trigger phrase does in fact work when "Only use trigger phrase for build triggering" is unchecked/disabled.

The trigger phrase works fine when <onlyTriggerPhrase>true</onlyTriggerPhrase> is set to false. As soon as the trigger phrase is required, the PR's are not built.

Here's a logging example of the behavior with the option enabled, and without the option enabled:

=== With “Only use trigger phrase for build triggering" enabled ===

Mar 02, 2017 6:18:01 PM INFO org.jenkinsci.plugins.ghprb.GhprbPullRequest updatePR Pull request #1,708 was updated/initialized on codice/ddf at 3/2/17 6:17 PM by null (PR update) Mar 02, 2017 6:18:01 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Checking for comments after: 3/2/17 5:17 PM Mar 02, 2017 6:18:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/20/17 11:06 AM Mar 02, 2017 6:18:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/21/17 10:59 AM Mar 02, 2017 6:18:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/21/17 4:40 PM Mar 02, 2017 6:18:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/23/17 2:58 PM Mar 02, 2017 6:18:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/27/17 6:32 PM Mar 02, 2017 6:18:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/28/17 11:47 AM Mar 02, 2017 6:18:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 3/2/17 12:30 PM Mar 02, 2017 6:18:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 3/2/17 6:17 PM Mar 02, 2017 6:18:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made after last update time, ok to test Mar 02, 2017 6:18:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComment [Matthew Bates] Added comment: ok to test Mar 02, 2017 6:18:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComment Admin GHUser@1e9a64d[login=LinkMJB,location=Phoenix, AZ,blog=https://github.com/connexta,email=,name=Matthew Bates,company=Connexta,followers=0,following=0,url=https://api.github.com/users/LinkMJB,id=8824103] gave OK to test

=== Without “Only use trigger phrase for build triggering" enabled ===

Mar 02, 2017 6:24:02 PM INFO org.jenkinsci.plugins.ghprb.GhprbPullRequest updatePR Pull request #1,708 was updated/initialized on codice/ddf at 3/2/17 6:17 PM by null (PR update) Mar 02, 2017 6:24:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Checking for comments after: 3/2/17 5:17 PM Mar 02, 2017 6:24:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/20/17 11:06 AM Mar 02, 2017 6:24:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/21/17 10:59 AM Mar 02, 2017 6:24:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/21/17 4:40 PM Mar 02, 2017 6:24:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/23/17 2:58 PM Mar 02, 2017 6:24:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/27/17 6:32 PM Mar 02, 2017 6:24:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/28/17 11:47 AM Mar 02, 2017 6:24:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 3/2/17 12:30 PM Mar 02, 2017 6:24:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 3/2/17 6:17 PM Mar 02, 2017 6:24:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made after last update time, ok to test Mar 02, 2017 6:24:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComment [Matthew Bates] Added comment: ok to test Mar 02, 2017 6:24:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComment Admin GHUser@1450890[login=LinkMJB,location=Phoenix, AZ,blog=https://github.com/connexta,email=,name=Matthew Bates,company=Connexta,followers=0,following=0,url=https://api.github.com/users/LinkMJB,id=8824103] gave OK to test Mar 02, 2017 6:24:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest tryBuild Running the build Mar 02, 2017 6:24:02 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest tryBuild PR is not null, checking if mergable Mar 02, 2017 6:24:03 PM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest tryBuild Running build...

Some more details -- when trying to use the retest phrase it claims that the target branch is either not listed in the whitelist, even though it is -- or that it is not triggered. I haven't been able to figure out the context of the logs in regard to which job/branch the PR poll is performed for/by.

Is this plugin only supported to be used as a single Jenkins project for PR builds against an entire github repo? We have separate jobs created for each branch, in our configuration.

From the master branch PR builder job's config.xml:

<whiteListTargetBranches>
<org.jenkinsci.plugins.ghprb.GhprbBranch>
<branch>master</branch>
</org.jenkinsci.plugins.ghprb.GhprbBranch>
</whiteListTargetBranches>

=== From the org.jenkinsci.plugins.ghprb.GhprbPullRequest logger: ===

Pull request #1,708 was updated/initialized on codice/ddf at 3/3/17 9:07 AM by null (PR update) Mar 03, 2017 9:08:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Checking for comments after: 3/3/17 8:38 AM Mar 03, 2017 9:08:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/20/17 11:06 AM Mar 03, 2017 9:08:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/21/17 10:59 AM Mar 03, 2017 9:08:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/21/17 4:40 PM Mar 03, 2017 9:08:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/23/17 2:58 PM Mar 03, 2017 9:08:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/27/17 6:32 PM Mar 03, 2017 9:08:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/28/17 11:47 AM Mar 03, 2017 9:08:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 3/2/17 12:30 PM Mar 03, 2017 9:08:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 3/2/17 6:17 PM Mar 03, 2017 9:08:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 3/3/17 12:41 AM Mar 03, 2017 9:08:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 3/3/17 9:07 AM Mar 03, 2017 9:08:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made after last update time, test this please Mar 03, 2017 9:08:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComment [Matthew Bates] Added comment: test this please Mar 03, 2017 9:08:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComment Retest phrase Mar 03, 2017 9:08:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComment Admin GHUser@1e9a64d[login=LinkMJB,location=Phoenix, AZ,blog=https://github.com/connexta,email=,name=Matthew Bates,company=Connexta,followers=0,following=0,url=https://api.github.com/users/LinkMJB,id=8824103] gave retest phrase Mar 03, 2017 9:08:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest isAllowedTargetBranch PR #1,708 target branch: master isn't in our whitelist of target branches: org.jenkinsci.plugins.ghprb.GhprbBranch@7fa99c0 Mar 03, 2017 9:08:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest tryBuild Branch is not whitelisted or is blacklisted, skipping the build

=== Another run ===

Pull request #1,708 was updated/initialized on codice/ddf at 3/3/17 9:07 AM by null (PR update) Mar 03, 2017 9:14:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Checking for comments after: 3/3/17 8:38 AM Mar 03, 2017 9:14:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/20/17 11:06 AM Mar 03, 2017 9:14:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/21/17 10:59 AM Mar 03, 2017 9:14:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/21/17 4:40 PM Mar 03, 2017 9:14:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/23/17 2:58 PM Mar 03, 2017 9:14:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/27/17 6:32 PM Mar 03, 2017 9:14:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 2/28/17 11:47 AM Mar 03, 2017 9:14:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 3/2/17 12:30 PM Mar 03, 2017 9:14:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 3/2/17 6:17 PM Mar 03, 2017 9:14:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 3/3/17 12:41 AM Mar 03, 2017 9:14:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made at: 3/3/17 9:07 AM Mar 03, 2017 9:14:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComments Comment was made after last update time, test this please Mar 03, 2017 9:14:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComment [Matthew Bates] Added comment: test this please Mar 03, 2017 9:14:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComment Retest phrase Mar 03, 2017 9:14:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest checkComment Admin GHUser@1450890[login=LinkMJB,location=Phoenix, AZ,blog=https://github.com/connexta,email=,name=Matthew Bates,company=Connexta,followers=0,following=0,url=https://api.github.com/users/LinkMJB,id=8824103] gave retest phrase Mar 03, 2017 9:14:02 AM FINEST org.jenkinsci.plugins.ghprb.GhprbPullRequest tryBuild Trigger only phrase but we are not triggered

Ippo343 commented 5 years ago

For the record, unfortunately this issue seems to be still present and we are hitting it too.

cben commented 5 years ago

I'm just here in hope to understand the intended difference between triggerPhrase and retestPhrase, and what the hell onlyTriggerPhrase means.

Looking at https://github.com/jenkinsci/ghprb-plugin/blob/57f12d0a4e4d6846833ca560c4dd3c5320ab45de/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java#L610-L630 they seem equivalent but only triggerPhrase sets triggered = true. Later in tryBuild(): https://github.com/jenkinsci/ghprb-plugin/blob/57f12d0a4e4d6846833ca560c4dd3c5320ab45de/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java#L521-L524

=> So my impression is that onlyTriggerPhrase means no other phrase can run a build. Whitelist and ok to test phrases can have side effect of allowing future builds, but can't trigger it themselves. :question:

Now, for this issue: README says:

... isn't whitelisted ... One of the admins can comment ... test this please for one time test run ...

and later:

A new build can also be started with a comment which contains, retest this please

so I was thinking these are 2 distinct phrases, trigger vs retest? But the actual retestPhrase default is .*test\W+this\W+please.* so it also matches test this please! And afaict, triggerPhrase is unset by default?

So yes, if same string matches both retest and trigger phrase, if (helper.isRetestPhrase(body)) will win and it will be interpreted as retest! => triggered == false => no effect if onlyTriggerPhrase enabled. This could be argued a mild bug. I'd expect it be the other way around (it was a valid trigger phrase, thus should take effect)?

But it sounds best to configure regexps such that only one can match a given comment.