diff --git a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java index e7a13567e..b1e5bea1a 100644 --- a/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java +++ b/bots/mlbridge/src/main/java/org/openjdk/skara/bots/mlbridge/ArchiveItem.java @@ -297,14 +297,11 @@ private static Optional findLastQuoted(String commentText, List generated, Comment comment) { - ArchiveItem lastCommentOrReview = generated.get(0); var eligible = new ArrayList(); - 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 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 generated, Hash hash) { diff --git a/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/ArchiveItemTests.java b/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/ArchiveItemTests.java index 919e9917e..79536ab6c 100644 --- a/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/ArchiveItemTests.java +++ b/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/ArchiveItemTests.java @@ -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"))); } } } diff --git a/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java b/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java index f4a61e2b1..c1771a30c 100644 --- a/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java +++ b/bots/mlbridge/src/test/java/org/openjdk/skara/bots/mlbridge/MailingListBridgeBotTests.java @@ -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);