Skip to content

Commit a2d0b92

Browse files
JLLeitschuhJLLeitschuh
committed
Adds Tests For New Publishing and Fixes Various Bugs
1 parent 92c8914 commit a2d0b92

File tree

11 files changed

+472
-185
lines changed

11 files changed

+472
-185
lines changed

‎core/src/main/java/edu/wpi/grip/core/operations/network/MapNetworkPublisher.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@ protected MapNetworkPublisher(Set<String> keys) {
3232
public final void publish(Map<String, T> publishMap) {
3333
final Map<String, T> publishMapCopy = ImmutableMap.copyOf(publishMap);
3434
if (!keys.isEmpty()) {
35-
publishMap.entrySet().forEach(key -> checkArgument(keys.contains(key), "Key must be in keys list: " + key));
35+
publishMap.keySet().forEach(key -> checkArgument(keys.contains(key), "Key must be in keys list: " + key));
3636
}
3737
checkNamePresent();
38-
if (!publishMapCopy.containsKey("")) {
38+
if (!publishMapCopy.containsKey("") && !keys.isEmpty()) {
3939
doPublish(publishMapCopy);
4040
} else if (!publishMapCopy.keySet().isEmpty()) {
4141
doPublishSingle(publishMapCopy.get(""));

‎core/src/main/java/edu/wpi/grip/core/operations/network/NetworkPublisher.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ protected final void checkNamePresent() {
4949
}
5050
}
5151

52+
/**
53+
* Should be called to pass a value to be published to the NetworkPublisher
54+
*
55+
* @param publish The value to publish
56+
*/
5257
public abstract void publish(T publish);
5358

5459
/**

‎core/src/main/java/edu/wpi/grip/core/operations/network/PublishAnnotatedOperation.java

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import edu.wpi.grip.core.SocketHints;
1313

1414
import java.lang.reflect.InvocationTargetException;
15+
import java.lang.reflect.Modifier;
1516
import java.util.*;
1617
import java.util.function.Function;
1718
import java.util.stream.Collectors;
@@ -73,14 +74,23 @@ public PublishAnnotatedOperation(MapNetworkPublisherFactory factory, Function<S,
7374
throw new IllegalArgumentException("Must be at least one @PublishValue method on " + reportType + " to be published");
7475
}
7576

77+
if (!Modifier.isPublic(reportType.getRawType().getModifiers())) {
78+
throw new IllegalAccessError(reportType.getRawType().getName() + " must be public");
79+
}
80+
81+
if (reportType.getRawType().getEnclosingClass() != null && !Modifier.isStatic(reportType.getRawType().getModifiers())) {
82+
throw new IllegalAccessError(reportType.getRawType().getName() + " must be static if declared as an inner class");
83+
}
84+
7685
// In order for PublishAnnotatedOperation to call the accessor methods, they must all be public
7786
this.valueMethods.stream()
7887
.filter(m -> !m.isPublic())
7988
.findAny()
8089
.ifPresent(m -> {
81-
throw new IllegalArgumentException("@PublishValue method must be public: " + m);
90+
throw new IllegalAccessError("@PublishValue method must be public: " + m);
8291
});
8392

93+
8494
// In order for PublishAnnotatedOperation to call the accessor methods, they must all have no parameters
8595
this.valueMethods.stream()
8696
.filter(method -> method.getParameters().size() > 0)
@@ -95,7 +105,9 @@ public PublishAnnotatedOperation(MapNetworkPublisherFactory factory, Function<S,
95105
throw new IllegalArgumentException("@PublishValue methods must have distinct keys: " + reportType);
96106
}
97107
this.keys = ImmutableSet
98-
.copyOf(this.valueMethods.stream().map(method -> method.getAnnotation(PublishValue.class).key())
108+
.copyOf(this.valueMethods
109+
.stream()
110+
.map(method -> method.getAnnotation(PublishValue.class).key())
99111
.filter(k -> !k.isEmpty()) // Check that the key is not empty
100112
.iterator());
101113

‎core/src/main/java/edu/wpi/grip/core/operations/network/networktables/NTManager.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ protected void publishNameChanged(Optional<String> oldName, String newName) {
140140
@Override
141141
protected void doPublish(Map<String, P> publishValueMap) {
142142
publishValueMap.forEach(getTable()::putValue);
143-
Sets.difference(keys, publishValueMap.entrySet()).forEach(getTable()::delete);
143+
Sets.difference(keys, publishValueMap.keySet()).forEach(getTable()::delete);
144144
}
145145

146146
@Override

‎core/src/test/java/edu/wpi/grip/core/operations/network/MapNetworkPublisherTest.java

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
package edu.wpi.grip.core.operations.network;
2+
3+
4+
import autovalue.shaded.com.google.common.common.collect.ImmutableMap;
5+
import org.junit.Test;
6+
7+
import java.util.*;
8+
9+
import static org.junit.Assert.assertEquals;
10+
import static org.junit.Assert.assertTrue;
11+
import static org.junit.Assert.fail;
12+
13+
public class MapNetworkPublisherTest {
14+
private static final String SHOULD_NOT_HAVE_BEEN_CALLED = "Should not have been called";
15+
16+
private abstract static class DoubleMapPublisher extends MapNetworkPublisher<Double> {
17+
18+
protected DoubleMapPublisher(Set<String> keys) {
19+
super(keys);
20+
}
21+
22+
@Override
23+
protected void publishNameChanged(Optional<String> oldName, String newName) {
24+
/* no-op */
25+
}
26+
27+
@Override
28+
public void close() {
29+
/* no-op */
30+
}
31+
}
32+
33+
34+
@Test
35+
public void testPublisherCallsCorrectDoPublishWhenKeysAreProvided() {
36+
boolean doPublishMapWasCalled[] = {false};
37+
final DoubleMapPublisher doubleMapPublisher =
38+
new DoubleMapPublisher(
39+
new HashSet<>(Arrays.asList("Apple"))) {
40+
@Override
41+
protected void doPublish(Map<String, Double> publishMap) {
42+
assertTrue("Map should be empty", publishMap.isEmpty());
43+
doPublishMapWasCalled[0] = true;
44+
}
45+
46+
@Override
47+
protected void doPublishSingle(Double value) {
48+
fail(SHOULD_NOT_HAVE_BEEN_CALLED);
49+
}
50+
51+
@Override
52+
protected void doPublish() {
53+
fail(SHOULD_NOT_HAVE_BEEN_CALLED);
54+
}
55+
};
56+
doubleMapPublisher.setName("Don't care");
57+
doubleMapPublisher.publish(new HashMap<>());
58+
assertTrue("doPublish should have been called", doPublishMapWasCalled[0]);
59+
}
60+
61+
@Test
62+
public void testPublisherCallsCorrectDoPublishWhenNoKeysAreProvided() {
63+
boolean doPublishNothingWasCalled[] = {false};
64+
final DoubleMapPublisher doubleMapPublisher =
65+
new DoubleMapPublisher(
66+
new HashSet<>()) {
67+
@Override
68+
protected void doPublish(Map<String, Double> publishMap) {
69+
fail(SHOULD_NOT_HAVE_BEEN_CALLED);
70+
}
71+
72+
@Override
73+
protected void doPublishSingle(Double value) {
74+
fail(SHOULD_NOT_HAVE_BEEN_CALLED);
75+
}
76+
77+
@Override
78+
protected void doPublish() {
79+
doPublishNothingWasCalled[0] = true;
80+
}
81+
};
82+
doubleMapPublisher.setName("Don't care");
83+
doubleMapPublisher.publish(new HashMap<>());
84+
assertTrue("doPublish should have been called", doPublishNothingWasCalled[0]);
85+
}
86+
87+
@Test
88+
public void testPublisherCallsCorrectDoPublishWhenNoKeysAreProvidedAndMapIsNotEmpty() {
89+
double EXPECTED_VALUE = Math.PI;
90+
boolean doPublishSingleValueWasCalled[] = {false};
91+
final DoubleMapPublisher doubleMapPublisher =
92+
new DoubleMapPublisher(
93+
new HashSet<>()) {
94+
@Override
95+
protected void doPublish(Map<String, Double> publishMap) {
96+
fail(SHOULD_NOT_HAVE_BEEN_CALLED);
97+
}
98+
99+
@Override
100+
protected void doPublishSingle(Double value) {
101+
doPublishSingleValueWasCalled[0] = true;
102+
assertEquals("Should have published the expected value", EXPECTED_VALUE, value, 0.001);
103+
}
104+
105+
@Override
106+
protected void doPublish() {
107+
fail(SHOULD_NOT_HAVE_BEEN_CALLED);
108+
}
109+
};
110+
doubleMapPublisher.setName("Don't care");
111+
doubleMapPublisher.publish(ImmutableMap.of("", EXPECTED_VALUE));
112+
assertTrue("doPublish with a single value should have been called", doPublishSingleValueWasCalled[0]);
113+
}
114+
115+
}

