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.

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

Add support for adding an issue when formatting the commit message #148

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -191,7 +191,8 @@ public void visit(MessageIssue issue) {

@Override
public void visit(IssuesIssue issue) {
messages.add("The commit message does not reference any issue");
messages.add("The commit message does not reference any issue. To add an issue reference to this PR, " +
"edit the title to be of the format <issue number>: <message>.");
failedChecks.add(issue.check().getClass());
readyForReview = false;
}
@@ -81,13 +81,13 @@ private String commitMessage(List<Review> activeReviews, Namespace namespace, bo
.collect(Collectors.toList());

var summary = Summary.summary(pr.repository().host().getCurrentUserDetails(), comments);

var commitMessage = CommitMessage.title(isMerge ? "Merge" : pr.getTitle())
.contributors(additionalContributors)
var issue = Issue.fromString(pr.getTitle());
var commitMessageBuilder = issue.map(CommitMessage::title).orElseGet(() -> CommitMessage.title(isMerge ? "Merge" : pr.getTitle()));
commitMessageBuilder.contributors(additionalContributors)
.reviewers(reviewers);
summary.ifPresent(commitMessage::summary);
summary.ifPresent(commitMessageBuilder::summary);

return String.join("\n", commitMessage.format(CommitMessageFormatters.v1));
return String.join("\n", commitMessageBuilder.format(CommitMessageFormatters.v1));
}

private Hash commitSquashed(List<Review> activeReviews, Namespace namespace, String censusDomain, String sponsorId) throws IOException {
@@ -28,7 +28,7 @@
import org.junit.jupiter.api.*;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.*;
import java.util.*;
import java.util.regex.Pattern;

@@ -733,4 +733,51 @@ void missingReadyComment(TestInfo testInfo) throws IOException {
assertTrue(pr.getLabels().contains("rfr"));
}
}

@Test
void issueIssue(TestInfo testInfo) throws IOException {
try (var credentials = new HostCredentials(testInfo);
var tempFolder = new TemporaryDirectory()) {
var author = credentials.getHostedRepository();
var reviewer = credentials.getHostedRepository();

var censusBuilder = credentials.getCensusBuilder()
.addAuthor(author.host().getCurrentUserDetails().id())
.addReviewer(reviewer.host().getCurrentUserDetails().id());
var checkBot = new PullRequestBot(author, censusBuilder.build(), "master", Map.of(), Map.of(),
Map.of(), Set.of(), Map.of());

// Populate the projects repository
var localRepo = CheckableRepository.init(tempFolder.path(), author.getRepositoryType(), Path.of("appendable.txt"),
Set.of("issues"));
var masterHash = localRepo.resolve("master").orElseThrow();
localRepo.push(masterHash, author.getUrl(), "master", true);

// Make a change with a corresponding PR
var editHash = CheckableRepository.appendAndCommit(localRepo);
localRepo.push(editHash, author.getUrl(), "edit", true);
var pr = credentials.createPullRequest(author, "master", "edit", "This is a pull request");

// Check the status
TestBotRunner.runPeriodicItems(checkBot);

// Verify that the check failed
var checks = pr.getChecks(editHash);
assertEquals(1, checks.size());
var check = checks.get("jcheck");
assertEquals(CheckStatus.FAILURE, check.status());

// Add an issue to the title
pr.setTitle("1234: This is a pull request");

// Check the status again
TestBotRunner.runPeriodicItems(checkBot);

// The check should now be successful
checks = pr.getChecks(editHash);
assertEquals(1, checks.size());
check = checks.get("jcheck");
assertEquals(CheckStatus.SUCCESS, check.status());
}
}
}
@@ -109,6 +109,12 @@
*/
String getTitle();

/**
* Update the title of the request.
* @param title
*/
void setTitle(String title);

