Improve ref-update validators tests
Make sure that ref-update validators tests are more representative
of a real scenario, having the local ref-update to actually perform the
operation or, when used as a mock, simulate the before and after the update
local ref status.
Change-Id: I1c05dc3d9521cd2579ca0d3d8ec21024a26b47a5
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidatorTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidatorTest.java
index e5d3409..85f95ec 100644
--- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidatorTest.java
+++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/BatchRefUpdateValidatorTest.java
@@ -17,14 +17,15 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.util.Collections.singletonList;
+import static org.eclipse.jgit.transport.ReceiveCommand.Type.CREATE;
import static org.eclipse.jgit.transport.ReceiveCommand.Type.UPDATE;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -37,7 +38,6 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.metrics.DisabledMetricMaker;
import java.io.IOException;
-import java.util.Arrays;
import java.util.List;
import org.eclipse.jgit.internal.storage.file.RefDirectory;
import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase;
@@ -47,6 +47,7 @@
import org.eclipse.jgit.lib.NullProgressMonitor;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
@@ -62,6 +63,9 @@
@RunWith(MockitoJUnitRunner.class)
public class BatchRefUpdateValidatorTest extends LocalDiskRepositoryTestCase implements RefFixture {
+ private static final String A_REF_NAME_1 = "refs/heads/a-ref-name-1";
+ private static final String A_REF_NAME_2 = "refs/heads/a-ref-name-2";
+
@Rule public TestName nameRule = new TestName();
private Repository diskRepo;
@@ -79,7 +83,7 @@
@Before
public void setup() throws Exception {
super.setUp();
- doReturn(false).when(sharedRefDatabase).isUpToDate(any(), any());
+ lenient().doReturn(false).when(sharedRefDatabase).isUpToDate(any(), any());
when(projectsFilter.matches(anyString())).thenReturn(true);
gitRepoSetup();
}
@@ -89,27 +93,26 @@
refdir = (RefDirectory) diskRepo.getRefDatabase();
repo = new TestRepository<>(diskRepo);
A = repo.commit().create();
+ updateRef(A_REF_NAME_1, A.getId());
+ updateRef(A_REF_NAME_2, A.getId());
B = repo.commit(repo.getRevWalk().parseCommit(A));
}
@Test
public void shouldUpdateSharedRefDbForAllRefUpdates() throws IOException {
- String REF_NAME_A = "refs/changes/01/1/1";
- String REF_NAME_B = "refs/changes/02/1/1";
BatchRefUpdate batchRefUpdate =
newBatchUpdate(
List.of(
- new ReceiveCommand(A, B, REF_NAME_A, UPDATE),
- new ReceiveCommand(A, B, REF_NAME_B, UPDATE)));
+ new ReceiveCommand(A, B, A_REF_NAME_1, UPDATE),
+ new ReceiveCommand(A, B, A_REF_NAME_2, UPDATE)));
BatchRefUpdateValidator batchRefUpdateValidator =
- getRefValidatorForEnforcement(A_TEST_PROJECT_NAME, tmpRefEnforcement);
+ getRefValidatorForEnforcement(tmpRefEnforcement);
doReturn(SharedRefEnforcement.EnforcePolicy.REQUIRED)
.when(batchRefUpdateValidator.refEnforcement)
- .getPolicy(A_TEST_PROJECT_NAME, REF_NAME_A);
+ .getPolicy(A_TEST_PROJECT_NAME, A_REF_NAME_1);
doReturn(true).when(sharedRefDatabase).isUpToDate(any(), any());
-
doReturn(true).when(sharedRefDatabase).compareAndPut(any(), any(), any());
batchRefUpdateValidator.executeBatchUpdateWithValidation(
@@ -127,72 +130,67 @@
public void immutableChangeShouldNotBeWrittenIntoSharedRefDb() throws Exception {
String AN_IMMUTABLE_REF = "refs/changes/01/1/1";
- List<ReceiveCommand> cmds = Arrays.asList(new ReceiveCommand(A, B, AN_IMMUTABLE_REF, UPDATE));
+ ReceiveCommand receiveCommand =
+ new ReceiveCommand(ObjectId.zeroId(), B, AN_IMMUTABLE_REF, CREATE);
+ BatchRefUpdate batchRefUpdate = newBatchUpdate(List.of(receiveCommand));
- BatchRefUpdate batchRefUpdate = newBatchUpdate(cmds);
- BatchRefUpdateValidator BatchRefUpdateValidator = newDefaultValidator(A_TEST_PROJECT_NAME);
-
+ BatchRefUpdateValidator BatchRefUpdateValidator = newDefaultValidator();
BatchRefUpdateValidator.executeBatchUpdateWithValidation(
batchRefUpdate, () -> execute(batchRefUpdate), this::defaultRollback);
verify(sharedRefDatabase, never())
.compareAndPut(any(Project.NameKey.class), any(Ref.class), any(ObjectId.class));
+ assertThatReceiveCommandIsSuccessful(receiveCommand);
}
@Test
public void compareAndPutShouldAlwaysIngoreAlwaysDraftCommentsEvenOutOfOrder() throws Exception {
String DRAFT_COMMENT = "refs/draft-comments/56/450756/1013728";
- List<ReceiveCommand> cmds = Arrays.asList(new ReceiveCommand(A, B, DRAFT_COMMENT, UPDATE));
+ ReceiveCommand receiveCommand = new ReceiveCommand(ObjectId.zeroId(), B, DRAFT_COMMENT, CREATE);
- BatchRefUpdate batchRefUpdate = newBatchUpdate(cmds);
- BatchRefUpdateValidator BatchRefUpdateValidator = newDefaultValidator(A_TEST_PROJECT_NAME);
+ BatchRefUpdate batchRefUpdate = newBatchUpdate(List.of(receiveCommand));
+ BatchRefUpdateValidator BatchRefUpdateValidator = newDefaultValidator();
BatchRefUpdateValidator.executeBatchUpdateWithValidation(
batchRefUpdate, () -> execute(batchRefUpdate), this::defaultRollback);
verify(sharedRefDatabase, never())
.compareAndPut(A_TEST_PROJECT_NAME_KEY, newRef(DRAFT_COMMENT, A.getId()), B.getId());
+ assertThatReceiveCommandIsSuccessful(receiveCommand);
}
@Test
public void validationShouldFailWhenLocalRefDbIsOutOfSync() throws Exception {
- String AN_OUT_OF_SYNC_REF = "refs/changes/01/1/1";
- BatchRefUpdate batchRefUpdate =
- newBatchUpdate(singletonList(new ReceiveCommand(A, B, AN_OUT_OF_SYNC_REF, UPDATE)));
+ ReceiveCommand receiveCommand = new ReceiveCommand(A, B, A_REF_NAME_1, UPDATE);
+ BatchRefUpdate batchRefUpdate = newBatchUpdate(singletonList(receiveCommand));
BatchRefUpdateValidator batchRefUpdateValidator =
- getRefValidatorForEnforcement(A_TEST_PROJECT_NAME, tmpRefEnforcement);
+ getRefValidatorForEnforcement(tmpRefEnforcement);
doReturn(SharedRefEnforcement.EnforcePolicy.REQUIRED)
.when(batchRefUpdateValidator.refEnforcement)
- .getPolicy(A_TEST_PROJECT_NAME, AN_OUT_OF_SYNC_REF);
- lenient()
- .doReturn(false)
- .when(sharedRefDatabase)
- .isUpToDate(A_TEST_PROJECT_NAME_KEY, newRef(AN_OUT_OF_SYNC_REF, AN_OBJECT_ID_1));
+ .getPolicy(A_TEST_PROJECT_NAME, A_REF_NAME_1);
+ doReturn(false).when(sharedRefDatabase).isUpToDate(eq(A_TEST_PROJECT_NAME_KEY), any());
+ doReturn(true).when(sharedRefDatabase).exists(eq(A_TEST_PROJECT_NAME_KEY), any());
batchRefUpdateValidator.executeBatchUpdateWithValidation(
batchRefUpdate, () -> execute(batchRefUpdate), rollbackFunction);
verify(rollbackFunction, never()).invoke(any());
- final List<ReceiveCommand> commands = batchRefUpdate.getCommands();
- assertThat(commands.size()).isEqualTo(1);
- commands.forEach(
- (command) -> assertThat(command.getResult()).isEqualTo(ReceiveCommand.Result.LOCK_FAILURE));
+ assertThat(receiveCommand.getResult()).isEqualTo(Result.LOCK_FAILURE);
}
@Test
public void shouldRollbackWhenSharedRefUpdateCompareAndPutThrowsUncaughtThrowable()
throws Exception {
- String REF_NAME = "refs/changes/01/1/meta";
- BatchRefUpdate batchRefUpdate =
- newBatchUpdate(singletonList(new ReceiveCommand(A, B, REF_NAME, UPDATE)));
+ ReceiveCommand receiveCommand = new ReceiveCommand(A, B, A_REF_NAME_1, UPDATE);
+ BatchRefUpdate batchRefUpdate = newBatchUpdate(singletonList(receiveCommand));
BatchRefUpdateValidator batchRefUpdateValidator =
- getRefValidatorForEnforcement(A_TEST_PROJECT_NAME, tmpRefEnforcement);
+ getRefValidatorForEnforcement(tmpRefEnforcement);
doReturn(SharedRefEnforcement.EnforcePolicy.REQUIRED)
.when(batchRefUpdateValidator.refEnforcement)
- .getPolicy(A_TEST_PROJECT_NAME, REF_NAME);
+ .getPolicy(A_TEST_PROJECT_NAME, A_REF_NAME_1);
doReturn(true).when(sharedRefDatabase).isUpToDate(any(), any());
doThrow(TestError.class).when(sharedRefDatabase).compareAndPut(any(), any(), any());
@@ -204,23 +202,19 @@
batchRefUpdate, () -> execute(batchRefUpdate), rollbackFunction));
verify(rollbackFunction).invoke(any());
- List<ReceiveCommand> commands = batchRefUpdate.getCommands();
- assertThat(commands.size()).isEqualTo(1);
- commands.forEach(
- (command) -> assertThat(command.getResult()).isEqualTo(ReceiveCommand.Result.LOCK_FAILURE));
+ assertThat(receiveCommand.getResult()).isEqualTo(ReceiveCommand.Result.LOCK_FAILURE);
}
@Test
public void shouldRollbackRefUpdateWhenRefDbIsNotUpdated() throws Exception {
- String REF_NAME = "refs/changes/01/1/meta";
- BatchRefUpdate batchRefUpdate =
- newBatchUpdate(singletonList(new ReceiveCommand(A, B, REF_NAME, UPDATE)));
+ ReceiveCommand receiveCommand = new ReceiveCommand(A, B, A_REF_NAME_1, UPDATE);
+ BatchRefUpdate batchRefUpdate = newBatchUpdate(singletonList(receiveCommand));
BatchRefUpdateValidator batchRefUpdateValidator =
- getRefValidatorForEnforcement(A_TEST_PROJECT_NAME, tmpRefEnforcement);
+ getRefValidatorForEnforcement(tmpRefEnforcement);
doReturn(SharedRefEnforcement.EnforcePolicy.REQUIRED)
.when(batchRefUpdateValidator.refEnforcement)
- .getPolicy(A_TEST_PROJECT_NAME, REF_NAME);
+ .getPolicy(A_TEST_PROJECT_NAME, A_REF_NAME_1);
doReturn(true).when(sharedRefDatabase).isUpToDate(any(), any());
@@ -232,23 +226,18 @@
batchRefUpdateValidator.executeBatchUpdateWithValidation(
batchRefUpdate, () -> execute(batchRefUpdate), rollbackFunction);
- verify(rollbackFunction, times(1)).invoke(any());
-
- final List<ReceiveCommand> commands = batchRefUpdate.getCommands();
- assertThat(commands.size()).isEqualTo(1);
- commands.forEach(
- (command) -> assertThat(command.getResult()).isEqualTo(ReceiveCommand.Result.LOCK_FAILURE));
+ verify(rollbackFunction).invoke(any());
+ assertThat(receiveCommand.getResult()).isEqualTo(ReceiveCommand.Result.LOCK_FAILURE);
}
@Test
public void shouldNotUpdateSharedRefDbWhenProjectIsLocal() throws Exception {
when(projectsFilter.matches(anyString())).thenReturn(false);
- String AN_OUT_OF_SYNC_REF = "refs/changes/01/1/1";
BatchRefUpdate batchRefUpdate =
- newBatchUpdate(singletonList(new ReceiveCommand(A, B, AN_OUT_OF_SYNC_REF, UPDATE)));
+ newBatchUpdate(singletonList(new ReceiveCommand(A, B, A_REF_NAME_1, UPDATE)));
BatchRefUpdateValidator batchRefUpdateValidator =
- getRefValidatorForEnforcement(A_TEST_PROJECT_NAME, tmpRefEnforcement);
+ getRefValidatorForEnforcement(tmpRefEnforcement);
batchRefUpdateValidator.executeBatchUpdateWithValidation(
batchRefUpdate, () -> execute(batchRefUpdate), this::defaultRollback);
@@ -257,12 +246,22 @@
.compareAndPut(any(Project.NameKey.class), any(Ref.class), any(ObjectId.class));
}
- private BatchRefUpdateValidator newDefaultValidator(String projectName) {
- return getRefValidatorForEnforcement(projectName, new DefaultSharedRefEnforcement());
+ private void updateRef(String refName, ObjectId sha1) throws IOException {
+ RefUpdate refUpdate = refdir.newUpdate(refName, false);
+ refUpdate.setNewObjectId(sha1);
+ refUpdate.update();
+ }
+
+ private static void assertThatReceiveCommandIsSuccessful(ReceiveCommand receiveCommand) {
+ assertThat(receiveCommand.getResult()).isEqualTo(Result.OK);
+ }
+
+ private BatchRefUpdateValidator newDefaultValidator() {
+ return getRefValidatorForEnforcement(new DefaultSharedRefEnforcement());
}
private BatchRefUpdateValidator getRefValidatorForEnforcement(
- String projectName, SharedRefEnforcement sharedRefEnforcement) {
+ SharedRefEnforcement sharedRefEnforcement) {
return new BatchRefUpdateValidator(
sharedRefDatabase,
new ValidationMetrics(
@@ -270,22 +269,21 @@
sharedRefEnforcement,
new DummyLockWrapper(),
projectsFilter,
- projectName,
+ RefFixture.A_TEST_PROJECT_NAME,
diskRepo.getRefDatabase(),
ImmutableSet.of());
}
- private Void execute(BatchRefUpdate u) throws IOException {
+ private void execute(BatchRefUpdate u) throws IOException {
try (RevWalk rw = new RevWalk(diskRepo)) {
u.execute(rw, NullProgressMonitor.INSTANCE);
}
- return null;
}
private BatchRefUpdate newBatchUpdate(List<ReceiveCommand> cmds) {
BatchRefUpdate u = refdir.newBatchUpdate();
u.addCommand(cmds);
- cmds.forEach(c -> c.setResult(Result.OK));
+ cmds.forEach(c -> c.setResult(Result.NOT_ATTEMPTED));
return u;
}
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/ProjectDeletedSharedDbCleanupTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/ProjectDeletedSharedDbCleanupTest.java
index cf383bf..ba54cb9 100644
--- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/ProjectDeletedSharedDbCleanupTest.java
+++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/ProjectDeletedSharedDbCleanupTest.java
@@ -14,6 +14,8 @@
package com.gerritforge.gerrit.globalrefdb.validation;
+import static org.mockito.Mockito.verify;
+
import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.RefFixture;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.events.ProjectDeletedListener;
@@ -22,7 +24,6 @@
import org.junit.rules.TestName;
import org.junit.runner.RunWith;
import org.mockito.Mock;
-import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;
@RunWith(MockitoJUnitRunner.class)
@@ -53,6 +54,6 @@
projectDeletedSharedDbCleanup.onProjectDeleted(event);
- Mockito.verify(sharedRefDatabase, Mockito.times(1)).remove(A_TEST_PROJECT_NAME_KEY);
+ verify(sharedRefDatabase).remove(A_TEST_PROJECT_NAME_KEY);
}
}
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidatorTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidatorTest.java
index 3c63d13..b99749f 100644
--- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidatorTest.java
+++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/RefUpdateValidatorTest.java
@@ -20,7 +20,6 @@
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -30,7 +29,9 @@
import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.RefFixture;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Project;
+import java.io.IOException;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectIdRef;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.RefUpdate;
@@ -117,7 +118,8 @@
.compareAndPut(A_TEST_PROJECT_NAME_KEY, localRef, newUpdateRef.getObjectId());
Result result =
- refUpdateValidator.executeRefUpdate(refUpdate, () -> Result.NEW, this::defaultRollback);
+ refUpdateValidator.executeRefUpdate(
+ refUpdate, () -> doLocalRefUpdate(refName), this::defaultRollback);
assertThat(result).isEqualTo(Result.NEW);
}
@@ -190,13 +192,15 @@
.compareAndPut(any(Project.NameKey.class), any(Ref.class), any(ObjectId.class));
doReturn(false)
.when(sharedRefDb)
- .compareAndPut(A_TEST_PROJECT_NAME_KEY, localRef, newUpdateRef.getObjectId());
+ .compareAndPut(A_TEST_PROJECT_NAME_KEY, localRef, AN_OBJECT_ID_2);
doReturn(lock).when(sharedRefDb).lockRef(any(), anyString());
Result result =
- refUpdateValidator.executeRefUpdate(refUpdate, () -> Result.NEW, rollbackFunction);
+ refUpdateValidator.executeRefUpdate(
+ refUpdate, () -> doLocalRefUpdate(refName), rollbackFunction);
- verify(rollbackFunction, times(1)).invoke(any());
+ verify(sharedRefDb).compareAndPut(A_TEST_PROJECT_NAME_KEY, localRef, AN_OBJECT_ID_2);
+ verify(rollbackFunction).invoke(any());
assertThat(result).isEqualTo(Result.LOCK_FAILURE);
}
@@ -230,9 +234,10 @@
when(rollbackFunction.invoke(any())).thenReturn(Result.LOCK_FAILURE);
Result result =
- refUpdateValidator.executeRefUpdate(refUpdate, () -> Result.NEW, rollbackFunction);
+ refUpdateValidator.executeRefUpdate(
+ refUpdate, () -> doLocalRefUpdate(localRef.getName()), rollbackFunction);
- verify(rollbackFunction, times(1)).invoke(any());
+ verify(rollbackFunction).invoke(any());
}
@Test
@@ -249,6 +254,14 @@
return Result.NO_CHANGE;
}
+ private Result doLocalRefUpdate(String refName) throws IOException {
+ lenient()
+ .doReturn(new ObjectIdRef.Unpeeled(Ref.Storage.LOOSE, refName, AN_OBJECT_ID_2))
+ .when(localRefDb)
+ .findRef(refName);
+ return Result.NEW;
+ }
+
private RefUpdateValidator newRefUpdateValidator(SharedRefDatabaseWrapper refDbWrapper) {
return new RefUpdateValidator(
refDbWrapper,
diff --git a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbBatchRefUpdateTest.java b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbBatchRefUpdateTest.java
index ca3d57b..26761f1 100644
--- a/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbBatchRefUpdateTest.java
+++ b/src/test/java/com/gerritforge/gerrit/globalrefdb/validation/SharedRefDbBatchRefUpdateTest.java
@@ -22,6 +22,7 @@
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;
@@ -30,6 +31,7 @@
import com.gerritforge.gerrit.globalrefdb.validation.dfsrefdb.RefFixture;
import com.google.common.collect.ImmutableSet;
import java.io.IOException;
+import java.util.List;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectIdRef;
@@ -108,8 +110,13 @@
.thenReturn(asList(receiveCommandBeforeExecution))
.thenReturn(asList(successReceiveCommandAfterExecution));
- doReturn(oldRef).when(refDatabase).getRef(A_TEST_REF_NAME);
- doReturn(oldRef).when(refDatabase).exactRef(A_TEST_REF_NAME);
+ // Only needed to prevent NPEs should a rollback happen on the global-refdb updates
+ // and not supposed to be used if all tests are successful.
+ lenient().when(batchRefUpdate.addCommand(any(List.class))).thenReturn(batchRefUpdate);
+
+ lenient().when(refDatabase.getRef(A_TEST_REF_NAME)).thenReturn(oldRef).thenReturn(newRef);
+ lenient().when(refDatabase.exactRef(A_TEST_REF_NAME)).thenReturn(oldRef).thenReturn(newRef);
+ lenient().when(refDatabase.findRef(A_TEST_REF_NAME)).thenReturn(oldRef).thenReturn(newRef);
sharedRefDbRefUpdate = getSharedRefDbBatchRefUpdateWithDefaultPolicyEnforcement();
