CommentJsonMigratorTest: Check change equivalence more explicitly

ChangeBundle is pretty tightly coupled to the ReviewDb migration, and
contains a lot of fuzzy-matching cases that aren't really relevant for
direct comparison of NoteDb changes. We want to delete it sooner rather
than later.

There are two important properties being tested here:
 * The draft and published Comment entities are exactly equal before and
   after migration.
 * Migration doesn't affect anything else about the NoteDb commits,
   specifically the commit messages.

Change the implementation of assertNoDifferences to test these directly
instead of relying on ChangeBundle. This new implementation subsumes the
other assertLogEqualExceptTrees calls in the test method bodies.

Change-Id: I5955674ff30eaa6664217693d6810454dc0df62c

javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java

@@ -38,6 +38,7 @@ import com.google.gerrit.server.util.time.TimeUtil;
 import com.google.gerrit.testing.TestChanges;
 import com.google.inject.Inject;
 import java.io.ByteArrayOutputStream;
+import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.eclipse.jgit.junit.TestRepository;
@@ -185,7 +186,6 @@ public class CommentJsonMigratorTest extends AbstractChangeNotesTest {
 
     // Comments at each commit all have JSON format.
     ImmutableList<RevCommit> newLog = log(project, RefNames.changeMetaRef(c.getId()));
-    assertLogEqualExceptTrees(newLog, oldLog);
     assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(0))).isEmpty();
     assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(1))).isEmpty();
     assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(2)))
@@ -297,7 +297,6 @@ public class CommentJsonMigratorTest extends AbstractChangeNotesTest {
     // Comments at each commit all have JSON format.
     ImmutableList<RevCommit> newOwnerLog =
         log(allUsers, RefNames.refsDraftComments(c.getId(), changeOwner.getAccountId()));
-    assertLogEqualExceptTrees(newOwnerLog, oldOwnerLog);
     assertThat(getLegacyFormatMapForDraftComments(notes, newOwnerLog.get(0)))
         .containsExactly(ownerCommentPs1.key, false);
     assertThat(getLegacyFormatMapForDraftComments(notes, newOwnerLog.get(1)))
@@ -305,7 +304,6 @@ public class CommentJsonMigratorTest extends AbstractChangeNotesTest {
 
     ImmutableList<RevCommit> newOtherLog =
         log(allUsers, RefNames.refsDraftComments(c.getId(), otherUser.getAccountId()));
-    assertLogEqualExceptTrees(newOtherLog, oldOtherLog);
     assertThat(getLegacyFormatMapForDraftComments(notes, newOtherLog.get(0)))
         .containsExactly(otherCommentPs1.key, false);
   }
@@ -392,7 +390,6 @@ public class CommentJsonMigratorTest extends AbstractChangeNotesTest {
 
     // Comments at each commit all have JSON format.
     ImmutableList<RevCommit> newLog = log(project, RefNames.changeMetaRef(c.getId()));
-    assertLogEqualExceptTrees(newLog, oldLog);
     assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(0))).isEmpty();
     assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(1))).isEmpty();
     assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(2))).isEmpty();
