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

685: Show a warning if PR author's full name does not match commit author's full name #840

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -170,9 +170,6 @@ private List<String> botSpecificChecks(Hash finalHash) throws IOException {
ret.add(error);
}

var headHash = pr.headHash();
var originalCommits = localRepo.commitMetadata(baseHash, headHash);

for (var blocker : workItem.bot.blockingCheckLabels().entrySet()) {
if (labels.contains(blocker.getKey())) {
ret.add(blocker.getValue());
@@ -115,8 +115,18 @@ Hash commit(Hash finalHead, Namespace namespace, String censusDomain, String spo
throw new CommitFailure("Merge PRs can only be created by known OpenJDK authors.");
}

var headCommit = localRepo.commitMetadata(pr.headHash().hex() + "^.." + pr.headHash().hex()).get(0);
author = headCommit.author();
var headHash = pr.headHash();
var headCommit = localRepo.lookup(headHash).get();
var headAuthor = headCommit.author();
var prAuthor = pr.author();
if (!prAuthor.fullName().equals(prAuthor.userName())) {
if (!headAuthor.name().equals(prAuthor.fullName())) {
throw new CommitFailure("The HEAD commit of this pull request, " + headHash.abbreviate() +
", has a different author, `" + headAuthor.name() + "`" +
", than the author of this pull request: `" + prAuthor.fullName() + "`");
}
}
author = headAuthor;
} else {
author = new Author(contributor.fullName().orElseThrow(), contributor.username() + "@" + censusDomain);
}
@@ -1630,4 +1630,41 @@ void overrideNonexistingJcheckConf(TestInfo testInfo) throws IOException {
assertEquals(Set.of("rfr", "ready"), new HashSet<>(pr.labels()));
}
}

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

var censusBuilder = credentials.getCensusBuilder()
.addCommitter(committer.forge().currentUser().id())
.addReviewer(reviewer.forge().currentUser().id());
var mergeBot = PullRequestBot.newBuilder().repo(reviewer).censusRepo(censusBuilder.build()).build();

// Populate the projects repository
var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType());
var masterHash = localRepo.resolve("master").orElseThrow();
assertFalse(CheckableRepository.hasBeenEdited(localRepo));
localRepo.push(masterHash, author.url(), "master", true);

// Make a change with a corresponding PR with an empty e-mail
var editHash = CheckableRepository.appendAndCommit(localRepo, "Content", "A commit", "A Random User", "a.random.user@foo.com");
localRepo.push(editHash, author.url(), "refs/heads/edit", true);
var pr = credentials.createPullRequest(author, "master", "edit", "This is a pull request");

// Run the bot
TestBotRunner.runPeriodicItems(mergeBot);

// The bot should respond with a failure about different authors
pr = author.pullRequest(pr.id());
assertFalse(pr.labels().contains("rfr"));
assertTrue(pr.body().contains("The HEAD commit of this pull request"));
assertTrue(pr.body().contains("has a different author"));
assertTrue(pr.body().contains("than the author of this pull request"));
}
}
}
@@ -805,7 +805,8 @@ void contributorMissingEmail(TestInfo testInfo) throws IOException {
localRepo.push(masterHash, author.url(), "master", true);

// Make a change with a corresponding PR with an empty e-mail
var editHash = CheckableRepository.appendAndCommit(localRepo, "Content", "A commit", "Duke", "");
var authorFullName = author.forge().currentUser().fullName();
var editHash = CheckableRepository.appendAndCommit(localRepo, "Content", "A commit", authorFullName, "");
localRepo.push(editHash, author.url(), "refs/heads/edit", true);
var pr = credentials.createPullRequest(author, "master", "edit", "This is a pull request");

@@ -58,7 +58,9 @@ private void runSponsortest(TestInfo testInfo, boolean isAuthor) throws IOExcept
localRepo.push(masterHash, author.url(), "master", true);

// Make a change with a corresponding PR
var editHash = CheckableRepository.appendAndCommit(localRepo);
var authorFullName = author.forge().currentUser().fullName();
var authorEmail = "ta@none.none";
var editHash = CheckableRepository.appendAndCommit(localRepo, "This is a new line", "Append commit", authorFullName, authorEmail);
localRepo.push(editHash, author.url(), "edit", true);
var pr = credentials.createPullRequest(author, "master", "edit", "This is a pull request");

@@ -106,8 +108,8 @@ private void runSponsortest(TestInfo testInfo, boolean isAuthor) throws IOExcept
assertEquals("Generated Author 2", headCommit.author().name());
assertEquals("integrationauthor2@openjdk.java.net", headCommit.author().email());
} else {
assertEquals("testauthor", headCommit.author().name());
assertEquals("ta@none.none", headCommit.author().email());
assertEquals(authorFullName, headCommit.author().name());
assertEquals(authorEmail, headCommit.author().email());
}

assertEquals("Generated Reviewer 1", headCommit.committer().name());
@@ -252,7 +254,8 @@ void sponsorAfterChanges(TestInfo testInfo) throws IOException {
localRepo.push(masterHash, author.url(), "master", true);

// Make a change with a corresponding PR
var editHash = CheckableRepository.appendAndCommit(localRepo);
var authorFullName = author.forge().currentUser().fullName();
var editHash = CheckableRepository.appendAndCommit(localRepo, "This is a new line", "Append commit", authorFullName, "ta@none.none");
localRepo.push(editHash, author.url(), "edit", true);
var pr = credentials.createPullRequest(author, "master", "edit", "This is a pull request");

@@ -276,7 +279,7 @@ void sponsorAfterChanges(TestInfo testInfo) throws IOException {
assertTrue(pr.labels().contains("sponsor"));

// Push another change
var updateHash = CheckableRepository.appendAndCommit(localRepo,"Yet more stuff");
var updateHash = CheckableRepository.appendAndCommit(localRepo, "Yet more stuff", "Append commit", authorFullName, "ta@none.none");
localRepo.push(updateHash, author.url(), "edit");

// Make sure that the push registered
@@ -487,7 +490,8 @@ void sponsorAfterFailingCheck(TestInfo testInfo) throws IOException {
localRepo.push(masterHash, author.url(), "master", true);

// Make a change with a corresponding PR
var editHash = CheckableRepository.appendAndCommit(localRepo);
var authorFullName = author.forge().currentUser().fullName();
var editHash = CheckableRepository.appendAndCommit(localRepo, "This is a new line", "Append commit", authorFullName, "ta@none.none");
localRepo.push(editHash, author.url(), "edit", true);
var pr = credentials.createPullRequest(author, "master", "edit", "This is a pull request");