/**
* The main body of the request.
* @return
@@ -212,6 +212,11 @@ public String getTitle() {
return json.get("title").asString();
}

@Override
public void setTitle(String title) {
throw new RuntimeException("not implemented yet");

This comment has been minimized.

Copy link
@edvbld

edvbld Sep 19, 2019

Member

Won't this be an issue if we run the unit tests as integration tests against real hosts?

This comment has been minimized.

Copy link
@rwestberg

rwestberg Sep 20, 2019

Author Member

You are quite right, I was planning to save it for another task, as there are already existing issues with the real host tests..

This comment has been minimized.

Copy link
@edvbld

edvbld Sep 20, 2019

Member

Ah ok, lets tackle it later then 👍

}

@Override
public String getBody() {
var body = json.get("body").asString();
@@ -254,6 +254,11 @@ public String getTitle() {
return json.get("title").asString();
}

@Override
public void setTitle(String title) {
throw new RuntimeException("not implemented yet");

This comment has been minimized.

Copy link
@edvbld

edvbld Sep 19, 2019

Member

Same issue here?

}

@Override
public String getBody() {
var body = json.get("description").asString();
@@ -24,9 +24,10 @@

import org.openjdk.skara.vcs.*;

import java.io.*;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.*;
import java.util.Set;

public class CheckableRepository {
private static String markerLine = "The very first line\n";
@@ -38,7 +39,7 @@ private static Path checkableFile(Path path) throws IOException {
}
}

public static Repository init(Path path, VCS vcs, Path appendableFilePath) throws IOException {
public static Repository init(Path path, VCS vcs, Path appendableFilePath, Set<String> checks) throws IOException {
var repo = Repository.init(path, vcs);

Files.createDirectories(path.resolve(".checkable"));
@@ -60,8 +61,9 @@ public static Repository init(Path path, VCS vcs, Path appendableFilePath) throw
output.append("project=test\n");
output.append("\n");
output.append("[checks]\n");
output.append("error=author,reviewers,whitespace\n");
output.append("\n");
output.append("error=");
output.append(String.join(",", checks));
output.append("\n\n");
output.append("[census]\n");
output.append("version=0\n");
output.append("domain=openjdk.java.net\n");
@@ -79,6 +81,10 @@ public static Repository init(Path path, VCS vcs, Path appendableFilePath) throw
return repo;
}

public static Repository init(Path path, VCS vcs, Path appendableFilePath) throws IOException {
return init(path, vcs, appendableFilePath, Set.of("author", "reviewers", "whitespace"));
}

public static Repository init(Path path, VCS vcs) throws IOException {
return init(path, vcs, Path.of("appendable.txt"));
}
@@ -39,14 +39,13 @@
private final HostUserDetails user;
private final String targetRef;
private final String sourceRef;
private final String title;
private final List<String> body;
private final PullRequestData data;

private static class PullRequestData {
private Hash headHash;
PullRequest.State state = PullRequest.State.OPEN;
String body = "";
String title = "";
final List<Comment> comments = new ArrayList<>();
final List<ReviewComment> reviewComments = new ArrayList<>();
final Set<Check> checks = new HashSet<>();
@@ -56,15 +55,13 @@
ZonedDateTime lastUpdate = created;
}

private TestPullRequest(TestHostedRepository repository, String id, HostUserDetails author, HostUserDetails user, String targetRef, String sourceRef, String title, List<String> body, PullRequestData data) {
private TestPullRequest(TestHostedRepository repository, String id, HostUserDetails author, HostUserDetails user, String targetRef, String sourceRef, PullRequestData data) {
this.repository = repository;
this.id = id;
this.author = author;
this.user = user;
this.targetRef = targetRef;
this.sourceRef = sourceRef;
this.title = title;
this.body = body;
this.data = data;

try {
@@ -80,13 +77,14 @@ private TestPullRequest(TestHostedRepository repository, String id, HostUserDeta

static TestPullRequest createNew(TestHostedRepository repository, String id, String targetRef, String sourceRef, String title, List<String> body) {
var data = new PullRequestData();
data.title = title;
data.body = String.join("\n", body);
var pr = new TestPullRequest(repository, id, repository.host().getCurrentUserDetails(), repository.host().getCurrentUserDetails(), targetRef, sourceRef, title, body, data);
var pr = new TestPullRequest(repository, id, repository.host().getCurrentUserDetails(), repository.host().getCurrentUserDetails(), targetRef, sourceRef, data);
return pr;
}

static TestPullRequest createFrom(TestHostedRepository repository, TestPullRequest other) {
var pr = new TestPullRequest(repository, other.id, other.author, repository.host().getCurrentUserDetails(), other.targetRef, other.sourceRef, other.title, other.body, other.data);
var pr = new TestPullRequest(repository, other.id, other.author, repository.host().getCurrentUserDetails(), other.targetRef, other.sourceRef, other.data);
return pr;
}

@@ -172,7 +170,13 @@ public Hash getTargetHash() {

@Override
public String getTitle() {
return title;
return data.title;
}

@Override
public void setTitle(String title) {
data.title = title;
data.lastUpdate = ZonedDateTime.now();
}

@Override
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.