@@ -491,17 +488,35 @@ public class CommentJsonMigratorTest extends AbstractChangeNotesTest {
   }
 
   private ImmutableList<RevCommit> log(Project.NameKey project, String refName) throws Exception {
-    try (Repository repo = repoManager.openRepository(project);
-        RevWalk rw = new RevWalk(repo)) {
+    try (Repository repo = repoManager.openRepository(project)) {
+      return log(repo, refName);
+    }
+  }
+
+  private ImmutableList<RevCommit> log(Repository repo, String refName) throws Exception {
+    try (RevWalk rw = new RevWalk(repo)) {
       rw.sort(RevSort.TOPO);
       rw.sort(RevSort.REVERSE);
       Ref ref = repo.exactRef(refName);
-      checkArgument(ref != null, "missing ref: %s", refName);
+      if (ref == null) {
+        return ImmutableList.of();
+      }
       rw.markStart(rw.parseCommit(ref.getObjectId()));
       return ImmutableList.copyOf(rw);
     }
   }
 
+  private ImmutableListMultimap<String, RevCommit> logAll(
+      Project.NameKey project, Collection<Ref> refs) throws Exception {
+    ImmutableListMultimap.Builder<String, RevCommit> logs = ImmutableListMultimap.builder();
+    try (Repository repo = repoManager.openRepository(project)) {
+      for (Ref r : refs) {
+        logs.putAll(r.getName(), log(repo, r.getName()));
+      }
+    }
+    return logs.build();
+  }
+
   private static void assertLogEqualExceptTrees(
       ImmutableList<RevCommit> actualLog, ImmutableList<RevCommit> expectedLog) {
     assertThat(actualLog).hasSize(expectedLog.size());
@@ -522,9 +537,63 @@ public class CommentJsonMigratorTest extends AbstractChangeNotesTest {
   }
 
   private void assertNoDifferences(ChangeNotes actual, ChangeNotes expected) throws Exception {
-    assertThat(
-            ChangeBundle.fromNotes(commentsUtil, actual)
-                .differencesFrom(ChangeBundle.fromNotes(commentsUtil, expected)))
-        .isEmpty();
+    checkArgument(
+        actual.getChangeId().equals(expected.getChangeId()),
+        "must be same change: %s != %s",
+        actual.getChangeId(),
+        expected.getChangeId());
+
+    // Parsed comment representations are equal.
+    // TODO(dborowitz): Comparing collections directly would be much easier, but Comment doesn't
+    // have a proper equals; switch to that when the issues with
+    // https://gerrit-review.googlesource.com/c/gerrit/+/207013 are resolved.
+    assertCommentsEqual(
+        commentsUtil.draftByChange(null, actual), commentsUtil.draftByChange(null, expected));
+    assertCommentsEqual(
+        commentsUtil.publishedByChange(null, actual),
+        commentsUtil.publishedByChange(null, expected));
+
+    // Change metadata is equal.
+    assertLogEqualExceptTrees(
+        log(project, actual.getRefName()), log(project, expected.getRefName()));
+
+    // Logs of all draft refs are equal.
+    ImmutableListMultimap<String, RevCommit> actualDraftLogs =
+        logAll(allUsersName, commentsUtil.getDraftRefs(actual.getChangeId()));
+    ImmutableListMultimap<String, RevCommit> expectedDraftLogs =
+        logAll(allUsersName, commentsUtil.getDraftRefs(expected.getChangeId()));
+    assertThat(actualDraftLogs.keySet())
+        .named("draft ref names")
+        .containsExactlyElementsIn(expectedDraftLogs.keySet());
+    for (String refName : actualDraftLogs.keySet()) {
+      assertLogEqualExceptTrees(actualDraftLogs.get(refName), actualDraftLogs.get(refName));
+    }
+  }
+
+  private static void assertCommentsEqual(List<Comment> actualList, List<Comment> expectedList) {
+    ImmutableMap<Comment.Key, Comment> actualMap = byKey(actualList);
+    ImmutableMap<Comment.Key, Comment> expectedMap = byKey(expectedList);
+    assertThat(actualMap.keySet()).isEqualTo(expectedMap.keySet());
+    for (Comment.Key key : actualMap.keySet()) {
+      Comment actual = actualMap.get(key);
+      Comment expected = expectedMap.get(key);
+      assertThat(actual.key).isEqualTo(expected.key);
+      assertThat(actual.lineNbr).isEqualTo(expected.lineNbr);
+      assertThat(actual.author).isEqualTo(expected.author);
+      assertThat(actual.getRealAuthor()).isEqualTo(expected.getRealAuthor());
+      assertThat(actual.writtenOn).isEqualTo(expected.writtenOn);
+      assertThat(actual.side).isEqualTo(expected.side);
+      assertThat(actual.message).isEqualTo(expected.message);
+      assertThat(actual.parentUuid).isEqualTo(expected.parentUuid);
+      assertThat(actual.range).isEqualTo(expected.range);
+      assertThat(actual.tag).isEqualTo(expected.tag);
+      assertThat(actual.revId).isEqualTo(expected.revId);
+      assertThat(actual.serverId).isEqualTo(expected.serverId);
+      assertThat(actual.unresolved).isEqualTo(expected.unresolved);
+    }
+  }
+
+  private static ImmutableMap<Comment.Key, Comment> byKey(List<Comment> comments) {
+    return comments.stream().collect(toImmutableMap(c -> c.key, c -> c));
   }
 }