Skip to content

Commit 5364eee

Browse files
committed
Fix Flaky Tests in GraphSONTypedCompatibilityTest
We observed several tests in GraphSONTypedCompatibilityTest that exhibited flaky behavior when executed with NonDex. Specifically speaking, we can reproduce them by using the following commands. ``` mvn clean install -DskipTests -Drat.skip=true ``` * Test shouldReadWriteEdge[expect(v2)] ```bash mvn clean -pl gremlin-util edu.illinois:nondex-maven-plugin:2.2.1:nondex -Dtest="org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONTypedCompatibilityTest#shouldReadWriteEdge[expect(v2)]" -Drat.skip=true ``` * Test shouldReadWriteEdge[expect(v3)] ```bash mvn clean -pl gremlin-util edu.illinois:nondex-maven-plugin:2.2.1:nondex -Dtest="org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONTypedCompatibilityTest#shouldReadWriteEdge[expect(v3)]" -Drat.skip=true ``` * Test shouldReadWritePath[expect(v2)] ```bash mvn clean -pl gremlin-util edu.illinois:nondex-maven-plugin:2.2.1:nondex -Dtest="org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONTypedCompatibilityTest#shouldReadWritePath[expect(v2)]" -Drat.skip=true ``` * Test shouldReadWritePath[expect(v3)] ```bash mvn clean -pl gremlin-util edu.illinois:nondex-maven-plugin:2.2.1:nondex -Dtest="org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONTypedCompatibilityTest#shouldReadWritePath[expect(v3)]" -Drat.skip=true ``` * Test shouldReadWriteProperty[expect(v2)] ```bash mvn clean -pl gremlin-util edu.illinois:nondex-maven-plugin:2.2.1:nondex -Dtest="org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONTypedCompatibilityTest#shouldReadWriteProperty[expect(v2)]" -Drat.skip=true ``` * Test shouldReadWriteProperty[expect(v3)] ```bash mvn clean -pl gremlin-util edu.illinois:nondex-maven-plugin:2.2.1:nondex -Dtest="org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONTypedCompatibilityTest#shouldReadWriteProperty[expect(v3)]" -Drat.skip=true ``` * Test shouldReadWriteTraverser[expect(v2)] ```bash mvn clean -pl gremlin-util edu.illinois:nondex-maven-plugin:2.2.1:nondex -Dtest="org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONTypedCompatibilityTest#shouldReadWriteTraverser[expect(v2)]" -Drat.skip=true ``` * Test shouldReadWriteTraverser[expect(v3)] ```bash mvn clean -pl gremlin-util edu.illinois:nondex-maven-plugin:2.2.1:nondex -Dtest="org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONTypedCompatibilityTest#shouldReadWriteTraverser[expect(v3)]" -Drat.skip=true ``` * Test shouldReadWriteVertexProperty[expect(v2)] ```bash mvn clean -pl gremlin-util edu.illinois:nondex-maven-plugin:2.2.1:nondex -Dtest="org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONTypedCompatibilityTest#shouldReadWriteVertexProperty[expect(v2)]" -Drat.skip=true ``` * Test shouldReadWriteVertexProperty[expect(v3)] ```bash mvn clean -pl gremlin-util edu.illinois:nondex-maven-plugin:2.2.1:nondex -Dtest="org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONTypedCompatibilityTest#shouldReadWriteVertexProperty[expect(v3)]" -Drat.skip=true ``` * Test shouldReadWriteVertex[expect(v2)] ```bash mvn clean -pl gremlin-util edu.illinois:nondex-maven-plugin:2.2.1:nondex -Dtest="org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONTypedCompatibilityTest#shouldReadWriteVertex[expect(v2)]" -Drat.skip=true ``` * Test shouldReadWriteVertex[expect(v3)] ```bash mvn clean -pl gremlin-util edu.illinois:nondex-maven-plugin:2.2.1:nondex -Dtest="org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONTypedCompatibilityTest#shouldReadWriteVertex[expect(v3)]" -Drat.skip=true ``` And the error should be something like this: ``` [ERROR] Failures: [ERROR] GraphSONTypedCompatibilityTest>AbstractTypedCompatibilityTest.shouldReadWriteEdge:322 expected:<e[17][7-develops->10]> but was:<e[13][1-develops->10]> [ERROR] GraphSONTypedCompatibilityTest>AbstractTypedCompatibilityTest.shouldReadWriteEdge:322 expected:<e[17][7-develops->10]> but was:<e[13][1-develops->10]> ``` Upon our investigation, the root cause is the use of: ``` protected Map<Object, Vertex> vertices = new ConcurrentHashMap<>(); protected Map<Object, Edge> edges = new ConcurrentHashMap<>(); ``` in TinkerGraph.java, which does not guarantee a deterministic order. The simplest fix would be to replace ConcurrentHashMap with LinkedHashMap, as we did in a previous PR. We've confirmed that this change could remove the flakiness of these tests. However, we are concerned that such a change might introduce unintended side effects in the code under test. Another possible fix would be to deterministically select a fixed id, but that approach would make the test become sensitive to future implementation changes. Thus, we decided to use this sorting-based approach to deflake this test.
1 parent c0962da commit 5364eee

