Apply a bunch of IDE suggestions
Various code cleanups, spelling fixes, etc. This also converts the test
classes to use JUnit 4.
Change-Id: I0955168eeba59d59691968e4ca172c0d33f6a275
diff --git a/src/main/java/com/googlesource/gerrit/plugins/depends/on/ChangeMessageStore.java b/src/main/java/com/googlesource/gerrit/plugins/depends/on/ChangeMessageStore.java
index 6633132..227381a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/depends/on/ChangeMessageStore.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/depends/on/ChangeMessageStore.java
@@ -40,18 +40,18 @@
import com.googlesource.gerrit.plugins.depends.on.formats.Comment;
import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
-import java.util.stream.Collectors;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class ChangeMessageStore implements DependencyResolver {
private static final Logger log = LoggerFactory.getLogger(ChangeMessageStore.class);
- public static class DependsOnByChangeCache extends HashMap {}
+ public static class DependsOnByChangeCache extends HashMap<Change.Id, List<DependsOn>> {}
protected static final PerThreadCache.Key<DependsOnByChangeCache> DEPENDS_ON_BY_CHANGE_CACHE_KEY =
PerThreadCache.Key.create(DependsOnByChangeCache.class);
@@ -88,17 +88,17 @@
/**
* Load the current DependsOn from the DB for a specific change. "Current" is defined as the last
- * Depends-on defined. Older Depends-ons are assumed to be overriden by the last one. If the last
+ * Depends-on defined. Older Depends-ons are assumed to be overridden by the last one. If the last
* Depends-on is blank, it deletes any previous dependencies.
*
* <p>return empty set means no dependencies found.
*/
public Set<DependsOn> load(Change.Id cid) throws StorageException {
- return loadWithOrder(cid).stream().collect(Collectors.toSet());
+ return new HashSet<>(loadWithOrder(cid));
}
public Set<DependsOn> load(ChangeNotes changeNotes) throws StorageException {
- return loadWithOrder(changeNotes).stream().collect(Collectors.toSet());
+ return new HashSet<>(loadWithOrder(changeNotes));
}
public List<DependsOn> loadWithOrder(Change.Id cid) throws StorageException {
@@ -168,7 +168,7 @@
throws InvalidChangeOperationException, StorageException {
StringBuilder comment = new StringBuilder();
if (message != null) {
- comment.append(message + "\n\n");
+ comment.append(message).append("\n\n");
}
comment.append(Comment.getMessages(deps));
ReviewInput review = new ReviewInput();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/depends/on/CoreListener.java b/src/main/java/com/googlesource/gerrit/plugins/depends/on/CoreListener.java
index 74cfd25..95fa930 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/depends/on/CoreListener.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/depends/on/CoreListener.java
@@ -42,8 +42,7 @@
@Override
public void onEvent(Event event) {
- if (event instanceof PatchSetCreatedEvent) {
- PatchSetCreatedEvent patchSetCreatedEvent = (PatchSetCreatedEvent) event;
+ if (event instanceof PatchSetCreatedEvent patchSetCreatedEvent) {
ChangeAttribute change = patchSetCreatedEvent.change.get();
if (change.cherryPickOfChange != null && patchSetCreatedEvent.patchSet.get().number == 1) {
try {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/depends/on/DependsOn.java b/src/main/java/com/googlesource/gerrit/plugins/depends/on/DependsOn.java
index a27ad5e..00cdb0f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/depends/on/DependsOn.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/depends/on/DependsOn.java
@@ -38,13 +38,8 @@
public abstract Change.Key key();
public static DependsOn create(String change) {
- Optional<Id> id = null;
- Change.Key key = null;
- id = Change.Id.tryParse(change);
- if (!id.isPresent()) {
- return create(Change.Key.parse(change));
- }
- return create(id.get(), key);
+ Optional<Id> id = Change.Id.tryParse(change);
+ return id.map(cid -> create(cid, null)).orElseGet(() -> create(Change.Key.parse(change)));
}
public static DependsOn create(Change.Key key) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/depends/on/DependsOnAttributeFactory.java b/src/main/java/com/googlesource/gerrit/plugins/depends/on/DependsOnAttributeFactory.java
index 5688e5a..5809aa6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/depends/on/DependsOnAttributeFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/depends/on/DependsOnAttributeFactory.java
@@ -52,7 +52,7 @@
for (ChangeData changeData : cds) {
try {
List<DependsOn> dependsOns = changeMessageStore.loadWithOrder(changeData.getId());
- if (dependsOns.size() > 0) {
+ if (!dependsOns.isEmpty()) {
DependsOnPluginAttributes dependsOnPluginAttributes = new DependsOnPluginAttributes();
dependsOnPluginAttributes.addDependsOns(dependsOns);
dependsOnAttributesByChange.put(changeData.getId(), dependsOnPluginAttributes);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/depends/on/DependsOnOperator.java b/src/main/java/com/googlesource/gerrit/plugins/depends/on/DependsOnOperator.java
index 871fb46..9a8b1fb 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/depends/on/DependsOnOperator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/depends/on/DependsOnOperator.java
@@ -31,7 +31,7 @@
public static final String FIELD = "has";
public class DependsOnPredicate extends PostFilterPredicate<ChangeData> {
- private Predicate<ChangeData> subQuery;
+ private final Predicate<ChangeData> subQuery;
public DependsOnPredicate(Predicate<ChangeData> subQuery) {
super(FIELD, subQuery.toString());
@@ -49,7 +49,7 @@
List<ChangeNotes> changeNotes =
changeNotesFactory.createUsingIndexLookup(
dependOns.stream()
- .filter(d -> d.isResolved())
+ .filter(DependsOn::isResolved)
.map(DependsOn::id)
.collect(Collectors.toList()));
return changeNotes.stream()
diff --git a/src/main/java/com/googlesource/gerrit/plugins/depends/on/InDependsOnOperator.java b/src/main/java/com/googlesource/gerrit/plugins/depends/on/InDependsOnOperator.java
index df52117..1d2c3ec 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/depends/on/InDependsOnOperator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/depends/on/InDependsOnOperator.java
@@ -27,13 +27,9 @@
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
@Singleton
public class InDependsOnOperator implements ChangeOperatorFactory {
- private static final Logger log = LoggerFactory.getLogger(InDependsOnOperator.class);
-
public static final String FIELD = "in";
protected final ChangeMessageStore changeMessageStore;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/depends/on/Propagator.java b/src/main/java/com/googlesource/gerrit/plugins/depends/on/Propagator.java
index a6a241c..2ffdb6c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/depends/on/Propagator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/depends/on/Propagator.java
@@ -41,7 +41,7 @@
Set<DependsOn> deps = changeMessageStore.load(srcChange);
if (!deps.isEmpty()) {
- Set<DependsOn> keyDeps = new HashSet<DependsOn>(deps.size());
+ Set<DependsOn> keyDeps = new HashSet<>(deps.size());
for (DependsOn dep : deps) {
keyDeps.add(DependsOn.create(loadChangeKey(dep)));
}
@@ -68,7 +68,7 @@
// as a Key, it will inherently carry along random strings.
// This is thus used to carry unidentified dependencies to
// propagated changes (the assumption is that the user needs
- // to fix it). Piggy back here on this idea for change-nums
+ // to fix it). Piggyback here on this idea for change-nums
// that lead to unidentified changes (treat them as bad
// strings, and throw them into the change-id to get
// propagated).
diff --git a/src/main/java/com/googlesource/gerrit/plugins/depends/on/formats/Comment.java b/src/main/java/com/googlesource/gerrit/plugins/depends/on/formats/Comment.java
index 8b35bcd..5faefa0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/depends/on/formats/Comment.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/depends/on/formats/Comment.java
@@ -41,7 +41,7 @@
return Optional.of(
Arrays.stream(changes.split("\\s+", -1)) // -> ["1234", "4444"]
.filter(c -> !c.isEmpty())
- .map(c -> DependsOn.create(c))
+ .map(DependsOn::create)
.collect(Collectors.toList()));
}
}
@@ -60,7 +60,7 @@
public static StringBuilder getMessages(Set<DependsOn> dependsons) {
StringBuilder dependencies = new StringBuilder("Depends-on:");
for (DependsOn dep : dependsons) {
- dependencies.append(" " + getMessage(dep));
+ dependencies.append(" ").append(getMessage(dep));
}
return dependencies;
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/depends/on/DependsOnParsingTest.java b/src/test/java/com/googlesource/gerrit/plugins/depends/on/DependsOnParsingTest.java
index 491d572..371d020 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/depends/on/DependsOnParsingTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/depends/on/DependsOnParsingTest.java
@@ -14,14 +14,20 @@
package com.googlesource.gerrit.plugins.depends.on;
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
+
import com.google.gerrit.testing.InMemoryModule;
import com.googlesource.gerrit.plugins.depends.on.formats.Comment;
import java.util.List;
import java.util.Optional;
-import junit.framework.TestCase;
+import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
-public class DependsOnParsingTest extends TestCase {
+@RunWith(JUnit4.class)
+public class DependsOnParsingTest {
public static final String NUM = "1234";
public static final String NUM2 = "345";
public static final String KEY = "Iabcdef7890abcdef7890abcdef7890abcdef7890";
@@ -30,175 +36,184 @@
public static DependsOn NUM_DEP;
public static DependsOn KEY_DEP;
- @Override
- protected void setUp() throws Exception {
- super.setUp();
+ @Before
+ public void setUp() {
new InMemoryModule().inject(this); // Needed to setup KeyUtil.ENCODER_IMPL
NUM_DEP = DependsOn.create(NUM);
KEY_DEP = DependsOn.create(KEY);
}
- @Override
- protected void tearDown() throws Exception {
- super.tearDown();
- }
-
@Test
public void testCommentMessageChangeNum() {
- assertTrue(NUM.equals(Comment.getMessage(NUM_DEP)));
+ assertThat(Comment.getMessage(NUM_DEP)).isEqualTo(NUM);
}
@Test
public void testCommentMessageChangeKey() {
- assertTrue(KEY.equals(Comment.getMessage(KEY_DEP)));
+ assertThat(Comment.getMessage(KEY_DEP)).isEqualTo(KEY);
}
@Test
public void testParseNum() {
DependsOn dep = DependsOn.create(NUM);
- assertTrue(NUM.equals("" + dep.id().get()));
+ assertThat("" + dep.id().get()).isEqualTo(NUM);
}
@Test
public void testParseKey() {
DependsOn dep = DependsOn.create(KEY);
- assertTrue(KEY.equals(dep.key().get()));
+ assertThat(dep.key().get()).isEqualTo(KEY);
}
@Test
public void testParseNoneComment() {
String comment = "My Very Educated Mother Just Served Us Nothing!";
- Optional<List<DependsOn>> deps = Comment.from(comment);
- assertFalse(deps.isPresent());
+ assertThat(Comment.from(comment)).isEmpty();
}
@Test
public void testParseEmptyComment() {
String comment = "Depends-on:";
Optional<List<DependsOn>> deps = Comment.from(comment);
- assertTrue(deps.get().size() == 0);
+ assertThat(deps).isPresent();
+ assertThat(deps.get()).hasSize(0);
}
@Test
public void testParseOneNumComment() {
String comment = "Depends-on:" + NUM;
Optional<List<DependsOn>> deps = Comment.from(comment);
+ assertThat(deps).isPresent();
for (DependsOn dep : deps.get()) {
- assertTrue(NUM.equals("" + dep.id().get()));
+ assertThat("" + dep.id().get()).isEqualTo(NUM);
return;
}
- assertTrue(false);
+ assertWithMessage("not expected to reach").fail();
}
@Test
public void testParseOneKeyComment() {
String comment = "Depends-on:" + KEY;
Optional<List<DependsOn>> deps = Comment.from(comment);
+ assertThat(deps).isPresent();
for (DependsOn dep : deps.get()) {
- assertTrue(KEY.equals("" + dep.key().get()));
+ assertThat(dep.key().get()).isEqualTo(KEY);
return;
}
- assertTrue(false);
+ assertWithMessage("not expected to reach").fail();
}
@Test
public void testParseTwoNumsComment() {
String comment = "Depends-on:" + NUM + " " + NUM2;
Optional<List<DependsOn>> deps = Comment.from(comment);
- assertTrue(deps.get().size() == 2);
+ assertThat(deps).isPresent();
+ assertThat(deps.get()).hasSize(2);
int found = 0;
for (DependsOn dep : deps.get()) {
- assertTrue(NUM.equals("" + dep.id().get()) || NUM2.equals("" + dep.id().get()));
+ assertThat("" + dep.id().get()).isAnyOf(NUM, NUM2);
found++;
}
- assertTrue(found == 2);
+ assertThat(found).isEqualTo(2);
}
@Test
public void testParseTwoKeyComments() {
String comment = "Depends-on:" + KEY + " " + KEY2;
Optional<List<DependsOn>> deps = Comment.from(comment);
- assertTrue(deps.get().size() == 2);
+ assertThat(deps).isPresent();
+ assertThat(deps.get()).hasSize(2);
int found = 0;
for (DependsOn dep : deps.get()) {
- assertTrue(KEY.equals("" + dep.key().get()) || KEY2.equals("" + dep.key().get()));
+ assertThat(dep.key().get()).isAnyOf(KEY, KEY2);
found++;
}
- assertTrue(found == 2);
+ assertThat(found).isEqualTo(2);
}
@Test
public void testParseNumAndKeyComment() {
String comment = "Depends-on:" + NUM + " " + KEY;
Optional<List<DependsOn>> deps = Comment.from(comment);
- assertTrue(deps.get().size() == 2);
+ assertThat(deps).isPresent();
+ assertThat(deps.get()).hasSize(2);
int found = 0;
for (DependsOn dep : deps.get()) {
try {
- assertTrue(NUM.equals("" + dep.id().get()));
+ assertThat("" + dep.id().get()).isEqualTo(NUM);
} catch (Exception e) {
- assertTrue(KEY.equals("" + dep.key().get()));
+ assertThat(dep.key().get()).isEqualTo(KEY);
}
found++;
}
- assertTrue(found == 2);
+ assertThat(found).isEqualTo(2);
}
+ @Test
public void testParseTwoNumsCommaComment() {
String comment = "Depends-on:" + NUM + "," + NUM2;
Optional<List<DependsOn>> deps = Comment.from(comment);
- assertTrue(deps.get().size() == 2);
+ assertThat(deps).isPresent();
+ assertThat(deps.get()).hasSize(2);
int found = 0;
for (DependsOn dep : deps.get()) {
- assertTrue(NUM.equals("" + dep.id().get()) || NUM2.equals("" + dep.id().get()));
+ assertThat("" + dep.id().get()).isAnyOf(NUM, NUM2);
found++;
}
- assertTrue(found == 2);
+ assertThat(found).isEqualTo(2);
}
+ @Test
public void testParseTwoNumsCommaSpaceComment() {
String comment = "Depends-on:" + NUM + ", " + NUM2;
Optional<List<DependsOn>> deps = Comment.from(comment);
- assertTrue(deps.get().size() == 2);
+ assertThat(deps).isPresent();
+ assertThat(deps.get()).hasSize(2);
int found = 0;
for (DependsOn dep : deps.get()) {
- assertTrue(NUM.equals("" + dep.id().get()) || NUM2.equals("" + dep.id().get()));
+ assertThat("" + dep.id().get()).isAnyOf(NUM, NUM2);
found++;
}
- assertTrue(found == 2);
+ assertThat(found).isEqualTo(2);
}
+ @Test
public void testParseTwoNumsWhiteComment() {
String comment = "Depends-on:" + NUM + ", \t" + NUM2;
Optional<List<DependsOn>> deps = Comment.from(comment);
- assertTrue(deps.get().size() == 2);
+ assertThat(deps).isPresent();
+ assertThat(deps.get()).hasSize(2);
int found = 0;
for (DependsOn dep : deps.get()) {
- assertTrue(NUM.equals("" + dep.id().get()) || NUM2.equals("" + dep.id().get()));
+ assertThat("" + dep.id().get()).isAnyOf(NUM, NUM2);
found++;
}
- assertTrue(found == 2);
+ assertThat(found).isEqualTo(2);
}
+ @Test
public void testParseTwoNumsNewLineWhiteComment() {
// Should stop processing at newline
String comment = "Depends-on:" + NUM + ", \t\n" + NUM2;
Optional<List<DependsOn>> deps = Comment.from(comment);
- assertTrue(deps.get().size() == 1);
+ assertThat(deps).isPresent();
+ assertThat(deps.get()).hasSize(1);
for (DependsOn dep : deps.get()) {
- assertTrue(NUM.equals("" + dep.id().get()));
+ assertThat("" + dep.id().get()).isEqualTo(NUM);
}
}
+ @Test
public void testParseEmbeddedComment() {
String comment = "Patch Set 2:\n\nDepends-on:" + NUM + " " + NUM2 + "\nHey\n";
Optional<List<DependsOn>> deps = Comment.from(comment);
- assertTrue(deps.get().size() == 2);
+ assertThat(deps).isPresent();
+ assertThat(deps.get()).hasSize(2);
int found = 0;
for (DependsOn dep : deps.get()) {
- assertTrue(NUM.equals("" + dep.id().get()) || NUM2.equals("" + dep.id().get()));
+ assertThat("" + dep.id().get()).isAnyOf(NUM, NUM2);
found++;
}
- assertTrue(found == 2);
+ assertThat(found).isEqualTo(2);
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/depends/on/ResolveDependsOnTest.java b/src/test/java/com/googlesource/gerrit/plugins/depends/on/ResolveDependsOnTest.java
index 9fc4a05..bd7d34d 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/depends/on/ResolveDependsOnTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/depends/on/ResolveDependsOnTest.java
@@ -14,13 +14,18 @@
package com.googlesource.gerrit.plugins.depends.on;
+import static com.google.common.truth.Truth.assertThat;
+
import com.google.gerrit.testing.InMemoryModule;
import java.util.HashSet;
import java.util.Set;
-import junit.framework.TestCase;
+import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
-public class ResolveDependsOnTest extends TestCase {
+@RunWith(JUnit4.class)
+public class ResolveDependsOnTest {
public static final String NUM = "1234";
public static final String NUM2 = "345";
public static final String KEY = "Iabcdef7890abcdef7890abcdef7890abcdef7890";
@@ -31,42 +36,36 @@
public static DependsOn KEY_DEP;
public static DependsOn KEY2_DEP;
- @Override
- protected void setUp() throws Exception {
- super.setUp();
- new InMemoryModule().inject(this); // Needed to setup KeyUtil.ENCODER_IMPL
+ @Before
+ public void setUp() {
+ new InMemoryModule().inject(this); // Needed to set up KeyUtil.ENCODER_IMPL
NUM_DEP = DependsOn.create(NUM);
NUM2_DEP = DependsOn.create(NUM2);
KEY_DEP = DependsOn.create(KEY);
KEY2_DEP = DependsOn.create(KEY2);
}
- @Override
- protected void tearDown() throws Exception {
- super.tearDown();
- }
-
@Test
public void testResolved2Nums() {
- Set<DependsOn> deps = new HashSet<DependsOn>();
+ Set<DependsOn> deps = new HashSet<>();
deps.add(NUM_DEP);
deps.add(NUM2_DEP);
- assertTrue(Resolver.isResolved(deps));
+ assertThat(Resolver.isResolved(deps)).isTrue();
}
@Test
public void testResolved2Keys() {
- Set<DependsOn> deps = new HashSet<DependsOn>();
+ Set<DependsOn> deps = new HashSet<>();
deps.add(KEY_DEP);
deps.add(KEY2_DEP);
- assertFalse(Resolver.isResolved(deps));
+ assertThat(Resolver.isResolved(deps)).isFalse();
}
@Test
public void testResolvedNumAndKey() {
- Set<DependsOn> deps = new HashSet<DependsOn>();
+ Set<DependsOn> deps = new HashSet<>();
deps.add(NUM_DEP);
deps.add(KEY_DEP);
- assertFalse(Resolver.isResolved(deps));
+ assertThat(Resolver.isResolved(deps)).isFalse();
}
}
