Skip to content
Permalink
Browse files
715: Generated email wrongly includes previous unrelated comments
Reviewed-by: erikj
  • Loading branch information
rwestberg committed Apr 8, 2021
1 parent b404a14 commit 4ca65debfe21e5b82b82421cb60a5cc9221a5ed5
@@ -297,14 +297,11 @@ private static Optional<ArchiveItem> findLastQuoted(String commentText, List<Arc
}

static ArchiveItem findParent(List<ArchiveItem> generated, Comment comment) {
ArchiveItem lastCommentOrReview = generated.get(0);
var eligible = new ArrayList<ArchiveItem>();
eligible.add(lastCommentOrReview);
for (var item : generated) {
if (item.id().startsWith("pc") || item.id().startsWith("rv")) {
if (item.createdAt().isBefore(comment.createdAt()) && item.createdAt().isAfter(lastCommentOrReview.createdAt())) {
lastCommentOrReview = item;
eligible.add(lastCommentOrReview);
if (item.createdAt().isBefore(comment.createdAt())) {
eligible.add(item);
}
}
}
@@ -318,7 +315,15 @@ static ArchiveItem findParent(List<ArchiveItem> generated, Comment comment) {
return lastQuoted.get();
}

return lastCommentOrReview;
ArchiveItem lastRevisionItem = generated.get(0);
for (var item : generated) {
if (item.id().startsWith("ha")) {
if (item.createdAt().isBefore(comment.createdAt())) {
lastRevisionItem = item;
}
}
}
return lastRevisionItem;
}

static ArchiveItem findRevisionItem(List<ArchiveItem> generated, Hash hash) {
@@ -23,12 +23,14 @@
package org.openjdk.skara.bots.mlbridge;

import org.junit.jupiter.api.*;
import org.openjdk.skara.forge.PullRequest;
import org.openjdk.skara.forge.*;
import org.openjdk.skara.host.HostUser;
import org.openjdk.skara.issuetracker.Comment;
import org.openjdk.skara.test.HostCredentials;
import org.openjdk.skara.test.*;
import org.openjdk.skara.vcs.Repository;

import java.io.IOException;
import java.net.URI;
import java.time.ZonedDateTime;
import java.util.List;

@@ -41,14 +43,21 @@ private Comment createComment(HostUser user, String body) {
return new Comment(Integer.toString(curId++), body, user, ZonedDateTime.now(), ZonedDateTime.now());
}

private ArchiveItem fromPullRequest(PullRequest pr, Repository repo) throws IOException {
var base = repo.resolve("master").orElseThrow();
return ArchiveItem.from(pr, repo, null, URI.create("http://www.example.com"), "", null, null, ZonedDateTime.now(), ZonedDateTime.now(), base, base, "", "");
}

private ArchiveItem fromComment(PullRequest pr, Comment comment) {
return ArchiveItem.from(pr, comment, null, null);
}

@Test
void simple(TestInfo testInfo) throws IOException {
try (var credentials = new HostCredentials(testInfo)) {
try (var credentials = new HostCredentials(testInfo);
var tempFolder = new TemporaryDirectory()) {
var repo = credentials.getHostedRepository();
var localRepo = CheckableRepository.init(tempFolder.path(), repo.repositoryType());
var pr = credentials.createPullRequest(repo, "master", "master", "Test");

var user1 = HostUser.create("1", "user1", "User Uno");
@@ -58,17 +67,18 @@ void simple(TestInfo testInfo) throws IOException {
var c1 = createComment(user1, "First comment\nwith two lines");
var c2 = createComment(user2, "Second comment");

var a0 = fromPullRequest(pr, localRepo);
var a1 = fromComment(pr, c1);
var a2 = fromComment(pr, c2);

assertEquals(a2, ArchiveItem.findParent(List.of(a1, a2), createComment(user3, "Plain reply")));
assertEquals(a0, ArchiveItem.findParent(List.of(a0, a1, a2), createComment(user3, "Plain unrelated reply")));

assertEquals(a1, ArchiveItem.findParent(List.of(a1, a2), createComment(user3, "> First comment\n\nI agree")));
assertEquals(a1, ArchiveItem.findParent(List.of(a1, a2), createComment(user3, "> First comment\n>with two lines\n\nI agree")));
assertEquals(a1, ArchiveItem.findParent(List.of(a1, a2), createComment(user3, "\n> First comment\n\nI agree")));
assertEquals(a1, ArchiveItem.findParent(List.of(a0, a1, a2), createComment(user3, "> First comment\n\nI agree")));
assertEquals(a1, ArchiveItem.findParent(List.of(a0, a1, a2), createComment(user3, "> First comment\n>with two lines\n\nI agree")));
assertEquals(a1, ArchiveItem.findParent(List.of(a0, a1, a2), createComment(user3, "\n> First comment\n\nI agree")));

assertEquals(a1, ArchiveItem.findParent(List.of(a1, a2), createComment(user3, "@user1 I agree")));
assertEquals(a1, ArchiveItem.findParent(List.of(a1, a2), createComment(user3, "@user1\nI agree")));
assertEquals(a1, ArchiveItem.findParent(List.of(a0, a1, a2), createComment(user3, "@user1 I agree")));
assertEquals(a1, ArchiveItem.findParent(List.of(a0, a1, a2), createComment(user3, "@user1\nI agree")));
}
}
}
@@ -22,16 +22,15 @@
*/
package org.openjdk.skara.bots.mlbridge;

import org.junit.jupiter.api.*;
import org.openjdk.skara.email.EmailAddress;
import org.openjdk.skara.forge.*;
import org.openjdk.skara.issuetracker.Issue;
import org.openjdk.skara.json.JSON;
import org.openjdk.skara.mailinglist.*;
import org.openjdk.skara.network.URIBuilder;
import org.openjdk.skara.mailinglist.MailingListServerFactory;
import org.openjdk.skara.test.*;
import org.openjdk.skara.vcs.Repository;
import org.openjdk.skara.json.JSON;

import org.junit.jupiter.api.*;

import java.io.*;
import java.nio.charset.StandardCharsets;
@@ -259,7 +258,7 @@ void simpleArchive(TestInfo testInfo) throws IOException {

// Remove the rfr flag and post another comment
pr.addLabel("rfr");
pr.addComment("This is another comment");
pr.addComment("@" + pr.author().username() +" This is another comment");

// Run another archive pass
TestBotRunner.runPeriodicItems(mlBot);
@@ -1178,7 +1177,7 @@ void commentWithMention(TestInfo testInfo) throws IOException {

// The first comment should be quoted more often than the second
Repository.materialize(archiveFolder.path(), archive.url(), "master");
assertEquals(3, archiveContainsCount(archiveFolder.path(), "First comment"));
assertEquals(2, archiveContainsCount(archiveFolder.path(), "First comment"));
assertEquals(1, archiveContainsCount(archiveFolder.path(), "Second comment"));
}
}
@@ -1296,7 +1295,8 @@ void commentWithQuote(TestInfo testInfo) throws IOException {
// Make two comments from different authors
var reviewPr = reviewer.pullRequest(pr.id());
reviewPr.addComment("First comment\nsecond line");
pr.addComment("Second comment\nfourth line");
var authorPr = author.pullRequest(pr.id());
authorPr.addComment("Second comment\nfourth line");

TestBotRunner.runPeriodicItems(mlBot);
listServer.processIncoming();
@@ -1305,11 +1305,10 @@ void commentWithQuote(TestInfo testInfo) throws IOException {
TestBotRunner.runPeriodicItems(mlBot);
listServer.processIncoming();

// The first comment should be quoted more often than the second
// The first comment should be replied to once, and the original post once
Repository.materialize(archiveFolder.path(), archive.url(), "master");
assertEquals(2, archiveContainsCount(archiveFolder.path(), "First comment"));
assertEquals(3, archiveContainsCount(archiveFolder.path(), "First comm"));
assertEquals(1, archiveContainsCount(archiveFolder.path(), "Second comment"));
assertEquals(1, archiveContainsCount(archiveFolder.path(), Pattern.quote(reviewPr.author().fullName()) + ".* wrote"));
assertEquals(1, archiveContainsCount(archiveFolder.path(), Pattern.quote(pr.author().fullName()) + ".* wrote"));
}
}

@@ -2352,7 +2351,7 @@ void replyToEmptyReview(TestInfo testInfo) throws IOException {
TestBotRunner.runPeriodicItems(mlBot);
listServer.processIncoming();

pr.addComment("Thanks for the review!");
pr.addComment("@" + reviewer.forge().currentUser().username() + " Thanks for the review!");
TestBotRunner.runPeriodicItems(mlBot);
listServer.processIncoming();

@@ -3107,7 +3106,7 @@ void jsonArchive(TestInfo testInfo) throws IOException {

// Remove the rfr flag and post another comment
pr.addLabel("rfr");
pr.addComment("This is another comment");
pr.addComment("@" + pr.author().username() + " This is another comment");

// Run another archive pass
TestBotRunner.runPeriodicItems(mlBot);

1 comment on commit 4ca65de

@openjdk-notifier
Copy link

@openjdk-notifier openjdk-notifier bot commented on 4ca65de Apr 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.