Distinguish between assertProblems and assertNoProblems
assertProblems(foo) with an implicit empty list can be confusing to
read. You can tell I figured this out, because
GerritPublicKeyCheckerTest, which postdates the other tests, already
used this pattern. Use it consistently everywhere.
Use (String, String...) in method signatures to make invalid use of
assertProblems with no expected problems a compile-time error.
Change-Id: I5621c586ac16dff2a34d30efd1cca8949907316d
diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
index db0bb88..4df9d37 100644
--- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
+++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
@@ -14,7 +14,6 @@
package com.google.gerrit.gpg;
-import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.gpg.GerritPublicKeyChecker.toExtIdKey;
import static com.google.gerrit.gpg.PublicKeyStore.keyToString;
@@ -67,6 +66,7 @@
import org.junit.Test;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.List;
@@ -433,11 +433,13 @@
}
private void assertProblems(CheckResult result, Status expectedStatus,
- String... expectedProblems) throws Exception {
- checkArgument(expectedProblems.length > 0);
+ String first, String... rest) throws Exception {
+ List<String> expectedProblems = new ArrayList<>();
+ expectedProblems.add(first);
+ expectedProblems.addAll(Arrays.asList(rest));
assertThat(result.getStatus()).isEqualTo(expectedStatus);
assertThat(result.getProblems())
- .containsExactly((Object[]) expectedProblems)
+ .containsExactlyElementsIn(expectedProblems)
.inOrder();
}
diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyCheckerTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyCheckerTest.java
index 576ffbd..742bf1a 100644
--- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyCheckerTest.java
+++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PublicKeyCheckerTest.java
@@ -54,10 +54,13 @@
import org.junit.rules.ExpectedException;
import java.text.SimpleDateFormat;
+import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.Iterator;
+import java.util.List;
import java.util.Map;
public class PublicKeyCheckerTest {
@@ -87,7 +90,7 @@
@Test
public void validKey() throws Exception {
- assertProblems(validKeyWithoutExpiration());
+ assertNoProblems(validKeyWithoutExpiration());
}
@Test
@@ -96,10 +99,10 @@
PublicKeyChecker checker = new PublicKeyChecker()
.setStore(store);
- assertProblems(checker, k);
+ assertNoProblems(checker, k);
checker.setEffectiveTime(parseDate("2015-07-10 12:00:00 -0400"));
- assertProblems(checker, k);
+ assertNoProblems(checker, k);
checker.setEffectiveTime(parseDate("2075-07-10 12:00:00 -0400"));
assertProblems(checker, k, "Key is expired");
@@ -138,10 +141,10 @@
save();
PublicKeyChecker checker = newChecker(2, kb, kd);
- assertProblems(checker, ka);
+ assertNoProblems(checker, ka);
assertProblems(checker, kb, "Key is expired");
- assertProblems(checker, kc);
- assertProblems(checker, kd);
+ assertNoProblems(checker, kc);
+ assertNoProblems(checker, kd);
assertProblems(checker, ke, "Key is expired", "No path to a trusted key");
}
@@ -192,7 +195,7 @@
// J trusts I to a depth of 1, so I itself is valid, but I's certification
// of K is not valid.
- assertProblems(checker, ki);
+ assertNoProblems(checker, ki);
assertProblems(checker, kh,
"No path to a trusted key", notTrusted(ki));
}
@@ -212,7 +215,7 @@
save();
// Key no longer specified as revoker.
- assertProblems(kr.getPublicKey());
+ assertNoProblems(kr.getPublicKey());
}
@Test
@@ -266,7 +269,7 @@
PublicKeyChecker checker = new PublicKeyChecker()
.setStore(store)
.setEffectiveTime(parseDate("2010-01-01 12:00:00 -0400"));
- assertProblems(checker, k);
+ assertNoProblems(checker, k);
}
@Test
@@ -277,7 +280,7 @@
save();
PublicKeyChecker checker = new PublicKeyChecker().setStore(store);
- assertProblems(checker, k);
+ assertNoProblems(checker, k);
}
@Test
@@ -293,7 +296,7 @@
// Set time between key creation and revocation.
checker.setEffectiveTime(parseDate("2005-08-01 13:00:00 -0400"));
- assertProblems(checker, k);
+ assertNoProblems(checker, k);
}
private PGPPublicKeyRing removeRevokers(PGPPublicKeyRing kr) {
@@ -342,21 +345,38 @@
}
private void assertProblems(PublicKeyChecker checker, TestKey k,
- String... expected) {
+ String first, String... rest) {
CheckResult result = checker.setStore(store)
.check(k.getPublicKey());
- assertEquals(Arrays.asList(expected), result.getProblems());
+ assertEquals(list(first, rest), result.getProblems());
}
- private void assertProblems(TestKey tk, String... expected) throws Exception {
- assertProblems(tk.getPublicKey(), expected);
+ private void assertNoProblems(PublicKeyChecker checker, TestKey k) {
+ CheckResult result = checker.setStore(store)
+ .check(k.getPublicKey());
+ assertEquals(Collections.emptyList(), result.getProblems());
}
- private void assertProblems(PGPPublicKey k, String... expected) throws Exception {
+ private void assertProblems(TestKey tk, String first, String... rest) {
+ assertProblems(tk.getPublicKey(), first, rest);
+ }
+
+ private void assertNoProblems(TestKey tk) {
+ assertNoProblems(tk.getPublicKey());
+ }
+
+ private void assertProblems(PGPPublicKey k, String first, String... rest) {
CheckResult result = new PublicKeyChecker()
.setStore(store)
.check(k);
- assertEquals(Arrays.asList(expected), result.getProblems());
+ assertEquals(list(first, rest), result.getProblems());
+ }
+
+ private void assertNoProblems(PGPPublicKey k) {
+ CheckResult result = new PublicKeyChecker()
+ .setStore(store)
+ .check(k);
+ assertEquals(Collections.emptyList(), result.getProblems());
}
private static String notTrusted(TestKey k) {
@@ -367,4 +387,11 @@
private static Date parseDate(String str) throws Exception {
return new SimpleDateFormat("YYYY-MM-dd HH:mm:ss Z").parse(str);
}
+
+ private static List<String> list(String first, String[] rest) {
+ List<String> all = new ArrayList<>();
+ all.add(first);
+ all.addAll(Arrays.asList(rest));
+ return all;
+ }
}
diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PushCertificateCheckerTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PushCertificateCheckerTest.java
index 9778958..ee07d55 100644
--- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PushCertificateCheckerTest.java
+++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/PushCertificateCheckerTest.java
@@ -49,8 +49,11 @@
import java.io.InputStreamReader;
import java.io.Reader;
import java.text.SimpleDateFormat;
+import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.Date;
+import java.util.List;
public class PushCertificateCheckerTest {
private InMemoryRepository repo;
@@ -98,7 +101,7 @@
public void validCert() throws Exception {
PushCertificate cert =
newSignedCert(validNonce(), validKeyWithoutExpiration());
- assertProblems(cert);
+ assertNoProblems(cert);
}
@Test
@@ -113,7 +116,7 @@
checker = newChecker(false);
PushCertificate cert =
newSignedCert("invalid-nonce", validKeyWithoutExpiration());
- assertProblems(cert);
+ assertNoProblems(cert);
}
@Test
@@ -139,7 +142,7 @@
Date now = new SimpleDateFormat("YYYY-MM-dd HH:mm:ss Z")
.parse("2005-07-10 12:00:00 -0400");
PushCertificate cert = newSignedCert(validNonce(), key3, now);
- assertProblems(cert);
+ assertNoProblems(cert);
}
private String validNonce() {
@@ -193,9 +196,17 @@
return parser.parse(reader);
}
- private void assertProblems(PushCertificate cert, String... expected)
- throws Exception {
+ private void assertProblems(PushCertificate cert, String first,
+ String... rest) throws Exception {
+ List<String> expected = new ArrayList<>();
+ expected.add(first);
+ expected.addAll(Arrays.asList(rest));
CheckResult result = checker.check(cert).getCheckResult();
- assertEquals(Arrays.asList(expected), result.getProblems());
+ assertEquals(expected, result.getProblems());
+ }
+
+ private void assertNoProblems(PushCertificate cert) {
+ CheckResult result = checker.check(cert).getCheckResult();
+ assertEquals(Collections.emptyList(), result.getProblems());
}
}
