AbstractPushForReview: Add tests for pushing with skip-validation option
The option to skip commit validation was added in change I80ad47852,
with the name `bypass-review`, which was later renamed in I012e1ea42 to
`skip-validation`. In the former change it was noted in a post-submit
review that no tests were included.
Add test coverage for usage of the skip-validation option, i.e. the
following use cases:
1. Commit validator is invoked on a normal push.
2. Commit validator is not invoked when using the skip-validation option.
3. Push is rejected when using the skip-validation option without having
the necessary permissions granted. The push is rejected before the
validator is invoked.
Change-Id: Iac2f1a83ad755d1b29b964b3e42d735379965a60
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 420fc04..27065b3 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -65,6 +65,8 @@
import com.google.gerrit.extensions.common.EditInfoSubject;
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
@@ -72,16 +74,22 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.ChangeMessagesUtil;
+import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.git.receive.ReceiveConstants;
+import com.google.gerrit.server.git.validators.CommitValidationException;
+import com.google.gerrit.server.git.validators.CommitValidationListener;
+import com.google.gerrit.server.git.validators.CommitValidationMessage;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.mail.Address;
import com.google.gerrit.server.project.Util;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.testutil.FakeEmailSender.Message;
import com.google.gerrit.testutil.TestTimeUtil;
+import com.google.inject.Inject;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.Comparator;
import java.util.EnumSet;
import java.util.HashMap;
@@ -89,6 +97,7 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
import java.util.regex.Pattern;
import java.util.stream.Stream;
import org.eclipse.jgit.api.errors.GitAPIException;
@@ -117,6 +126,8 @@
private LabelType patchSetLock;
+ @Inject private DynamicSet<CommitValidationListener> commitValidators;
+
@BeforeClass
public static void setTimeForTesting() {
TestTimeUtil.resetWithClockStep(1, SECONDS);
@@ -1952,6 +1963,57 @@
.isEqualTo(Iterables.getLast(commits).name());
}
+ private static class TestValidator implements CommitValidationListener {
+ private final AtomicInteger count = new AtomicInteger();
+
+ @Override
+ public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
+ throws CommitValidationException {
+ count.incrementAndGet();
+ return Collections.emptyList();
+ }
+
+ public int count() {
+ return count.get();
+ }
+ }
+
+ @Test
+ public void skipValidation() throws Exception {
+ String master = "refs/heads/master";
+ TestValidator validator = new TestValidator();
+ RegistrationHandle handle = commitValidators.add(validator);
+
+ try {
+ // Validation listener is called on normal push
+ PushOneCommit push =
+ pushFactory.create(db, admin.getIdent(), testRepo, "change1", "a.txt", "content");
+ PushOneCommit.Result r = push.to(master);
+ r.assertOkStatus();
+ assertThat(validator.count()).isEqualTo(1);
+
+ // Push is rejected and validation listener is not called when not allowed
+ // to use skip option
+ PushOneCommit push2 =
+ pushFactory.create(db, admin.getIdent(), testRepo, "change2", "b.txt", "content");
+ push2.setPushOptions(ImmutableList.of(PUSH_OPTION_SKIP_VALIDATION));
+ r = push2.to(master);
+ r.assertErrorStatus("skip validation not permitted for " + master);
+ assertThat(validator.count()).isEqualTo(1);
+
+ // Validation listener is not called when skip option is used
+ grantSkipValidation(project, master, SystemGroupBackend.REGISTERED_USERS);
+ PushOneCommit push3 =
+ pushFactory.create(db, admin.getIdent(), testRepo, "change2", "b.txt", "content");
+ push3.setPushOptions(ImmutableList.of(PUSH_OPTION_SKIP_VALIDATION));
+ r = push3.to(master);
+ r.assertOkStatus();
+ assertThat(validator.count()).isEqualTo(1);
+ } finally {
+ handle.remove();
+ }
+ }
+
@Test
public void pushToPublishMagicBranchIsAllowed() throws Exception {
// Push to "refs/publish/*" will be a synonym of "refs/for/*".
