-
Notifications
You must be signed in to change notification settings - Fork 838
Fix Flaky Tests in GraphSONTypedCompatibilityTest #3237
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.
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
base: 3.7-dev
Are you sure you want to change the base?
Fix Flaky Tests in GraphSONTypedCompatibilityTest #3237
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 3.7-dev #3237 +/- ##
=============================================
+ Coverage 76.14% 76.23% +0.09%
- Complexity 13152 13306 +154
=============================================
Files 1084 1092 +8
Lines 65160 67657 +2497
Branches 7285 7378 +93
=============================================
+ Hits 49616 51581 +1965
- Misses 12839 13330 +491
- Partials 2705 2746 +41 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
addGraphStructureEntry(g.V().order().by(T.id).out().out().path().next(), "Path", ""); | ||
addGraphStructureEntry(IteratorUtils.list(graph.edges()).stream() | ||
.sorted((e1, e2) -> Integer.compare((Integer)e1.id(), (Integer)e2.id())) | ||
.iterator().next().properties().next(), "Property", ""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the properties be sorted as well?
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.
5364eee
to
f9f7514
Compare
VOTE +1 |
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 -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
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
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
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
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
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
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
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
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
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
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
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:
Upon our investigation, the root cause is the use of:
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.