Skip to content

Commit 20e00a1

Browse files
cpovirkGoogle Java Core Libraries
authored andcommitted
Truthify graph tests.
We'd already been using Truth a lot, so many of the examples here are just to use `assertThat(x).isTrue()` instead of `assertTrue(x)` (or similarly for "false" assertions). It's reasonable to debate whether that's an improvement. I will say that I've been surprised multiple times recently by `assertFalse` assertions, similar to how it's unfortunate to have `if (!some.very().long(EXPRESSION).afterWhich(youHaveForgottenAboutTheNegation)`, so I'm more a fan of using Truth there than I used to be. The possibility of using Truth more came to my attention during cl/810738337 because of the `Optional` assertions in `GraphsTest` (and `ValueGraphTest`), where Truth provides clearer value. I noticed that `containsExactlySanityCheck` was doing [the kind of thing we discourage](google/truth#268), in which it calls methods like `hasSize` and `contains` with the apparently intention of testing our implementations of `size()` and `contains(...)`, rather than verifying that some _earlier_ step had produced the right answer. I've migrated it to direct calls to `Collection` methods. Arguably the best approach would be to have one such test and then to use Truth's usual methods (with their superior failure messages) for "normal" tests. Finally, in `TraverserTest`, I suspect that we were copying to an `ImmutableList` in order to be sure that we got a good failure message even in the absence of an implementation of `Traverser.toString()`. I verified that we do get a good message even without the copy because [Truth has our backs](https://github.com/google/truth/blob/22a4601c3c6b79b285740d37e82ec8dc56d2344d/core/src/main/java/com/google/common/truth/IterableSubject.java#L107-L119): ``` unexpected (1): b --- expected : [a] but was : [a, b] ``` RELNOTES=n/a PiperOrigin-RevId: 813317064
1 parent a49016f commit 20e00a1

20 files changed

+115
-121
lines changed

android/guava-tests/test/com/google/common/graph/AbstractNetworkTest.java

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,7 @@
2525
import static com.google.common.truth.Truth.assertThat;
2626
import static com.google.common.truth.TruthJUnit.assume;
2727
import static java.util.concurrent.Executors.newFixedThreadPool;
28-
import static org.junit.Assert.assertFalse;
2928
import static org.junit.Assert.assertThrows;
30-
import static org.junit.Assert.assertTrue;
3129
import static org.junit.Assert.fail;
3230

3331
import com.google.common.collect.ImmutableList;
@@ -187,8 +185,9 @@ static <N, E> void validateNetwork(Network<N, E> network) {
187185
for (N incidentNode : network.incidentNodes(edge)) {
188186
assertThat(network.nodes()).contains(incidentNode);
189187
for (E adjacentEdge : network.incidentEdges(incidentNode)) {
190-
assertTrue(
191-
edge.equals(adjacentEdge) || network.adjacentEdges(edge).contains(adjacentEdge));
188+
assertThat(
189+
edge.equals(adjacentEdge) || network.adjacentEdges(edge).contains(adjacentEdge))
190+
.isTrue();
192191
}
193192
}
194193
}
@@ -263,12 +262,14 @@ static <N, E> void validateNetwork(Network<N, E> network) {
263262
}
264263

265264
for (N adjacentNode : sanityCheckSet(network.adjacentNodes(node))) {
266-
assertTrue(
267-
network.predecessors(node).contains(adjacentNode)
268-
|| network.successors(node).contains(adjacentNode));
269-
assertTrue(
270-
!network.edgesConnecting(node, adjacentNode).isEmpty()
271-
|| !network.edgesConnecting(adjacentNode, node).isEmpty());
265+
assertThat(
266+
network.predecessors(node).contains(adjacentNode)
267+
|| network.successors(node).contains(adjacentNode))
268+
.isTrue();
269+
assertThat(
270+
!network.edgesConnecting(node, adjacentNode).isEmpty()
271+
|| !network.edgesConnecting(adjacentNode, node).isEmpty())
272+
.isTrue();
272273
}
273274

274275
for (N predecessor : sanityCheckSet(network.predecessors(node))) {
@@ -282,9 +283,10 @@ static <N, E> void validateNetwork(Network<N, E> network) {
282283
}
283284

284285
for (E incidentEdge : sanityCheckSet(network.incidentEdges(node))) {
285-
assertTrue(
286-
network.inEdges(node).contains(incidentEdge)
287-
|| network.outEdges(node).contains(incidentEdge));
286+
assertThat(
287+
network.inEdges(node).contains(incidentEdge)
288+
|| network.outEdges(node).contains(incidentEdge))
289+
.isTrue();
288290
assertThat(network.edges()).contains(incidentEdge);
289291
assertThat(network.incidentNodes(incidentEdge)).contains(node);
290292
}
@@ -623,7 +625,7 @@ public void successors_nodeNotInGraph() {
623625
public void addNode_newNode() {
624626
assume().that(graphIsMutable()).isTrue();
625627

626-
assertTrue(networkAsMutableNetwork.addNode(N1));
628+
assertThat(networkAsMutableNetwork.addNode(N1)).isTrue();
627629
assertThat(networkAsMutableNetwork.nodes()).contains(N1);
628630
}
629631

@@ -633,7 +635,7 @@ public void addNode_existingNode() {
633635

634636
addNode(N1);
635637
ImmutableSet<Integer> nodes = ImmutableSet.copyOf(networkAsMutableNetwork.nodes());
636-
assertFalse(networkAsMutableNetwork.addNode(N1));
638+
assertThat(networkAsMutableNetwork.addNode(N1)).isFalse();
637639
assertThat(networkAsMutableNetwork.nodes()).containsExactlyElementsIn(nodes);
638640
}
639641

@@ -643,8 +645,8 @@ public void removeNode_existingNode() {
643645

644646
addEdge(N1, N2, E12);
645647
addEdge(N4, N1, E41);
646-
assertTrue(networkAsMutableNetwork.removeNode(N1));
647-
assertFalse(networkAsMutableNetwork.removeNode(N1));
648+
assertThat(networkAsMutableNetwork.removeNode(N1)).isTrue();
649+
assertThat(networkAsMutableNetwork.removeNode(N1)).isFalse();
648650
assertThat(networkAsMutableNetwork.nodes()).containsExactly(N2, N4);
649651
assertThat(networkAsMutableNetwork.edges()).doesNotContain(E12);
650652
assertThat(networkAsMutableNetwork.edges()).doesNotContain(E41);
@@ -678,7 +680,7 @@ public void removeNode_nodeNotPresent() {
678680

679681
addNode(N1);
680682
ImmutableSet<Integer> nodes = ImmutableSet.copyOf(networkAsMutableNetwork.nodes());
681-
assertFalse(networkAsMutableNetwork.removeNode(NODE_NOT_IN_GRAPH));
683+
assertThat(networkAsMutableNetwork.removeNode(NODE_NOT_IN_GRAPH)).isFalse();
682684
assertThat(networkAsMutableNetwork.nodes()).containsExactlyElementsIn(nodes);
683685
}
684686

@@ -736,8 +738,8 @@ public void removeEdge_existingEdge() {
736738
assume().that(graphIsMutable()).isTrue();
737739

738740
addEdge(N1, N2, E12);
739-
assertTrue(networkAsMutableNetwork.removeEdge(E12));
740-
assertFalse(networkAsMutableNetwork.removeEdge(E12));
741+
assertThat(networkAsMutableNetwork.removeEdge(E12)).isTrue();
742+
assertThat(networkAsMutableNetwork.removeEdge(E12)).isFalse();
741743
assertThat(networkAsMutableNetwork.edges()).doesNotContain(E12);
742744
assertThat(networkAsMutableNetwork.edgesConnecting(N1, N2)).isEmpty();
743745
}
@@ -750,7 +752,7 @@ public void removeEdge_oneOfMany() {
750752
addEdge(N1, N3, E13);
751753
addEdge(N1, N4, E14);
752754
assertThat(networkAsMutableNetwork.edges()).containsExactly(E12, E13, E14);
753-
assertTrue(networkAsMutableNetwork.removeEdge(E13));
755+
assertThat(networkAsMutableNetwork.removeEdge(E13)).isTrue();
754756
assertThat(networkAsMutableNetwork.edges()).containsExactly(E12, E14);
755757
}
756758

@@ -760,7 +762,7 @@ public void removeEdge_edgeNotPresent() {
760762

761763
addEdge(N1, N2, E12);
762764
ImmutableSet<String> edges = ImmutableSet.copyOf(networkAsMutableNetwork.edges());
763-
assertFalse(networkAsMutableNetwork.removeEdge(EDGE_NOT_IN_GRAPH));
765+
assertThat(networkAsMutableNetwork.removeEdge(EDGE_NOT_IN_GRAPH)).isFalse();
764766
assertThat(networkAsMutableNetwork.edges()).containsExactlyElementsIn(edges);
765767
}
766768

@@ -772,7 +774,7 @@ public void removeEdge_queryAfterRemoval() {
772774
@SuppressWarnings("unused")
773775
EndpointPair<Integer> unused =
774776
networkAsMutableNetwork.incidentNodes(E12); // ensure cache (if any) is populated
775-
assertTrue(networkAsMutableNetwork.removeEdge(E12));
777+
assertThat(networkAsMutableNetwork.removeEdge(E12)).isTrue();
776778
assertEdgeNotInGraphErrorMessage(
777779
assertThrows(
778780
IllegalArgumentException.class, () -> networkAsMutableNetwork.incidentNodes(E12)));
@@ -785,7 +787,7 @@ public void removeEdge_parallelEdge() {
785787

786788
addEdge(N1, N2, E12);
787789
addEdge(N1, N2, E12_A);
788-
assertTrue(networkAsMutableNetwork.removeEdge(E12_A));
790+
assertThat(networkAsMutableNetwork.removeEdge(E12_A)).isTrue();
789791
assertThat(network.edgesConnecting(N1, N2)).containsExactly(E12);
790792
}
791793

@@ -798,10 +800,10 @@ public void removeEdge_parallelSelfLoopEdge() {
798800
addEdge(N1, N1, E11);
799801
addEdge(N1, N1, E11_A);
800802
addEdge(N1, N2, E12);
801-
assertTrue(networkAsMutableNetwork.removeEdge(E11_A));
803+
assertThat(networkAsMutableNetwork.removeEdge(E11_A)).isTrue();
802804
assertThat(network.edgesConnecting(N1, N1)).containsExactly(E11);
803805
assertThat(network.edgesConnecting(N1, N2)).containsExactly(E12);
804-
assertTrue(networkAsMutableNetwork.removeEdge(E11));
806+
assertThat(networkAsMutableNetwork.removeEdge(E11)).isTrue();
805807
assertThat(network.edgesConnecting(N1, N1)).isEmpty();
806808
assertThat(network.edgesConnecting(N1, N2)).containsExactly(E12);
807809
}

android/guava-tests/test/com/google/common/graph/AbstractStandardDirectedGraphTest.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import static com.google.common.truth.Truth.assertThat;
2121
import static com.google.common.truth.TruthJUnit.assume;
2222
import static org.junit.Assert.assertThrows;
23-
import static org.junit.Assert.assertTrue;
2423

2524
import java.util.Set;
2625
import org.jspecify.annotations.NullUnmarked;
@@ -362,9 +361,9 @@ public void putEdge_nodesNotInGraph() {
362361
assume().that(graphIsMutable()).isTrue();
363362

364363
graphAsMutableGraph.addNode(N1);
365-
assertTrue(graphAsMutableGraph.putEdge(N1, N5));
366-
assertTrue(graphAsMutableGraph.putEdge(N4, N1));
367-
assertTrue(graphAsMutableGraph.putEdge(N2, N3));
364+
assertThat(graphAsMutableGraph.putEdge(N1, N5)).isTrue();
365+
assertThat(graphAsMutableGraph.putEdge(N4, N1)).isTrue();
366+
assertThat(graphAsMutableGraph.putEdge(N2, N3)).isTrue();
368367
assertThat(graph.nodes()).containsExactly(N1, N5, N4, N2, N3).inOrder();
369368
assertThat(graph.successors(N1)).containsExactly(N5);
370369
assertThat(graph.successors(N2)).containsExactly(N3);

android/guava-tests/test/com/google/common/graph/AbstractStandardDirectedNetworkTest.java

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import static com.google.common.truth.Truth.assertThat;
2222
import static com.google.common.truth.TruthJUnit.assume;
2323
import static org.junit.Assert.assertThrows;
24-
import static org.junit.Assert.assertTrue;
2524
import static org.junit.Assert.fail;
2625

2726
import com.google.common.collect.ImmutableSet;
@@ -509,8 +508,8 @@ public void addEdge_parallelEdge_allowsParallelEdges() {
509508
assume().that(graphIsMutable()).isTrue();
510509
assume().that(network.allowsParallelEdges()).isTrue();
511510

512-
assertTrue(networkAsMutableNetwork.addEdge(N1, N2, E12));
513-
assertTrue(networkAsMutableNetwork.addEdge(N1, N2, E12_A));
511+
assertThat(networkAsMutableNetwork.addEdge(N1, N2, E12)).isTrue();
512+
assertThat(networkAsMutableNetwork.addEdge(N1, N2, E12_A)).isTrue();
514513
assertThat(network.edgesConnecting(N1, N2)).containsExactly(E12, E12_A);
515514
}
516515

@@ -547,9 +546,9 @@ public void addEdge_nodesNotInGraph() {
547546
assume().that(graphIsMutable()).isTrue();
548547

549548
networkAsMutableNetwork.addNode(N1);
550-
assertTrue(networkAsMutableNetwork.addEdge(N1, N5, E15));
551-
assertTrue(networkAsMutableNetwork.addEdge(N4, N1, E41));
552-
assertTrue(networkAsMutableNetwork.addEdge(N2, N3, E23));
549+
assertThat(networkAsMutableNetwork.addEdge(N1, N5, E15)).isTrue();
550+
assertThat(networkAsMutableNetwork.addEdge(N4, N1, E41)).isTrue();
551+
assertThat(networkAsMutableNetwork.addEdge(N2, N3, E23)).isTrue();
553552
assertThat(network.nodes()).containsExactly(N1, N5, N4, N2, N3);
554553
assertThat(network.edges()).containsExactly(E15, E41, E23);
555554
assertThat(network.edgesConnecting(N1, N5)).containsExactly(E15);
@@ -621,8 +620,8 @@ public void addEdge_parallelSelfLoopEdge_allowsParallelEdges() {
621620
assume().that(network.allowsSelfLoops()).isTrue();
622621
assume().that(network.allowsParallelEdges()).isTrue();
623622

624-
assertTrue(networkAsMutableNetwork.addEdge(N1, N1, E11));
625-
assertTrue(networkAsMutableNetwork.addEdge(N1, N1, E11_A));
623+
assertThat(networkAsMutableNetwork.addEdge(N1, N1, E11)).isTrue();
624+
assertThat(networkAsMutableNetwork.addEdge(N1, N1, E11_A)).isTrue();
626625
assertThat(network.edgesConnecting(N1, N1)).containsExactly(E11, E11_A);
627626
}
628627

android/guava-tests/test/com/google/common/graph/AbstractStandardUndirectedGraphTest.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import static com.google.common.truth.Truth.assertThat;
2020
import static com.google.common.truth.TruthJUnit.assume;
2121
import static org.junit.Assert.assertThrows;
22-
import static org.junit.Assert.assertTrue;
2322

2423
import com.google.common.testing.EqualsTester;
2524
import java.util.Set;
@@ -348,9 +347,9 @@ public void putEdge_nodesNotInGraph() {
348347
assume().that(graphIsMutable()).isTrue();
349348

350349
graphAsMutableGraph.addNode(N1);
351-
assertTrue(graphAsMutableGraph.putEdge(N1, N5));
352-
assertTrue(graphAsMutableGraph.putEdge(N4, N1));
353-
assertTrue(graphAsMutableGraph.putEdge(N2, N3));
350+
assertThat(graphAsMutableGraph.putEdge(N1, N5)).isTrue();
351+
assertThat(graphAsMutableGraph.putEdge(N4, N1)).isTrue();
352+
assertThat(graphAsMutableGraph.putEdge(N2, N3)).isTrue();
354353
assertThat(graph.nodes()).containsExactly(N1, N5, N4, N2, N3).inOrder();
355354
assertThat(graph.adjacentNodes(N1)).containsExactly(N4, N5);
356355
assertThat(graph.adjacentNodes(N2)).containsExactly(N3);

android/guava-tests/test/com/google/common/graph/AbstractStandardUndirectedNetworkTest.java

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import static com.google.common.truth.Truth.assertThat;
2121
import static com.google.common.truth.TruthJUnit.assume;
2222
import static org.junit.Assert.assertThrows;
23-
import static org.junit.Assert.assertTrue;
2423
import static org.junit.Assert.fail;
2524

2625
import com.google.common.collect.ImmutableSet;
@@ -427,9 +426,9 @@ public void addEdge_parallelEdge_allowsParallelEdges() {
427426
assume().that(graphIsMutable()).isTrue();
428427
assume().that(network.allowsParallelEdges()).isTrue();
429428

430-
assertTrue(networkAsMutableNetwork.addEdge(N1, N2, E12));
431-
assertTrue(networkAsMutableNetwork.addEdge(N2, N1, E21));
432-
assertTrue(networkAsMutableNetwork.addEdge(N1, N2, E12_A));
429+
assertThat(networkAsMutableNetwork.addEdge(N1, N2, E12)).isTrue();
430+
assertThat(networkAsMutableNetwork.addEdge(N2, N1, E21)).isTrue();
431+
assertThat(networkAsMutableNetwork.addEdge(N1, N2, E12_A)).isTrue();
433432
assertThat(network.edgesConnecting(N1, N2)).containsExactly(E12, E12_A, E21);
434433
}
435434

@@ -466,9 +465,9 @@ public void addEdge_nodesNotInGraph() {
466465
assume().that(graphIsMutable()).isTrue();
467466

468467
networkAsMutableNetwork.addNode(N1);
469-
assertTrue(networkAsMutableNetwork.addEdge(N1, N5, E15));
470-
assertTrue(networkAsMutableNetwork.addEdge(N4, N1, E41));
471-
assertTrue(networkAsMutableNetwork.addEdge(N2, N3, E23));
468+
assertThat(networkAsMutableNetwork.addEdge(N1, N5, E15)).isTrue();
469+
assertThat(networkAsMutableNetwork.addEdge(N4, N1, E41)).isTrue();
470+
assertThat(networkAsMutableNetwork.addEdge(N2, N3, E23)).isTrue();
472471
assertThat(network.nodes()).containsExactly(N1, N5, N4, N2, N3);
473472
assertThat(network.edges()).containsExactly(E15, E41, E23);
474473
assertThat(network.edgesConnecting(N1, N5)).containsExactly(E15);
@@ -539,8 +538,8 @@ public void addEdge_parallelSelfLoopEdge_allowsParallelEdges() {
539538
assume().that(network.allowsSelfLoops()).isTrue();
540539
assume().that(network.allowsParallelEdges()).isTrue();
541540

542-
assertTrue(networkAsMutableNetwork.addEdge(N1, N1, E11));
543-
assertTrue(networkAsMutableNetwork.addEdge(N1, N1, E11_A));
541+
assertThat(networkAsMutableNetwork.addEdge(N1, N1, E11)).isTrue();
542+
assertThat(networkAsMutableNetwork.addEdge(N1, N1, E11_A)).isTrue();
544543
assertThat(network.edgesConnecting(N1, N1)).containsExactly(E11, E11_A);
545544
}
546545

android/guava-tests/test/com/google/common/graph/EndpointPairTest.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -235,11 +235,13 @@ public void endpointPair_directed_contains() {
235235
assertThat(edges).doesNotContain(EndpointPair.ordered(N3, N4)); // nodes not in graph
236236
}
237237

238+
// We are testing our implementations of methods on Collection.
239+
@SuppressWarnings({"CollectionSizeTruth", "CollectionContainsTruth"})
238240
private static void containsExactlySanityCheck(Collection<?> collection, Object... varargs) {
239-
assertThat(collection).hasSize(varargs.length);
241+
assertThat(collection.size()).isEqualTo(varargs.length);
240242
for (Object obj : varargs) {
241-
assertThat(collection).contains(obj);
243+
assertThat(collection.contains(obj)).isTrue();
242244
}
243-
assertThat(collection).containsExactly(varargs);
245+
assertThat(ImmutableList.copyOf(collection.iterator())).containsExactlyElementsIn(varargs);
244246
}
245247
}

android/guava-tests/test/com/google/common/graph/ImmutableNetworkTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ public void immutableNetwork() {
3434
mutableNetwork.addNode("A");
3535
ImmutableNetwork<String, Integer> immutableNetwork = ImmutableNetwork.copyOf(mutableNetwork);
3636

37-
assertThat(immutableNetwork.asGraph()).isInstanceOf(ImmutableGraph.class);
3837
assertThat(immutableNetwork).isNotInstanceOf(MutableNetwork.class);
3938
assertThat(immutableNetwork).isEqualTo(mutableNetwork);
4039

android/guava-tests/test/com/google/common/graph/ImmutableValueGraphTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ public void immutableValueGraph() {
3535
ImmutableValueGraph<String, Integer> immutableValueGraph =
3636
ImmutableValueGraph.copyOf(mutableValueGraph);
3737

38-
assertThat(immutableValueGraph.asGraph()).isInstanceOf(ImmutableGraph.class);
3938
assertThat(immutableValueGraph).isNotInstanceOf(MutableValueGraph.class);
4039
assertThat(immutableValueGraph).isEqualTo(mutableValueGraph);
4140

android/guava-tests/test/com/google/common/graph/TraverserTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import static org.junit.Assert.assertThrows;
2525

2626
import com.google.common.collect.HashMultiset;
27-
import com.google.common.collect.ImmutableList;
2827
import com.google.common.collect.ImmutableMultimap;
2928
import com.google.common.collect.Iterables;
3029
import com.google.common.collect.Multiset;
@@ -1213,7 +1212,7 @@ private static ImmutableGraph<Character> createSingleRootGraph() {
12131212
}
12141213

12151214
private static void assertEqualCharNodes(Iterable<Character> result, String expectedCharacters) {
1216-
assertThat(ImmutableList.copyOf(result))
1215+
assertThat(result)
12171216
.containsExactlyElementsIn(Chars.asList(expectedCharacters.toCharArray()))
12181217
.inOrder();
12191218
}

0 commit comments

Comments
 (0)