opendev/gerrit
Code Issues Proposed changes

BanCommitITs: Check remote ref update status

Browse Source 
Without this explicit check, the ref update succeeds, but this is not
immediately obvious from the failure message:

java.lang.AssertionError: Not true that null reference starts with <"contains banned commit">

Now, we see that the ref was updated when it shouldn't have been:

java.lang.AssertionError: Not true that <OK> is equal to <REJECTED_OTHER_REASON>

Change-Id: I3752568966f579617d54fb09b0cdbdeeb5aa4de9
This commit is contained in:
Dave Borowitz
2016-01-06 10:26:28 -05:00
parent dd0f6dea2f
commit bb5c68c4b9
2 changed files with 14 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/BanCommitIT.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.rest.project;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.GitUtil.pushHead;
import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.REJECTED_OTHER_REASON;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -24,7 +25,7 @@ import com.google.gerrit.server.project.BanCommit;
import com.google.gerrit.server.project.BanCommit.BanResultInfo;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.PushResult;
import org.eclipse.jgit.transport.RemoteRefUpdate;
import org.junit.Test;
public class BanCommitIT extends AbstractDaemonTest {
@@ -44,9 +45,11 @@ public class BanCommitIT extends AbstractDaemonTest {
assertThat(info.alreadyBanned).isNull();
assertThat(info.ignored).isNull();
PushResult pushResult = pushHead(testRepo, "refs/heads/master", false);
assertThat(pushResult.getRemoteUpdate("refs/heads/master").getMessage())
.startsWith("contains banned commit");
RemoteRefUpdate u = pushHead(testRepo, "refs/heads/master", false)
.getRemoteUpdate("refs/heads/master");
assertThat(u).isNotNull();
assertThat(u.getStatus()).isEqualTo(REJECTED_OTHER_REASON);
assertThat(u.getMessage()).startsWith("contains banned commit");
}
@Test

11
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/BanCommitIT.java
View File

@@ -17,12 +17,13 @@ package com.google.gerrit.acceptance.ssh;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assert_;
import static com.google.gerrit.acceptance.GitUtil.pushHead;
import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.REJECTED_OTHER_REASON;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.PushResult;
import org.eclipse.jgit.transport.RemoteRefUpdate;
import org.junit.Test;
import java.util.Locale;
@@ -42,8 +43,10 @@ public class BanCommitIT extends AbstractDaemonTest {
.that(sshSession.hasError()).isFalse();
assertThat(response.toLowerCase(Locale.US)).doesNotContain("error");
PushResult pushResult = pushHead(testRepo, "refs/heads/master", false);
assertThat(pushResult.getRemoteUpdate("refs/heads/master").getMessage())
.startsWith("contains banned commit");
RemoteRefUpdate u = pushHead(testRepo, "refs/heads/master", false)
.getRemoteUpdate("refs/heads/master");
assertThat(u).isNotNull();
assertThat(u.getStatus()).isEqualTo(REJECTED_OTHER_REASON);
assertThat(u.getMessage()).startsWith("contains banned commit");
}
}