From f449ad4ce72c3870edcdb77003f21bb8216d7744 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Fri, 25 Mar 2016 10:03:04 -0400 Subject: [PATCH] NoteDbChecker: Refactor to support checking without rebuilding Change-Id: Idd6a154b25dc7e54bb8105ab8e75b4c36560422c --- .../gerrit/acceptance/GerritServer.java | 3 +- .../server/notedb/ChangeRebuilderIT.java | 4 +- .../google/gerrit/testutil/NoteDbChecker.java | 130 ++++++++++++------ 3 files changed, 91 insertions(+), 46 deletions(-) diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java index b71572fe67..08977f0a14 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritServer.java @@ -285,7 +285,8 @@ public class GerritServer { void stop() throws Exception { try { if (NoteDbMode.get().equals(NoteDbMode.CHECK)) { - testInjector.getInstance(NoteDbChecker.class).checkAllChanges(); + testInjector.getInstance(NoteDbChecker.class) + .rebuildAndCheckAllChanges(); } } finally { daemon.getLifecycleManager().stop(); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java index 3577d3c0aa..d304e4af9f 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java @@ -41,7 +41,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest { PushOneCommit.Result r = createChange(); Change.Id id = r.getPatchSetId().getParentKey(); gApi.changes().id(id.get()).topic(name("a-topic")); - checker.checkChanges(id); + checker.rebuildAndCheckChanges(id); } @Test @@ -49,6 +49,6 @@ public class ChangeRebuilderIT extends AbstractDaemonTest { PushOneCommit.Result r = createChange(); Change.Id id = r.getPatchSetId().getParentKey(); r = amendChange(r.getChangeId()); - checker.checkChanges(id); + checker.rebuildAndCheckChanges(id); } } diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbChecker.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbChecker.java index d86d2659f3..f87ed1e730 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbChecker.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbChecker.java @@ -59,63 +59,107 @@ public class NoteDbChecker { this.plcUtil = plcUtil; } - public void checkAllChanges() throws Exception { - checkChanges( + public void rebuildAndCheckAllChanges() throws Exception { + rebuildAndCheckChanges( Iterables.transform( unwrapDb().changes().all(), ReviewDbUtil.changeIdFunction())); } + public void rebuildAndCheckChanges(Change.Id... changeIds) throws Exception { + rebuildAndCheckChanges(Arrays.asList(changeIds)); + } + + public void rebuildAndCheckChanges(Iterable changeIds) + throws Exception { + ReviewDb db = unwrapDb(); + + List allExpected = readExpected(changeIds); + + boolean oldWrite = notesMigration.writeChanges(); + boolean oldRead = notesMigration.readChanges(); + try { + notesMigration.setWriteChanges(true); + notesMigration.setReadChanges(true); + List msgs = new ArrayList<>(); + for (ChangeBundle expected : allExpected) { + Change c = expected.getChange(); + try { + changeRebuilder.rebuild(db, c.getId()); + } catch (RepositoryNotFoundException e) { + msgs.add("Repository not found for change, cannot convert: " + c); + } + } + + checkActual(allExpected, msgs); + } finally { + notesMigration.setReadChanges(oldRead); + notesMigration.setWriteChanges(oldWrite); + } + } + public void checkChanges(Change.Id... changeIds) throws Exception { checkChanges(Arrays.asList(changeIds)); } public void checkChanges(Iterable changeIds) throws Exception { + checkActual(readExpected(changeIds), new ArrayList()); + } + + private List readExpected(Iterable changeIds) + throws Exception { ReviewDb db = unwrapDb(); + boolean old = notesMigration.readChanges(); + try { + notesMigration.setReadChanges(false); + List sortedIds = + ReviewDbUtil.intKeyOrdering().sortedCopy(changeIds); + List expected = new ArrayList<>(sortedIds.size()); + for (Change.Id id : sortedIds) { + expected.add(ChangeBundle.fromReviewDb(db, id)); + } + return expected; + } finally { + notesMigration.setReadChanges(old); + } + } - notesMigration.setReadChanges(false); - List sortedIds = - ReviewDbUtil.intKeyOrdering().sortedCopy(changeIds); - List allExpected = new ArrayList<>(sortedIds.size()); - for (Change.Id id : sortedIds) { - allExpected.add(ChangeBundle.fromReviewDb(db, id)); - } - - notesMigration.setWriteChanges(true); - notesMigration.setReadChanges(true); - List all = new ArrayList<>(); - for (ChangeBundle expected : allExpected) { - Change c = expected.getChange(); - try { - changeRebuilder.rebuild(db, c.getId()); - } catch (RepositoryNotFoundException e) { - all.add("Repository not found for change, cannot convert: " + c); + private void checkActual(List allExpected, List msgs) + throws Exception { + ReviewDb db = unwrapDb(); + boolean oldRead = notesMigration.readChanges(); + boolean oldWrite = notesMigration.writeChanges(); + try { + notesMigration.setWriteChanges(true); + notesMigration.setReadChanges(true); + for (ChangeBundle expected : allExpected) { + Change c = expected.getChange(); + ChangeBundle actual; + try { + actual = ChangeBundle.fromNotes( + plcUtil, notesFactory.create(db, c.getProject(), c.getId())); + } catch (Throwable t) { + String msg = "Error converting change: " + c; + msgs.add(msg); + log.error(msg, t); + continue; + } + List diff = expected.differencesFrom(actual); + if (!diff.isEmpty()) { + msgs.add("Differences between ReviewDb and NoteDb for " + c + ":"); + msgs.addAll(diff); + msgs.add(""); + } else { + System.err.println( + "NoteDb conversion of change " + c.getId() + " successful"); + } } + } finally { + notesMigration.setReadChanges(oldRead); + notesMigration.setWriteChanges(oldWrite); } - for (ChangeBundle expected : allExpected) { - Change c = expected.getChange(); - ChangeBundle actual; - try { - actual = ChangeBundle.fromNotes( - plcUtil, notesFactory.create(db, c.getProject(), c.getId())); - } catch (Throwable t) { - String msg = "Error converting change: " + c; - all.add(msg); - log.error(msg, t); - continue; - } - List diff = expected.differencesFrom(actual); - if (!diff.isEmpty()) { - all.add("Differences between ReviewDb and NoteDb for " + c + ":"); - all.addAll(diff); - all.add(""); - } else { - System.err.println( - "NoteDb conversion of change " + c.getId() + " successful"); - } - } - if (!all.isEmpty()) { - throw new AssertionError(Joiner.on('\n').join(all)); + if (!msgs.isEmpty()) { + throw new AssertionError(Joiner.on('\n').join(msgs)); } }