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

JENKINS-65075 Update PluginPatchsetCreatedEvent shouldTriggerOn #428

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+58 −13
Open

JENKINS-65075 Update PluginPatchsetCreatedEvent shouldTriggerOn #428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions ...icsson/hudson/plugins/gerrit/trigger/hudsontrigger/events/PluginPatchsetCreatedEvent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,14 @@ public boolean shouldTriggerOn(GerritTriggeredEvent event) {
if (excludeDrafts && ((PatchsetCreated)event).getPatchSet().isDraft()) {
return false;
}
if (StringUtils.isNotEmpty(commitMessageContainsRegEx)) {
if (commitMessagePattern == null) {
commitMessagePattern = Pattern.compile(
this.commitMessageContainsRegEx, Pattern.DOTALL | Pattern.MULTILINE);
}
String commitMessage = ((PatchsetCreated)event).getChange().getCommitMessage();
return commitMessagePattern.matcher(commitMessage).find();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning this early will will nullify all the other exclude checks below

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. I didn't think of that.

In my opinion, the ManualPatchsetCreated event should behave just like a PatchsetCreated event. That is, the two triggers should be identical. it's just that one allow you to retrigger jobs without having to actually create a new Gerrit patch set.

If that is the case, can't we just remove the event instanced ManualPatchsetCreated block?

}
if (event instanceof ManualPatchsetCreated) {
// always trigger build when the build is triggered manually
return true;
Expand All @@ -243,14 +251,6 @@ public boolean shouldTriggerOn(GerritTriggeredEvent event) {
if (excludeWipState && ((PatchsetCreated)event).getChange().isWip()) {
return false;
}
if (StringUtils.isNotEmpty(commitMessageContainsRegEx)) {
if (commitMessagePattern == null) {
commitMessagePattern = Pattern.compile(
this.commitMessageContainsRegEx, Pattern.DOTALL | Pattern.MULTILINE);
}
String commitMessage = ((PatchsetCreated)event).getChange().getCommitMessage();
return commitMessagePattern.matcher(commitMessage).find();
}
return true;
}

Expand Down
55 changes: 50 additions & 5 deletions ...son/hudson/plugins/gerrit/trigger/hudsontrigger/event/PluginPatchsetCreatedEventTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
package com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.event;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import org.junit.Test;

import com.sonyericsson.hudson.plugins.gerrit.trigger.events.ManualPatchsetCreated;
import com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.events.PluginPatchsetCreatedEvent;
import com.sonymobile.tools.gerrit.gerritevents.dto.GerritChangeKind;
import com.sonymobile.tools.gerrit.gerritevents.dto.attr.Change;
import com.sonymobile.tools.gerrit.gerritevents.dto.attr.PatchSet;
import com.sonymobile.tools.gerrit.gerritevents.dto.events.PatchsetCreated;
import org.junit.Test;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

/**
* Tests for {@link PluginPatchsetCreatedEvent}.
Expand Down Expand Up @@ -131,4 +131,49 @@ public void commitMessageRegExCheck() {
pluginPatchsetCreatedEvent.setCommitMessageContainsRegEx("MY_THING");
assertFalse(pluginPatchsetCreatedEvent.shouldTriggerOn(patchsetCreated));
}
/**
* Test that it should, or should not, fire if the commit message matches a regular expression.
*/
@Test
public void commitMessageRegExCheckManualPatchSetCreated() {
PluginPatchsetCreatedEvent pluginPatchsetCreatedEvent =
new PluginPatchsetCreatedEvent();
ManualPatchsetCreated patchsetCreated = new ManualPatchsetCreated();
patchsetCreated.setPatchset(new PatchSet());
StringBuilder commitMessage = new StringBuilder();
commitMessage.append("This is a summary\n");
commitMessage.append("\n");
commitMessage.append("This is my description.\n");
commitMessage.append("\n");
commitMessage.append("Issue: JENKINS-64091\n");
Change change = new Change();
change.setCommitMessage(commitMessage.toString());
patchsetCreated.setChange(change);

// Commit Message regular expression set to null
pluginPatchsetCreatedEvent.setCommitMessageContainsRegEx(null);
assertTrue(pluginPatchsetCreatedEvent.shouldTriggerOn(patchsetCreated));

// Commit message Regular expression is an empty string
pluginPatchsetCreatedEvent.setCommitMessageContainsRegEx("");
assertTrue(pluginPatchsetCreatedEvent.shouldTriggerOn(patchsetCreated));

// Commit message Regular expression matches
pluginPatchsetCreatedEvent.setCommitMessageContainsRegEx("JENKINS");
assertTrue(pluginPatchsetCreatedEvent.shouldTriggerOn(patchsetCreated));

// Commit message Regular expression matches
pluginPatchsetCreatedEvent.setCommitMessageContainsRegEx("Issue:.*JENKINS.*");
assertTrue(pluginPatchsetCreatedEvent.shouldTriggerOn(patchsetCreated));

// Commit message Regular expression does not match
pluginPatchsetCreatedEvent.setCommitMessageContainsRegEx("Issue:.*MY_THING.*");
boolean result = pluginPatchsetCreatedEvent.shouldTriggerOn(patchsetCreated);
assertFalse(result);

// Commit message Regular expression does not match
pluginPatchsetCreatedEvent.setCommitMessageContainsRegEx("MY_THING");
assertFalse(pluginPatchsetCreatedEvent.shouldTriggerOn(patchsetCreated));
}

}