Skip to content

Commit

Permalink
feat(#1237): Verify pull request check runs before merging changes
Browse files Browse the repository at this point in the history
  • Loading branch information
volodya-lombrozo committed Mar 22, 2023
1 parent e715ccd commit 620d707
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 25 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ OF THE POSSIBILITY OF SUCH DAMAGE.
<dependency>
<groupId>com.jcabi</groupId>
<artifactId>jcabi-github</artifactId>
<version>1.4.2</version>
<version>2.0-SNAPSHOT</version>
</dependency>
<dependency>
<groupId>com.jcabi</groupId>
Expand Down
49 changes: 38 additions & 11 deletions src/main/java/com/rultor/agents/github/qtn/QnMerge.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
package com.rultor.agents.github.qtn;

import com.jcabi.aspects.Immutable;
import com.jcabi.github.Check;
import com.jcabi.github.Comment;
import com.jcabi.github.Issue;
import com.jcabi.github.Pull;
Expand All @@ -40,6 +41,7 @@
import com.rultor.agents.github.Req;
import java.io.IOException;
import java.net.URI;
import java.util.Collection;
import java.util.ResourceBundle;
import lombok.EqualsAndHashCode;
import lombok.ToString;
Expand Down Expand Up @@ -72,21 +74,29 @@ public Req understand(final Comment.Smart comment,
final Issue.Smart issue = new Issue.Smart(comment.issue());
final Req req;
if (issue.isPull() && issue.isOpen()) {
new Answer(comment).post(
true,
String.format(
QnMerge.PHRASES.getString("QnMerge.start"),
home.toASCIIString()
)
);
Logger.info(
this, "merge request found in %s#%d, comment #%d",
issue.repo().coordinates(), issue.number(), comment.number()
);
req = QnMerge.pack(
comment,
issue.repo().pulls().get(issue.number())
);
if (QnMerge.allChecksSuccessful(issue.pull())) {
new Answer(comment).post(
true,
String.format(
QnMerge.PHRASES.getString("QnMerge.start"),
home.toASCIIString()
)
);
req = QnMerge.pack(
comment,
issue.repo().pulls().get(issue.number())
);
} else {
new Answer(comment).post(
false,
QnMerge.PHRASES.getString("QnMerge.checks-are-failed")
);
req = Req.EMPTY;
}
} else {
new Answer(comment).post(
false,
Expand Down Expand Up @@ -164,4 +174,21 @@ private static Req pack(final Comment.Smart comment,
return req;
}

/**
* Checks if all checks are successful.
* @param pull Pull
* @return True if all checks are successful
* @throws IOException If fails
*/
private static boolean allChecksSuccessful(final Pull pull) throws IOException {
boolean result = true;
for (final Check check : pull.checks().all()) {
if (!check.successful()) {
result = false;
break;
}
}
return result;
}

}
45 changes: 32 additions & 13 deletions src/test/java/com/rultor/agents/github/qtn/QnMergeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,14 @@
*/
package com.rultor.agents.github.qtn;

import com.jcabi.github.Check;
import com.jcabi.github.Checks;
import com.jcabi.github.Comment;
import com.jcabi.github.Comments;
import com.jcabi.github.Pull;
import com.jcabi.github.Repo;
import com.jcabi.github.mock.MkBranches;
import com.jcabi.github.mock.MkChecks;
import com.jcabi.github.mock.MkGithub;
import com.jcabi.matchers.XhtmlMatchers;
import com.rultor.agents.github.Req;
Expand All @@ -45,6 +49,7 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import org.xembly.Directives;
import org.xembly.Xembler;

Expand Down Expand Up @@ -74,6 +79,7 @@ final class QnMergeTest {
* All pull request comments.
*/
private transient Comments comments;
private Pull pull;

/**
* Initial phase for all tests.
Expand All @@ -87,8 +93,9 @@ public void setUp() throws IOException {
final String base = "base";
branches.create(head, "abcdef4");
branches.create(base, "abcdef5");
pull = repo.pulls().create("", head, base);
this.comments = repo.issues()
.get(repo.pulls().create("", head, base).number())
.get(pull.number())
.comments();
}

Expand All @@ -98,7 +105,7 @@ public void setUp() throws IOException {
*/
@Test
public void buildsRequest() throws Exception {
final String request = new Xembler(this.requestDirectives()).xml();
final String request = new Xembler(this.mergeRequest()).xml();
MatcherAssert.assertThat(
request,
Matchers.allOf(
Expand Down Expand Up @@ -130,36 +137,48 @@ public void buildsRequest() throws Exception {
* QnMerge can not build a request because some GitHub checks
* were failed.
* @throws Exception In case of error
* @todo #1237:90min We need to implement a verification logic
* for GitHub checks to determine whether they were completed
* correctly or not. If not, Rultor should not create a merge
* request and must display a comment to the user. Once the logic
* is implemented, we can enable that test.
*/
@Test
@Disabled
public void stopsBecauseCiChecksFailed() throws Exception {
final String request = new Xembler(this.requestDirectives()).xml();
final MkChecks checks = (MkChecks) this.pull.checks();
checks.create(Check.Status.IN_PROGRESS, Check.Conclusion.SUCCESS);
this.mergeRequest();
MatcherAssert.assertThat(
new Comment.Smart(this.comments.get(1)).body(),
Matchers.is(QnMergeTest.COMMAND)
);
MatcherAssert.assertThat(
new Comment.Smart(this.comments.get(2)).body(),
Matchers.equalTo(
Matchers.containsString(
QnMergeTest.PHRASES.getString("QnMerge.checks-are-failed")
)
);
MatcherAssert.assertThat(request, Matchers.equalTo(Req.EMPTY));
}

@Test
public void continuesBecauseCiChecksSuccessful() throws Exception {
final MkChecks checks = (MkChecks) this.pull.checks();
checks.create(Check.Status.COMPLETED, Check.Conclusion.SUCCESS);
this.mergeRequest();
MatcherAssert.assertThat(
new Comment.Smart(this.comments.get(1)).body(),
Matchers.is(QnMergeTest.COMMAND)
);
MatcherAssert.assertThat(
new Comment.Smart(this.comments.get(2)).body(),
Matchers.containsString(
String.format(QnMergeTest.PHRASES.getString("QnMerge.start"), "#")
)
);
}

/**
* Request directives.
* Merge request directives.
* @return Directives
* @throws IOException In case of error
* @throws URISyntaxException In case of error
*/
private Directives requestDirectives() throws IOException,
private Directives mergeRequest() throws IOException,
URISyntaxException {
return new Directives()
.add("request")
Expand Down

0 comments on commit 620d707

Please sign in to comment.