Skip to content

Commit

Permalink
Merge pull request #220 from bitwiseman/no-clone-tests
Browse files Browse the repository at this point in the history 
PullRequestSCMRevision tests and fixes
  • Loading branch information
@bitwiseman
bitwiseman committed Apr 26, 2019
2 parents 26b138e + 894415f commit a2de33f
Show file tree
Hide file tree
Showing 22 changed files with 2,023 additions and 21 deletions.
35 changes: 22 additions & 13 deletions src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1244,14 +1244,16 @@ protected SCMRevision retrieve(@NonNull String headName, @NonNull TaskListener l
pr, headName, strategy == ChangeRequestCheckoutStrategy.MERGE
);
PullRequestSCMRevision prRev = createPullRequestSCMRevision(pr, head, listener, github, ghRepository);
prRev.validateMergeHash(ghRepository);

switch (strategy) {
case MERGE:
if (Boolean.FALSE.equals(pr.getMergeable())) {
listener.getLogger().format("Resolved %s as pull request %d: Not mergeable.%n%n",
try {
prRev.validateMergeHash(ghRepository);
} catch (AbortException e) {
listener.getLogger().format("Resolved %s as pull request %d: %s.%n%n",
headName,
number);
number,
e.getMessage());
return null;
}
listener.getLogger().format(
Expand Down Expand Up @@ -1438,12 +1440,10 @@ protected SCMRevision retrieve(SCMHead head, TaskListener listener) throws IOExc
PullRequestSCMHead prhead = (PullRequestSCMHead) head;

GHPullRequest pr = getDetailedGHPullRequest(prhead.getNumber(), listener, github, ghRepository);
assert(pr.getMergeable() != null);
if (Boolean.FALSE.equals(pr.getMergeable())) {
throw new AbortException("Pull request " + prhead.getNumber() + " is not mergeable.");
}
PullRequestSCMRevision prRev = createPullRequestSCMRevision(pr, prhead, listener, github, ghRepository);
prRev.validateMergeHash(ghRepository);
if (prhead.isMerge()) {
prRev.validateMergeHash(ghRepository);
}
return prRev;
} else if (head instanceof GitHubTagSCMHead) {
GitHubTagSCMHead tagHead = (GitHubTagSCMHead) head;
Expand Down Expand Up @@ -1475,7 +1475,9 @@ private PullRequestSCMRevision createPullRequestSCMRevision(GHPullRequest pr, Pu
case MERGE:
Connector.checkApiRateLimit(listener, github);
baseHash = ghRepository.getRef("heads/" + pr.getBase().getRef()).getObject().getSha();
mergeHash = pr.getMergeCommitSha();
if (pr.getMergeable() != null) {
mergeHash = Boolean.TRUE.equals(pr.getMergeable()) ? pr.getMergeCommitSha() : PullRequestSCMRevision.NOT_MERGEABLE_HASH;
}
break;
default:
break;
Expand All @@ -1493,13 +1495,20 @@ private GHPullRequest getDetailedGHPullRequest(int number, TaskListener listener

private void ensureDetailedGHPullRequest(GHPullRequest pr, TaskListener listener, GitHub github, GHRepository ghRepository) throws IOException, InterruptedException {
final long sleep = 1000;
while (pr.getMergeableState() == null) {
int retryCountdown = 4;

Connector.checkApiRateLimit(listener, github);
while (pr.getMergeable() == null && retryCountdown > 1) {
listener.getLogger().format(
"Could not determine the mergability of pull request %d. Retrying...%n",
pr.getNumber());
"Could not determine the mergability of pull request %d. Retrying %d more times...%n",
pr.getNumber(),
retryCountdown);
retryCountdown -= 1;
Thread.sleep(sleep);
Connector.checkApiRateLimit(listener, github);
}


}

@Override
Expand Down
20 changes: 16 additions & 4 deletions src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMRevision.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
public class PullRequestSCMRevision extends ChangeRequestSCMRevision<PullRequestSCMHead> {

private static final long serialVersionUID = 1L;
static final String NOT_MERGEABLE_HASH = "NOT_MERGEABLE";

private final @NonNull String baseHash;
private final @NonNull String pullHash;
Expand Down Expand Up @@ -106,11 +107,17 @@ public String getMergeHash() {
return mergeHash;
}

void validateMergeHash(GHRepository repo) throws IOException {
if (this.mergeHash != null) {
void validateMergeHash(@NonNull GHRepository repo) throws IOException {
if (!isMerge()) {
throw new AbortException("Invalid merge hash for pull request " + ((PullRequestSCMHead)this.getHead()).getNumber() + " : Not a merge head");
} else if (this.mergeHash == null) {
throw new AbortException("Invalid merge hash for pull request " + ((PullRequestSCMHead)this.getHead()).getNumber() + " : Unknown merge state " + this.toString());
} else if (this.mergeHash == NOT_MERGEABLE_HASH) {
throw new AbortException("Invalid merge hash for pull request " + ((PullRequestSCMHead)this.getHead()).getNumber() + " : Not mergeable " + this.toString());
} else {
List<String> parents = repo.getCommit(this.mergeHash).getParentSHA1s();
if (parents.size() != 2 || !parents.contains(this.getBaseHash()) || !parents.contains(this.getPullHash())) {
throw new AbortException("Head and base commits do match merge commit for pull request " + ((PullRequestSCMHead)this.getHead()).getNumber() + " : " + this.toString() );
throw new AbortException("Invalid merge hash for pull request " + ((PullRequestSCMHead)this.getHead()).getNumber() + " : Head and base commits do match merge commit " + this.toString() );
}
}
}
Expand All @@ -131,6 +138,11 @@ public int _hashCode() {

@Override
public String toString() {
return getHead() instanceof PullRequestSCMHead && ((PullRequestSCMHead) getHead()).isMerge() ? pullHash + "+" + baseHash + " (" + mergeHash + ")" : pullHash;
String result = pullHash;
if (getHead() instanceof PullRequestSCMHead && ((PullRequestSCMHead) getHead()).isMerge()) {
result += "+" + baseHash +
" (" + StringUtils.defaultIfBlank(mergeHash, "UNKNOWN_MERGE_STATE") + ")";
}
return result;
}
}
84 changes: 83 additions & 1 deletion src/test/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMFileSystemTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,11 @@
import jenkins.scm.api.SCMFile;
import jenkins.scm.api.SCMFileSystem;
import jenkins.scm.api.SCMHead;
import jenkins.scm.api.SCMHeadOrigin;
import jenkins.scm.api.SCMRevision;
import jenkins.scm.api.mixin.ChangeRequestCheckoutStrategy;

import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.StringUtils;
import org.hamcrest.Matchers;
import org.junit.Before;
import org.junit.ClassRule;
Expand All @@ -53,14 +55,18 @@
import org.junit.runners.Parameterized;
import org.jvnet.hudson.test.JenkinsRule;

import hudson.AbortException;

import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
import static com.github.tomakehurst.wiremock.client.WireMock.get;
import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching;
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.core.IsNull.nullValue;
import static org.junit.Assert.assertThat;
import static org.junit.Assume.assumeThat;

Expand All @@ -75,6 +81,30 @@ public class GitHubSCMFileSystemTest {
public static WireMockRuleFactory factory = new WireMockRuleFactory();
public static SCMHead master = new BranchSCMHead("master");
private final SCMRevision revision;

public static PullRequestSCMHead prHead = new PullRequestSCMHead("PR-2", "stephenc", "yolo", "master", 2, (BranchSCMHead) master,
SCMHeadOrigin.Fork.DEFAULT, ChangeRequestCheckoutStrategy.HEAD);
public static PullRequestSCMRevision prHeadRevision = new PullRequestSCMRevision(
prHead,
"8f1314fc3c8284d8c6d5886d473db98f2126071c",
"c0e024f89969b976da165eecaa71e09dc60c3da1");


public static PullRequestSCMHead prMerge = new PullRequestSCMHead("PR-2", "stephenc", "yolo", "master", 2, (BranchSCMHead) master,
SCMHeadOrigin.Fork.DEFAULT, ChangeRequestCheckoutStrategy.MERGE);
public static PullRequestSCMRevision prMergeRevision = new PullRequestSCMRevision(
prMerge,
"8f1314fc3c8284d8c6d5886d473db98f2126071c",
"c0e024f89969b976da165eecaa71e09dc60c3da1",
"38814ca33833ff5583624c29f305be9133f27a40");

public static PullRequestSCMRevision prMergeInvalidRevision = new PullRequestSCMRevision(
prMerge,
"INVALIDc3c8284d8c6d5886d473db98f2126071c",
"c0e024f89969b976da165eecaa71e09dc60c3da1",
"38814ca33833ff5583624c29f305be9133f27a40");


@Rule
public WireMockRule githubRaw = factory.getRule(WireMockConfiguration.options()
.dynamicPort()
Expand Down Expand Up @@ -136,6 +166,14 @@ public void prepareMockGitHub() throws Exception {
new SingleRootFileSource("src/test/resources/api/__files"));
githubRaw.enableRecordMappings(new SingleRootFileSource("src/test/resources/raw/mappings"),
new SingleRootFileSource("src/test/resources/raw/__files"));

githubApi.stubFor(
get(urlEqualTo("/repos/cloudbeers/yolo/pulls/2"))
.willReturn(
aResponse()
.withHeader("Content-Type", "application/json; charset=utf-8")
.withBodyFile("body-yolo-pulls-2-mergeable-true.json")));

githubApi.stubFor(
get(urlMatching(".*")).atPriority(10).willReturn(aResponse().proxiedFrom("https://api.github.com/")));
githubRaw.stubFor(get(urlMatching(".*")).atPriority(10)
Expand Down Expand Up @@ -212,4 +250,48 @@ public void listDir() throws Exception {
hasItem(Matchers.<SCMFile>hasProperty("name", is("manchu.txt"))));
}

@Test
public void resolveDirPRHead() throws Exception {
assumeThat(revision, nullValue());

assertThat(prHeadRevision.isMerge(), is(false));

SCMFileSystem fs = SCMFileSystem.of(source, prHead, prHeadRevision);
assertThat(fs, instanceOf(GitHubSCMFileSystem.class));

// We can't check the sha, but we can check last modified
// which are different for head or merge
assertThat(((GitHubSCMFileSystem)fs).lastModified(),
is(1480691047000L));

assertThat(fs.getRoot().child("fu").getType(), is(SCMFile.Type.DIRECTORY));
}

@Test
public void resolveDirPRMerge() throws Exception {
assumeThat(revision, nullValue());

assertThat(prMergeRevision.isMerge(), is(true));

SCMFileSystem fs = SCMFileSystem.of(source, prMerge, prMergeRevision);
assertThat(fs, instanceOf(GitHubSCMFileSystem.class));

// We can't check the sha, but we can check last modified
// which are different for head or merge
assertThat(((GitHubSCMFileSystem)fs).lastModified(),
is(1480777447000L));

assertThat(fs.getRoot().child("fu").getType(), is(SCMFile.Type.DIRECTORY));
}

@Test(expected = AbortException.class)
public void resolveDirPRInvalidMerge() throws Exception {
assumeThat(revision, nullValue());

assertThat(prMergeInvalidRevision.isMerge(), is(true));

SCMFileSystem fs = SCMFileSystem.of(source, prMerge, prMergeInvalidRevision);
assertThat(fs, instanceOf(GitHubSCMFileSystem.class));
}

}

0 comments on commit a2de33f

Please sign in to comment.