From bb5c68c4b94585aa02847002a619f36bf32ec4ce Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Wed, 6 Jan 2016 10:26:28 -0500 Subject: [PATCH] BanCommitITs: Check remote ref update status 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 is equal to Change-Id: I3752568966f579617d54fb09b0cdbdeeb5aa4de9 --- .../gerrit/acceptance/rest/project/BanCommitIT.java | 11 +++++++---- .../com/google/gerrit/acceptance/ssh/BanCommitIT.java | 11 +++++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/BanCommitIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/BanCommitIT.java index 0362f59b19..d5845b2b0d 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/BanCommitIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/BanCommitIT.java @@ -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 diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/BanCommitIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/BanCommitIT.java index 3bef84b1dd..ab2dd81f23 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/BanCommitIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/BanCommitIT.java @@ -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"); } }