Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix issue #5843 #6058

Closed
wants to merge 1 commit into from
Closed

Fix issue #5843 #6058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions android/guava-tests/test/com/google/common/graph/AbstractStandardUndirectedGraphTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ public void hasEdgeConnecting_correct() {
@Test
public void hasEdgeConnecting_mismatch() {
putEdge(N1, N2);
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N1, N2))).isTrue();
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N2, N1))).isTrue();
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N1, N2))).isFalse();
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N2, N1))).isFalse();
}

@Test
Expand Down
45 changes: 33 additions & 12 deletions android/guava-tests/test/com/google/common/graph/AbstractStandardUndirectedNetworkTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.common.graph;

import static com.google.common.graph.GraphConstants.ENDPOINTS_MISMATCH;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
import static org.junit.Assert.assertTrue;
Expand Down Expand Up @@ -195,15 +196,30 @@ public void successors_checkReturnedSetMutability() {
@Test
public void edges_containsOrderMismatch() {
addEdge(N1, N2, E12);
assertThat(network.asGraph().edges()).contains(ENDPOINTS_N2N1);
assertThat(network.asGraph().edges()).contains(ENDPOINTS_N1N2);
assertThat(network.asGraph().edges()).doesNotContain(ENDPOINTS_N2N1);
assertThat(network.asGraph().edges()).doesNotContain(ENDPOINTS_N1N2);
}

@Test
public void edgesConnecting_orderMismatch() {
addEdge(N1, N2, E12);
try {
Set<String> unused = network.edgesConnecting(ENDPOINTS_N1N2);
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
}
}

@Test
public void edgeConnectingOrNull_orderMismatch() {
addEdge(N1, N2, E12);
assertThat(network.edgeConnectingOrNull(ENDPOINTS_N2N1)).isEqualTo(E12);
assertThat(network.edgeConnectingOrNull(ENDPOINTS_N1N2)).isEqualTo(E12);
try {
String unused = network.edgeConnectingOrNull(ENDPOINTS_N1N2);
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
}
}

@Test
Expand Down Expand Up @@ -418,7 +434,7 @@ public void addEdge_existingEdgeBetweenDifferentNodes() {
networkAsMutableNetwork.addEdge(N4, N5, E12);
fail(ERROR_ADDED_EXISTING_EDGE);
} catch (IllegalArgumentException e) {
assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
}
}

Expand All @@ -432,13 +448,13 @@ public void addEdge_parallelEdge_notAllowed() {
networkAsMutableNetwork.addEdge(N1, N2, EDGE_NOT_IN_GRAPH);
fail(ERROR_ADDED_PARALLEL_EDGE);
} catch (IllegalArgumentException e) {
assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
}
try {
networkAsMutableNetwork.addEdge(N2, N1, EDGE_NOT_IN_GRAPH);
fail(ERROR_ADDED_PARALLEL_EDGE);
} catch (IllegalArgumentException e) {
assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
}
}

Expand All @@ -458,7 +474,12 @@ public void addEdge_orderMismatch() {
assume().that(graphIsMutable()).isTrue();

EndpointPair<Integer> endpoints = EndpointPair.ordered(N1, N2);
assertThat(networkAsMutableNetwork.addEdge(endpoints, E12)).isTrue();
try {
networkAsMutableNetwork.addEdge(endpoints, E12);
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
}
}

@Test
Expand Down Expand Up @@ -527,20 +548,20 @@ public void addEdge_existingEdgeBetweenDifferentNodes_selfLoops() {
networkAsMutableNetwork.addEdge(N1, N2, E11);
fail("Reusing an existing self-loop edge to connect different nodes succeeded");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
}
try {
networkAsMutableNetwork.addEdge(N2, N2, E11);
fail("Reusing an existing self-loop edge to make a different self-loop edge succeeded");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
}
addEdge(N1, N2, E12);
try {
networkAsMutableNetwork.addEdge(N1, N1, E12);
fail("Reusing an existing edge to add a self-loop edge between different nodes succeeded");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
}
}

Expand All @@ -555,7 +576,7 @@ public void addEdge_parallelSelfLoopEdge_notAllowed() {
networkAsMutableNetwork.addEdge(N1, N1, EDGE_NOT_IN_GRAPH);
fail("Adding a parallel self-loop edge succeeded");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
}
}

Expand Down
4 changes: 2 additions & 2 deletions android/guava-tests/test/com/google/common/graph/EndpointPairTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ public void endpointPair_undirected_contains() {
assertThat(edges).contains(EndpointPair.unordered(N1, N2));
assertThat(edges).contains(EndpointPair.unordered(N2, N1)); // equal to unordered(N1, N2)

// ordered endpoints OK for undirected graph (because ordering is irrelevant)
assertThat(edges).contains(EndpointPair.ordered(N1, N2));
// ordered endpoints not compatible with undirected graph
assertThat(edges).doesNotContain(EndpointPair.ordered(N1, N2));

assertThat(edges).doesNotContain(EndpointPair.unordered(N2, N2)); // edge not present
assertThat(edges).doesNotContain(EndpointPair.unordered(N3, N4)); // nodes not in graph
Expand Down
40 changes: 34 additions & 6 deletions android/guava-tests/test/com/google/common/graph/ValueGraphTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ public void hasEdgeConnecting_undirected_backwards() {
public void hasEdgeConnecting_undirected_mismatch() {
graph = ValueGraphBuilder.undirected().build();
graph.putEdgeValue(1, 2, "A");
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(1, 2))).isTrue();
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(2, 1))).isTrue();
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(1, 2))).isFalse();
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(2, 1))).isFalse();
}