‎core/src/test/java/edu/wpi/grip/core/operations/network/MockManager.java

Lines changed: 0 additions & 5 deletions
This file was deleted.

‎core/src/test/java/edu/wpi/grip/core/operations/network/MockNetworkPublisher.java

Lines changed: 0 additions & 23 deletions
This file was deleted.

‎core/src/test/java/edu/wpi/grip/core/operations/network/NetworkPackageSanityTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
public class NetworkPackageSanityTest extends AbstractPackageSanityTests {
77
public NetworkPackageSanityTest() {
88
super();
9-
ignoreClasses(c -> c.equals(MockNetworkPublisher.class));
109
publicApiOnly();
1110
}
1211
}

‎core/src/test/java/edu/wpi/grip/core/operations/network/NetworkPublisherTest.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package edu.wpi.grip.core.operations.network;
2+
3+
4+
import org.junit.Test;
5+
6+
import java.util.Optional;
7+
8+
import static org.junit.Assert.assertEquals;
9+
10+
public class NetworkPublisherTest {
11+
private abstract static class TestNetworkPublisher extends NetworkPublisher<Double> {
12+
13+
@Override
14+
public void publish(Double publish) {
15+
/* no-op */
16+
}
17+
18+
@Override
19+
public void close(){
20+
/* no-op */
21+
}
22+
}
23+
24+
@Test
25+
public void testThatNameChangeIsCalledInitiallyButNotAgainIfNameIsSame() {
26+
final String NAME = "QUACKERY!";
27+
final int[] publishNameChangedCallCount = {0};
28+
final TestNetworkPublisher publisher = new TestNetworkPublisher() {
29+
@Override
30+
protected void publishNameChanged(Optional<String> oldName, String newName) {
31+
publishNameChangedCallCount[0]++;
32+
assertEquals("Name was not the new name", NAME, newName);
33+
}
34+
};
35+
publisher.setName(NAME);
36+
assertEquals("publishNameChanged was called an unexpected number of times", 1, publishNameChangedCallCount[0]);
37+
publisher.setName(NAME);
38+
assertEquals("publishNameChanged should not have been called agian", 1, publishNameChangedCallCount[0]);
39+
}
40+
}

0 commit comments

Comments
 (0)