File tree

1 file changed

+17
-6
lines changed
  • gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/structure/io

1 file changed

+17
-6
lines changed

gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/structure/io/Model.java

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import org.apache.tinkerpop.gremlin.structure.VertexProperty;
4141
import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerFactory;
4242
import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerGraph;
43+
import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
4344
import org.apache.tinkerpop.gremlin.util.message.RequestMessage;
4445
import org.apache.tinkerpop.gremlin.util.message.ResponseMessage;
4546

@@ -115,12 +116,22 @@ private Model() {
115116
addCoreEntry(new java.sql.Timestamp(1481750076295L), "Timestamp", "");
116117
addCoreEntry(UUID.fromString("41d2e28a-20a4-4ab0-b379-d810dede3786"), "UUID");
117118

118-
addGraphStructureEntry(graph.edges().next(), "Edge", "");
119-
addGraphStructureEntry(g.V().out().out().path().next(), "Path", "");
120-
addGraphStructureEntry(graph.edges().next().properties().next(), "Property", "");
119+
addGraphStructureEntry(IteratorUtils.list(graph.edges()).stream()
120+
.sorted((e1, e2) -> Integer.compare((Integer)e1.id(), (Integer)e2.id()))
121+
.iterator().next(), "Edge", "");
122+
addGraphStructureEntry(g.V().order().by(T.id).out().out().path().next(), "Path", "");
123+
addGraphStructureEntry(IteratorUtils.list(graph.edges()).stream()
124+
.sorted((e1, e2) -> Integer.compare((Integer)e1.id(), (Integer)e2.id()))
125+
.iterator().next().properties().next(), "Property", "");
121126
addGraphStructureEntry(graph, "TinkerGraph", "`TinkerGraph` has a custom serializer that is registered as part of the `TinkerIoRegistry`.");
122-
addGraphStructureEntry(graph.vertices().next(), "Vertex", "");
123-
addGraphStructureEntry(graph.vertices().next().properties().next(), "VertexProperty", "");
127+
addGraphStructureEntry(IteratorUtils.list(graph.vertices()).stream()
128+
.sorted((v1, v2) -> Integer.compare((Integer)v1.id(), (Integer)v2.id()))
129+
.iterator().next(), "Vertex", "");
130+
addGraphStructureEntry(IteratorUtils.list(IteratorUtils.list(graph.vertices()).stream()
131+
.sorted((v1, v2) -> Integer.compare((Integer)v1.id(), (Integer)v2.id()))
132+
.iterator().next().properties()).stream()
133+
.sorted((p1, p2) -> Long.compare((Long)p1.id(), (Long)p2.id()))
134+
.iterator().next(), "VertexProperty", "");
124135

125136
addGraphProcessEntry(SackFunctions.Barrier.normSack, "Barrier", "");
126137
addGraphProcessEntry(new Bytecode.Binding("x", 1), "Binding", "A \"Binding\" refers to a `Bytecode.Binding`.");
@@ -153,7 +164,7 @@ private Model() {
153164
// TextP was only added at 3.4.0 and is not supported with untyped GraphSON of any sort
154165
addGraphProcessEntry(TextP.containing("ark"), "TextP", "");
155166
addGraphProcessEntry(createStaticTraversalMetrics(), "TraversalMetrics", "");
156-
addGraphProcessEntry(g.V().hasLabel("person").asAdmin().nextTraverser(), "Traverser", "");
167+
addGraphProcessEntry(g.V().hasLabel("person").order().by(T.id).asAdmin().nextTraverser(), "Traverser", "");
157168

158169
final Map<String,Object> requestBindings = new HashMap<>();
159170
requestBindings.put("x", 1);

0 commit comments

Comments
 (0)