@Test
Expand Down Expand Up @@ -223,8 +223,19 @@ public void edgeValueOrDefault_undirected_backwards() {
public void edgeValueOrDefault_undirected_mismatch() {
graph = ValueGraphBuilder.undirected().build();
graph.putEdgeValue(1, 2, "A");
assertThat(graph.edgeValueOrDefault(EndpointPair.ordered(2, 1), "default")).isEqualTo("A");
assertThat(graph.edgeValueOrDefault(EndpointPair.ordered(2, 1), "default")).isEqualTo("A");
// Check that edgeValueOrDefault() throws on each possible ordering of an ordered EndpointPair
try {
String unused = graph.edgeValueOrDefault(EndpointPair.ordered(1, 2), "default");
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
}
try {
String unused = graph.edgeValueOrDefault(EndpointPair.ordered(2, 1), "default");
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
}
}

@Test
Expand All @@ -251,7 +262,12 @@ public void putEdgeValue_directed_orderMismatch() {
@Test
public void putEdgeValue_undirected_orderMismatch() {
graph = ValueGraphBuilder.undirected().build();
assertThat(graph.putEdgeValue(EndpointPair.ordered(1, 2), "irrelevant")).isNull();
try {
graph.putEdgeValue(EndpointPair.ordered(1, 2), "irrelevant");
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
}
}

@Test
Expand Down Expand Up @@ -311,7 +327,19 @@ public void removeEdge_directed_orderMismatch() {
public void removeEdge_undirected_orderMismatch() {
graph = ValueGraphBuilder.undirected().build();
graph.putEdgeValue(1, 2, "1-2");
assertThat(graph.removeEdge(EndpointPair.ordered(1, 2))).isEqualTo("1-2");
// Check that removeEdge() throws on each possible ordering of an ordered EndpointPair
try {
graph.removeEdge(EndpointPair.ordered(1, 2));
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
}
try {
graph.removeEdge(EndpointPair.ordered(2, 1));
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
}
}

@Test
Expand Down
6 changes: 5 additions & 1 deletion android/guava/src/com/google/common/graph/AbstractBaseGraph.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,11 @@ protected final void validateEndpoints(EndpointPair<?> endpoints) {
checkArgument(isOrderingCompatible(endpoints), ENDPOINTS_MISMATCH);
}

/**
* Returns {@code true} iff {@code endpoints}' ordering is compatible with the directionality of
* this graph.
*/
protected final boolean isOrderingCompatible(EndpointPair<?> endpoints) {
return endpoints.isOrdered() || !this.isDirected();
return endpoints.isOrdered() == this.isDirected();
}
}
2 changes: 1 addition & 1 deletion android/guava/src/com/google/common/graph/AbstractNetwork.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ protected final void validateEndpoints(EndpointPair<?> endpoints) {
}

protected final boolean isOrderingCompatible(EndpointPair<?> endpoints) {
return endpoints.isOrdered() || !this.isDirected();
return endpoints.isOrdered() == this.isDirected();
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion android/guava/src/com/google/common/graph/GraphConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ private GraphConstants() {}
+ "adjacentNode(node) if you already have a node, or nodeU()/nodeV() if you don't.";
static final String EDGE_ALREADY_EXISTS = "Edge %s already exists in the graph.";
static final String ENDPOINTS_MISMATCH =
"Mismatch: unordered endpoints cannot be used with directed graphs";
"Mismatch: endpoints' ordering is not compatible with directionality of the graph";

/** Singleton edge value for {@link Graph} implementations backed by {@link ValueGraph}s. */
enum Presence {
Expand Down
4 changes: 2 additions & 2 deletions guava-tests/test/com/google/common/graph/AbstractStandardUndirectedGraphTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ public void hasEdgeConnecting_correct() {
@Test
public void hasEdgeConnecting_mismatch() {
putEdge(N1, N2);
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N1, N2))).isTrue();
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N2, N1))).isTrue();
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N1, N2))).isFalse();
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N2, N1))).isFalse();
}

@Test
Expand Down
52 changes: 35 additions & 17 deletions guava-tests/test/com/google/common/graph/AbstractStandardUndirectedNetworkTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@

package com.google.common.graph;

