Skip to content

Commit 16922b9

Browse files
KostyaShaKostyaSha
authored
Check all comments nevertheless localPR was null. Exit on first cause. (#214)
* Check all comments nevertheless localPR was null. Exit on first cause. * version * fixup * travis_wait * wait * update wrapper * no wait
1 parent 1bb057a commit 16922b9

8 files changed

Lines changed: 111 additions & 64 deletions

File tree

‎.mvn/wrapper/maven-wrapper.jar‎

100644100755
-1.86 KB
Binary file not shown.

‎.mvn/wrapper/maven-wrapper.properties‎

100644100755
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
distributionUrl=https://repo1.maven.org/maven2/org/apache/maven/apache-maven/3.3.9/apache-maven-3.3.9-bin.zip
1+
distributionUrl=https://repo1.maven.org/maven2/org/apache/maven/apache-maven/3.5.0/apache-maven-3.5.0-bin.zip

‎github-pullrequest-plugin/src/main/java/org/jenkinsci/plugins/github/pullrequest/GitHubPRPullRequest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public GitHubPRPullRequest(GHPullRequest pr) throws IOException {
7171

7272
try {
7373
Date maxDate = new Date(0);
74-
List<GHIssueComment> comments = execute(() -> pr.getComments());
74+
List<GHIssueComment> comments = execute(pr::getComments);
7575
for (GHIssueComment comment : comments) {
7676
if (comment.getCreatedAt().compareTo(maxDate) > 0) {
7777
maxDate = comment.getCreatedAt();

‎github-pullrequest-plugin/src/main/java/org/jenkinsci/plugins/github/pullrequest/events/impl/GitHubPRCommentEvent.java‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.regex.Pattern;
2323

2424
import static org.jenkinsci.plugins.github.pullrequest.utils.ObjectsUtil.isNull;
25+
import static org.jenkinsci.plugins.github.pullrequest.utils.ObjectsUtil.nonNull;
2526

2627
/**
2728
* Trigger PR based on comment pattern.
@@ -30,7 +31,7 @@
3031
*/
3132
public class GitHubPRCommentEvent extends GitHubPREvent {
3233
private static final String DISPLAY_NAME = "Comment matched to pattern";
33-
private static final Logger LOGGER = LoggerFactory.getLogger(GitHubPRCommentEvent.class);
34+
private static final Logger LOG = LoggerFactory.getLogger(GitHubPRCommentEvent.class);
3435

3536
private String comment = "";
3637

@@ -46,25 +47,33 @@ public GitHubPRCommentEvent(String comment) {
4647
@Override
4748
public GitHubPRCause check(@Nonnull GitHubPRTrigger gitHubPRTrigger, GHPullRequest remotePR,
4849
@CheckForNull GitHubPRPullRequest localPR, TaskListener listener) {
49-
if (isNull(localPR)) {
50-
return null; // nothing to compare
51-
}
52-
final PrintStream logger = listener.getLogger();
50+
final PrintStream llog = listener.getLogger();
5351

5452
GitHubPRCause cause = null;
5553
try {
5654
for (GHIssueComment issueComment : remotePR.getComments()) {
57-
if (isNull(localPR.getLastCommentCreatedAt())
55+
if (isNull(localPR) // test all comments for trigger word even if we never saw PR before
56+
|| isNull(localPR.getLastCommentCreatedAt()) // PR was created but had no comments
57+
// don't check comments that we saw before
5858
|| localPR.getLastCommentCreatedAt().compareTo(issueComment.getCreatedAt()) < 0) {
59-
logger.printf("%s: state has changed (new comment found - '%s')%n",
59+
llog.printf("%s: state has changed (new comment found - '%s')%n",
6060
DISPLAY_NAME, issueComment.getBody());
6161
cause = checkComment(issueComment, gitHubPRTrigger.getUserRestriction(), remotePR, listener);
62+
if (nonNull(cause)) {
63+
break;
64+
}
6265
}
6366
}
6467
} catch (Exception e) {
65-
LOGGER.warn("Couldn't obtain comments: {}", e);
68+
LOG.warn("Couldn't obtain comments: {}", e);
6669
listener.error("Couldn't obtain comments", e);
6770
}
71+
72+
if (isNull(cause)) {
73+
LOG.debug("No matching comments found for {}", remotePR.getNumber());
74+
llog.println("No matching comments found for " + remotePR.getNumber());
75+
}
76+
6877
return cause;
6978
}
7079

@@ -80,7 +89,7 @@ private GitHubPRCause checkComment(GHIssueComment issueComment,
8089
final Matcher matcher = Pattern.compile(comment).matcher(body);
8190
if (matcher.matches()) {
8291
listener.getLogger().println(DISPLAY_NAME + ": matching comment " + body);
83-
LOGGER.trace("Event matches comment '{}'", body);
92+
LOG.trace("Event matches comment '{}'", body);
8493
cause = new GitHubPRCause(remotePR, "Comment matches to criteria.", false);
8594
cause.withCommentBody(body);
8695
if (matcher.groupCount() > 0) {
@@ -89,7 +98,7 @@ private GitHubPRCause checkComment(GHIssueComment issueComment,
8998
}
9099
}
91100
} catch (IOException ex) {
92-
LOGGER.error("Couldn't check comment #{}, skipping it.", issueComment.getId(), ex);
101+
LOG.error("Couldn't check comment #{}, skipping it.", issueComment.getId(), ex);
93102
}
94103
return cause;
95104
}

‎github-pullrequest-plugin/src/test/java/org/jenkinsci/plugins/github/pullrequest/events/impl/GitHubPRCommentEventTest.java‎

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,13 @@
2727
import static java.util.Collections.emptyList;
2828
import static java.util.Collections.emptySet;
2929
import static org.hamcrest.core.Is.is;
30+
import static org.hamcrest.core.IsNull.notNullValue;
3031
import static org.junit.Assert.assertNotNull;
3132
import static org.junit.Assert.assertNull;
3233
import static org.junit.Assert.assertThat;
3334
import static org.mockito.Matchers.anyInt;
35+
import static org.mockito.Matchers.isNotNull;
36+
import static org.mockito.Matchers.notNull;
3437
import static org.mockito.Mockito.mock;
3538
import static org.mockito.Mockito.when;
3639

@@ -63,8 +66,11 @@ public class GitHubPRCommentEventTest {
6366

6467
@Mock
6568
private GitHubPRTrigger trigger;
69+
6670
@Mock
6771
private GHIssueComment comment;
72+
@Mock
73+
private GHIssueComment comment2;
6874

6975
@Test
7076
public void testNullLocalComment() throws IOException {
@@ -77,7 +83,7 @@ public void testNullLocalComment() throws IOException {
7783
ghIssueComments.add(comment);
7884
when(remotePr.getComments()).thenReturn(ghIssueComments);
7985

80-
GitHubPRCause cause = new GitHubPRCommentEvent("Comment").check(trigger, remotePr, localPR, listener);
86+
GitHubPRCause cause = new GitHubPRCommentEvent("Comment").check(trigger, remotePr, null, listener);
8187

8288
assertNull(cause);
8389
}
@@ -104,20 +110,66 @@ public void testNullLocalCommentRemoteMatch() throws IOException {
104110
assertNotNull(cause);
105111
}
106112

113+
@Test
114+
public void firstCommentMatchSecondDont() throws IOException {
115+
commonExpectations(emptySet());
116+
causeCreationExpectations();
117+
118+
when(issue.getCreatedAt()).thenReturn(new Date());
119+
120+
final String body = "test foo, bar tags please.";
121+
when(comment.getBody()).thenReturn(body);
122+
when(comment.getCreatedAt()).thenReturn(new Date());
123+
124+
final String body2 = "no matching in second comment";
125+
when(comment2.getBody()).thenReturn(body2);
126+
when(comment2.getCreatedAt()).thenReturn(new Date());
127+
128+
129+
final ArrayList<GHIssueComment> ghIssueComments = new ArrayList<>();
130+
ghIssueComments.add(comment);
131+
ghIssueComments.add(comment2);
132+
when(remotePr.getComments()).thenReturn(ghIssueComments);
133+
134+
GitHubPRCause cause = new GitHubPRCommentEvent("test ([A-Za-z0-9 ,!]+) tags please.")
135+
.check(trigger, remotePr, localPR, listener);
136+
137+
assertThat(cause, notNullValue());
138+
assertThat(cause.getCommentBody(), is(body));
139+
assertThat(cause.getCommentBodyMatch(), is("foo, bar"));
140+
}
141+
107142
@Test
108143
public void testNoComments() throws IOException {
109144
when(remotePr.getComments()).thenReturn(emptyList());
145+
when(remotePr.getNumber()).thenReturn(14);
146+
when(listener.getLogger()).thenReturn(logger);
110147

111148
GitHubPRCause cause = new GitHubPRCommentEvent("Comment").check(null, remotePr, localPR, listener);
112149

113150
assertNull(cause);
114151
}
115152

116153
@Test
117-
public void testNullLocalPR() {
118-
GitHubPRCause cause = new GitHubPRCommentEvent("").check(null, null, null, listener);
154+
public void testNullLocalPR() throws IOException {
155+
commonExpectations(emptySet());
156+
causeCreationExpectations();
119157

120-
assertNull(cause);
158+
final String body = "test foo, bar tags please.";
159+
when(issue.getCreatedAt()).thenReturn(new Date());
160+
when(comment.getCreatedAt()).thenReturn(new Date());
161+
when(comment.getBody()).thenReturn(body);
162+
163+
final ArrayList<GHIssueComment> ghIssueComments = new ArrayList<>();
164+
ghIssueComments.add(comment);
165+
when(remotePr.getComments()).thenReturn(ghIssueComments);
166+
167+
GitHubPRCause cause = new GitHubPRCommentEvent("test ([A-Za-z0-9 ,!]+) tags please.")
168+
.check(trigger, remotePr, null, listener); // localPR is null
169+
170+
assertThat(cause.getCommentBody(), is(body));
171+
assertThat(cause.getCommentBodyMatch(), is("foo, bar"));
172+
assertNotNull(cause);
121173
}
122174

123175
private void commonExpectations(Set<String> localLabels) throws IOException {

‎mvnw‎

Lines changed: 31 additions & 42 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎mvnw.cmd‎

100644100755
Lines changed: 2 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎travis.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,5 @@ set -o pipefail
66
sudo apt-get update && sudo apt-get install oracle-java8-installer
77
java -version
88

9-
9+
./mvnw -v
1010
./mvnw -Dignore.random.failures=true clean verify -Ptravis | grep -v Download

0 commit comments

Comments
 (0)