Skip to content

Commit b177d98

Browse files
bitwisemanbitwiseman
committed
Populate Check pull requests
Turns out there were quite a few bugs in this area (it was exposed publicly before. Some tweaking was needed and updates to the tests to show this working as expected.
1 parent 5405fb0 commit b177d98

File tree

15 files changed

+1193
-13
lines changed

15 files changed

+1193
-13
lines changed

‎src/main/java/org/kohsuke/github/GHCheckRun.java

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.kohsuke.github;
22

3+
import com.fasterxml.jackson.annotation.JsonProperty;
34
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
45

56
import java.io.IOException;
@@ -17,6 +18,8 @@
1718
@SuppressFBWarnings(value = { "UWF_UNWRITTEN_FIELD", "NP_UNWRITTEN_FIELD", "URF_UNREAD_FIELD" },
1819
justification = "JSON API")
1920
public class GHCheckRun extends GHObject {
21+
22+
@JsonProperty("repository")
2023
GHRepository owner;
2124
GitHub root;
2225

@@ -37,15 +40,32 @@ public class GHCheckRun extends GHObject {
3740

3841
GHCheckRun wrap(GHRepository owner) {
3942
this.owner = owner;
40-
this.root = owner.root;
43+
wrap(owner.root);
4144
return this;
4245
}
4346

4447
GHCheckRun wrap(GitHub root) {
4548
this.root = root;
4649
if (owner != null) {
4750
owner.wrap(root);
51+
if (pullRequests != null && pullRequests.length != 0) {
52+
for (GHPullRequest singlePull : pullRequests) {
53+
singlePull.wrap(owner);
54+
}
55+
}
56+
57+
}
58+
if (checkSuite != null) {
59+
if (owner != null) {
60+
checkSuite.wrap(owner);
61+
} else {
62+
checkSuite.wrap(root);
63+
}
64+
}
65+
if (app != null) {
66+
app.wrapUp(root);
4867
}
68+
4969
return this;
5070
}
5171

@@ -108,14 +128,18 @@ public String getHeadSha() {
108128
/**
109129
* Gets the pull requests participated in this check run.
110130
*
111-
* @return Pull requests of this check run
131+
* Note this field is only populated for events. When getting a {@link GHCheckRun} outside of an event, this is
132+
* always empty.
133+
*
134+
* @return the list of {@link GHPullRequest}s for this check run. Only populated for events.
112135
* @throws IOException
113136
* the io exception
114137
*/
115138
public List<GHPullRequest> getPullRequests() throws IOException {
116139
if (pullRequests != null && pullRequests.length != 0) {
117140
for (GHPullRequest singlePull : pullRequests) {
118-
singlePull.refresh();
141+
// Only refresh if we haven't do so before
142+
singlePull.refresh(singlePull.getTitle());
119143
}
120144
return Collections.unmodifiableList(Arrays.asList(pullRequests));
121145
}

‎src/main/java/org/kohsuke/github/GHCheckSuite.java

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.kohsuke.github;
22

3+
import com.fasterxml.jackson.annotation.JsonProperty;
34
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
45

56
import java.io.IOException;
@@ -17,6 +18,8 @@
1718
@SuppressFBWarnings(value = { "UWF_UNWRITTEN_FIELD", "NP_UNWRITTEN_FIELD", "URF_UNREAD_FIELD" },
1819
justification = "JSON API")
1920
public class GHCheckSuite extends GHObject {
21+
22+
@JsonProperty("repository")
2023
GHRepository owner;
2124
GitHub root;
2225

@@ -35,14 +38,22 @@ public class GHCheckSuite extends GHObject {
3538

3639
GHCheckSuite wrap(GHRepository owner) {
3740
this.owner = owner;
38-
this.root = owner.root;
41+
this.wrap(owner.root);
3942
return this;
4043
}
4144

4245
GHCheckSuite wrap(GitHub root) {
4346
this.root = root;
4447
if (owner != null) {
4548
owner.wrap(root);
49+
if (pullRequests != null && pullRequests.length != 0) {
50+
for (GHPullRequest singlePull : pullRequests) {
51+
singlePull.wrap(owner);
52+
}
53+
}
54+
}
55+
if (app != null) {
56+
app.wrapUp(root);
4657
}
4758
return this;
4859
}
@@ -156,14 +167,18 @@ public GHApp getApp() {
156167
/**
157168
* Gets the pull requests participated in this check suite.
158169
*
159-
* @return Pull requests
170+
* Note this field is only populated for events. When getting a {@link GHCheckSuite} outside of an event, this is
171+
* always empty.
172+
*
173+
* @return the list of {@link GHPullRequest}s for this check suite. Only populated for events.
160174
* @throws IOException
161175
* the io exception
162176
*/
163177
public List<GHPullRequest> getPullRequests() throws IOException {
164178
if (pullRequests != null && pullRequests.length != 0) {
165179
for (GHPullRequest singlePull : pullRequests) {
166-
singlePull.refresh();
180+
// Only refresh if we haven't do so before
181+
singlePull.refresh(singlePull.getTitle());
167182
}
168183
return Collections.unmodifiableList(Arrays.asList(pullRequests));
169184
}

‎src/test/java/org/kohsuke/github/GHEventPayloadTest.java

Lines changed: 54 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import org.junit.Rule;
44
import org.junit.Test;
55

6+
import java.io.IOException;
67
import java.text.SimpleDateFormat;
78
import java.util.Collections;
89
import java.util.TimeZone;
@@ -387,7 +388,25 @@ public void status() throws Exception {
387388
@Payload("check-run")
388389
public void checkRunEvent() throws Exception {
389390
GHEventPayload.CheckRun event = GitHub.offline()
390-
.parseEventPayload(payload.asReader(), GHEventPayload.CheckRun.class);
391+
.parseEventPayload(payload.asReader(mockGitHub::mapToMockGitHub), GHEventPayload.CheckRun.class);
392+
GHCheckRun checkRun = verifyBasicCheckRunEvent(event);
393+
assertThat("pull body not populated offline", checkRun.getPullRequests().get(0).getBody(), nullValue());
394+
assertThat("using offline github", mockGitHub.getRequestCount(), equalTo(0));
395+
396+
gitHub = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl()).build();
397+
event = gitHub.parseEventPayload(payload.asReader(mockGitHub::mapToMockGitHub), GHEventPayload.CheckRun.class);
398+
checkRun = verifyBasicCheckRunEvent(event);
399+
400+
int expectedRequestCount = mockGitHub.isUseProxy() ? 3 : 2;
401+
assertThat("pull body should be populated",
402+
checkRun.getPullRequests().get(0).getBody(),
403+
equalTo("This is a pretty simple change that we need to pull into master."));
404+
assertThat("multiple getPullRequests() calls are made, the pull is populated only once",
405+
mockGitHub.getRequestCount(),
406+
equalTo(expectedRequestCount));
407+
}
408+
409+
private GHCheckRun verifyBasicCheckRunEvent(GHEventPayload.CheckRun event) throws IOException {
391410
assertThat(event.getRepository().getName(), is("Hello-World"));
392411
assertThat(event.getRepository().getOwner().getLogin(), is("Codertocat"));
393412
assertThat(event.getAction(), is("created"));
@@ -406,9 +425,9 @@ public void checkRunEvent() throws Exception {
406425
assertThat(formatter.format(checkRun.getCompletedAt()), is("2019-05-15T20:22:22Z"));
407426

408427
assertThat(checkRun.getConclusion(), is("success"));
409-
assertThat(checkRun.getUrl().toString(),
410-
is("https://api.github.com/repos/Codertocat/Hello-World/check-runs/128620228"));
411-
assertThat(checkRun.getHtmlUrl().toString(), is("https://github.com/Codertocat/Hello-World/runs/128620228"));
428+
assertThat(checkRun.getUrl().toString(), endsWith("/repos/Codertocat/Hello-World/check-runs/128620228"));
429+
assertThat(checkRun.getHtmlUrl().toString(),
430+
endsWith("https://github.com/Codertocat/Hello-World/runs/128620228"));
412431
assertThat(checkRun.getDetailsUrl().toString(), is("https://octocoders.io"));
413432
assertThat(checkRun.getApp().getId(), is(29310L));
414433
assertThat(checkRun.getCheckSuite().getId(), is(118578147L));
@@ -417,18 +436,41 @@ public void checkRunEvent() throws Exception {
417436
assertThat(checkRun.getOutput().getText(), nullValue());
418437
assertThat(checkRun.getOutput().getAnnotationsCount(), is(0));
419438
assertThat(checkRun.getOutput().getAnnotationsUrl().toString(),
420-
is("https://api.github.com/repos/Codertocat/Hello-World/check-runs/128620228/annotations"));
439+
endsWith("/repos/Codertocat/Hello-World/check-runs/128620228/annotations"));
421440

422441
// Checks the deserialization of sender
423442
assertThat(event.getSender().getId(), is(21031067L));
443+
444+
assertThat(checkRun.getPullRequests(), notNullValue());
445+
assertThat(checkRun.getPullRequests().size(), equalTo(1));
446+
assertThat(checkRun.getPullRequests().get(0).getNumber(), equalTo(2));
447+
return checkRun;
424448
}
425449

426450
@Test
427451
@Payload("check-suite")
428452
public void checkSuiteEvent() throws Exception {
429453
GHEventPayload.CheckSuite event = GitHub.offline()
430-
.parseEventPayload(payload.asReader(), GHEventPayload.CheckSuite.class);
454+
.parseEventPayload(payload.asReader(mockGitHub::mapToMockGitHub), GHEventPayload.CheckSuite.class);
455+
GHCheckSuite checkSuite = verifyBasicCheckSuiteEvent(event);
456+
assertThat("pull body not populated offline", checkSuite.getPullRequests().get(0).getBody(), nullValue());
457+
assertThat("using offline github", mockGitHub.getRequestCount(), equalTo(0));
431458

459+
gitHub = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl()).build();
460+
event = gitHub.parseEventPayload(payload.asReader(mockGitHub::mapToMockGitHub),
461+
GHEventPayload.CheckSuite.class);
462+
checkSuite = verifyBasicCheckSuiteEvent(event);
463+
464+
int expectedRequestCount = mockGitHub.isUseProxy() ? 3 : 2;
465+
assertThat("pull body should be populated",
466+
checkSuite.getPullRequests().get(0).getBody(),
467+
equalTo("This is a pretty simple change that we need to pull into master."));
468+
assertThat("multiple getPullRequests() calls are made, the pull is populated only once",
469+
mockGitHub.getRequestCount(),
470+
lessThanOrEqualTo(expectedRequestCount));
471+
}
472+
473+
private GHCheckSuite verifyBasicCheckSuiteEvent(GHEventPayload.CheckSuite event) throws IOException {
432474
assertThat(event.getRepository().getName(), is("Hello-World"));
433475
assertThat(event.getRepository().getOwner().getLogin(), is("Codertocat"));
434476
assertThat(event.getAction(), is("completed"));
@@ -445,7 +487,7 @@ public void checkSuiteEvent() throws Exception {
445487
assertThat(checkSuite.getAfter(), is("ec26c3e57ca3a959ca5aad62de7213c562f8c821"));
446488
assertThat(checkSuite.getLatestCheckRunsCount(), is(1));
447489
assertThat(checkSuite.getCheckRunsUrl().toString(),
448-
is("https://api.github.com/repos/Codertocat/Hello-World/check-suites/118578147/check-runs"));
490+
endsWith("/repos/Codertocat/Hello-World/check-suites/118578147/check-runs"));
449491
assertThat(checkSuite.getHeadCommit().getMessage(), is("Update README.md"));
450492
assertThat(checkSuite.getHeadCommit().getId(), is("ec26c3e57ca3a959ca5aad62de7213c562f8c821"));
451493
assertThat(checkSuite.getHeadCommit().getTreeId(), is("31b122c26a97cf9af023e9ddab94a82c6e77b0ea"));
@@ -457,6 +499,11 @@ public void checkSuiteEvent() throws Exception {
457499
assertThat(formatter.format(checkSuite.getHeadCommit().getTimestamp()), is("2019-05-15T15:20:30Z"));
458500

459501
assertThat(checkSuite.getApp().getId(), is(29310L));
502+
503+
assertThat(checkSuite.getPullRequests(), notNullValue());
504+
assertThat(checkSuite.getPullRequests().size(), equalTo(1));
505+
assertThat(checkSuite.getPullRequests().get(0).getNumber(), equalTo(2));
506+
return checkSuite;
460507
}
461508

462509
@Test

0 commit comments

Comments
 (0)