import static com.google.common.graph.GraphConstants.ENDPOINTS_MISMATCH;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import com.google.common.collect.ImmutableSet;
import com.google.common.testing.EqualsTester;
import java.util.Optional;
import java.util.Set;
import org.junit.After;
import org.junit.Test;
Expand Down Expand Up @@ -196,29 +197,41 @@ public void successors_checkReturnedSetMutability() {
@Test
public void edges_containsOrderMismatch() {
addEdge(N1, N2, E12);
assertThat(network.asGraph().edges()).contains(ENDPOINTS_N2N1);
assertThat(network.asGraph().edges()).contains(ENDPOINTS_N1N2);
assertThat(network.asGraph().edges()).doesNotContain(ENDPOINTS_N2N1);
assertThat(network.asGraph().edges()).doesNotContain(ENDPOINTS_N1N2);
}

@Test
public void edgesConnecting_orderMismatch() {
addEdge(N1, N2, E12);
assertThat(network.edgesConnecting(ENDPOINTS_N2N1)).containsExactly(E12);
assertThat(network.edgesConnecting(ENDPOINTS_N1N2)).containsExactly(E12);
try {
Set<String> unused = network.edgesConnecting(ENDPOINTS_N1N2);
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
}
}

@Test
public void edgeConnecting_orderMismatch() {
addEdge(N1, N2, E12);
assertThat(network.edgeConnecting(ENDPOINTS_N2N1)).hasValue(E12);
assertThat(network.edgeConnecting(ENDPOINTS_N1N2)).hasValue(E12);
try {
Optional<String> unused = network.edgeConnecting(ENDPOINTS_N1N2);
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
}
}

@Test
public void edgeConnectingOrNull_orderMismatch() {
addEdge(N1, N2, E12);
assertThat(network.edgeConnectingOrNull(ENDPOINTS_N2N1)).isEqualTo(E12);
assertThat(network.edgeConnectingOrNull(ENDPOINTS_N1N2)).isEqualTo(E12);
try {
String unused = network.edgeConnectingOrNull(ENDPOINTS_N1N2);
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
}
}

@Test
Expand Down Expand Up @@ -433,7 +446,7 @@ public void addEdge_existingEdgeBetweenDifferentNodes() {
networkAsMutableNetwork.addEdge(N4, N5, E12);
fail(ERROR_ADDED_EXISTING_EDGE);
} catch (IllegalArgumentException e) {
assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
}
}

Expand All @@ -447,13 +460,13 @@ public void addEdge_parallelEdge_notAllowed() {
networkAsMutableNetwork.addEdge(N1, N2, EDGE_NOT_IN_GRAPH);
fail(ERROR_ADDED_PARALLEL_EDGE);
} catch (IllegalArgumentException e) {
assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
}
try {
networkAsMutableNetwork.addEdge(N2, N1, EDGE_NOT_IN_GRAPH);
fail(ERROR_ADDED_PARALLEL_EDGE);
} catch (IllegalArgumentException e) {
assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
}
}

Expand All @@ -473,7 +486,12 @@ public void addEdge_orderMismatch() {
assume().that(graphIsMutable()).isTrue();

EndpointPair<Integer> endpoints = EndpointPair.ordered(N1, N2);
assertThat(networkAsMutableNetwork.addEdge(endpoints, E12)).isTrue();
try {
networkAsMutableNetwork.addEdge(endpoints, E12);
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
}
}

@Test
Expand Down Expand Up @@ -542,20 +560,20 @@ public void addEdge_existingEdgeBetweenDifferentNodes_selfLoops() {
networkAsMutableNetwork.addEdge(N1, N2, E11);
fail("Reusing an existing self-loop edge to connect different nodes succeeded");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
}
try {
networkAsMutableNetwork.addEdge(N2, N2, E11);
fail("Reusing an existing self-loop edge to make a different self-loop edge succeeded");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
}
addEdge(N1, N2, E12);
try {
networkAsMutableNetwork.addEdge(N1, N1, E12);
fail("Reusing an existing edge to add a self-loop edge between different nodes succeeded");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
}
}

Expand All @@ -570,7 +588,7 @@ public void addEdge_parallelSelfLoopEdge_notAllowed() {
networkAsMutableNetwork.addEdge(N1, N1, EDGE_NOT_IN_GRAPH);
fail("Adding a parallel self-loop edge succeeded");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
}
}

Expand Down
4 changes: 2 additions & 2 deletions guava-tests/test/com/google/common/graph/EndpointPairTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ public void endpointPair_undirected_contains() {
assertThat(edges).contains(EndpointPair.unordered(N1, N2));
assertThat(edges).contains(EndpointPair.unordered(N2, N1)); // equal to unordered(N1, N2)

// ordered endpoints OK for undirected graph (because ordering is irrelevant)
assertThat(edges).contains(EndpointPair.ordered(N1, N2));
// ordered endpoints not compatible with undirected graph
assertThat(edges).doesNotContain(EndpointPair.ordered(N1, N2));

assertThat(edges).doesNotContain(EndpointPair.unordered(N2, N2)); // edge not present
assertThat(edges).doesNotContain(EndpointPair.unordered(N3, N4)); // nodes not in graph
Expand Down
Loading