Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

"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 this issue Mar 23, 2017 · 2 comments · May be fixed by #757
Open

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

LinkMJB opened this issue Mar 23, 2017 · 2 comments · May be fixed by #757

Comments

@LinkMJB
Copy link

LinkMJB commented Mar 23, 2017 •
edited
Loading

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
@LinkMJB LinkMJB changed the title "Only use trigger phrase for build triggering" does not work "Only use trigger phrase for build triggering" does not work when common with "test phrase" or "add to whitelist" phrase Mar 29, 2017
@LinkMJB LinkMJB changed the title "Only use trigger phrase for build triggering" does not work when common with "test phrase" or "add to whitelist" phrase "Only use trigger phrase for build triggering" does not work when "trigger phrase" is common with "test phrase" or "add to whitelist" phrase Mar 29, 2017
@LinkMJB LinkMJB mentioned this issue Mar 31, 2017
Open
@Ippo343
Copy link
Contributor

Ippo343 commented Feb 21, 2019

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

@cben
Copy link

cben commented Jul 15, 2019

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

Looking at

ghprb-plugin/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java

Lines 610 to 630 in 57f12d0

} else if (helper.isRetestPhrase(body)) { // test this please
LOGGER.log(Level.FINEST, "Retest phrase");
if (helper.isAdmin(sender)) {
LOGGER.log(Level.FINEST, "Admin {0} gave retest phrase", sender);
shouldRun = true;
} else if (accepted && helper.isWhitelisted(sender)) {
LOGGER.log(Level.FINEST, "Retest accepted and user {0} is whitelisted", sender);
shouldRun = true;
}
} else if (helper.isTriggerPhrase(body)) { // trigger phrase
LOGGER.log(Level.FINEST, "Trigger phrase");
if (helper.isAdmin(sender)) {
LOGGER.log(Level.FINEST, "Admin {0} ran trigger phrase", sender);
shouldRun = true;
triggered = true;
} else if (accepted && helper.isWhitelisted(sender)) {
LOGGER.log(Level.FINEST, "Trigger accepted and user {0} is whitelisted", sender);
shouldRun = true;
triggered = true;
}
}

they seem equivalent but only triggerPhrase sets triggered = true.
Later in tryBuild():

ghprb-plugin/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java

Lines 521 to 524 in 57f12d0

if (helper.ifOnlyTriggerPhrase() && !triggered) {
LOGGER.log(Level.FINEST, "Trigger only phrase but we are not triggered");
shouldRun = false;
}

=> 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. ❓

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.

mabrowning added a commit to mabrowning/ghprb-plugin that referenced this issue Oct 17, 2019
@mabrowning
Fix jenkinsci#499 by prioritizing trigger phrase over retest phrase
70b1c65
@mabrowning mabrowning linked a pull request Oct 17, 2019 that will close this issue
Open
mabrowning added a commit to mabrowning/ghprb-plugin that referenced this issue Oct 18, 2019
@mabrowning
Fix jenkinsci#499 by prioritizing trigger phrase over retest phrase
6ae51f9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #499 by prioritizing trigger phrase over retest phrase mabrowning/ghprb-plugin
3 participants
@cben @Ippo343 @LinkMJB