divijvaidya commented on a change in pull request #1545:
URL: https://github.com/apache/tinkerpop/pull/1545#discussion_r782906051
##########
File path:
gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/feature-steps.js
##########
@@ -298,6 +305,17 @@ function toPath(value) {
return new Path(new Array(0), parts.map(x => parseValue.call(this, x)));
}
+/*
+ * TODO Implement me.
Review comment:
Please create a JIRA and add here
##########
File path:
gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/AbstractRemoteGraphProvider.java
##########
@@ -206,6 +206,10 @@
test =
"org.apache.tinkerpop.gremlin.process.traversal.step.map.UnfoldTest",
method = "g_V_valueMap_unfold_mapXkeyX",
reason = "Tests that include lambdas are not supported by the test
suite for remotes")
[email protected](
+ test =
"org.apache.tinkerpop.gremlin.process.traversal.step.OrderabilityTest",
+ method = "g_inject_order_with_unknown_type",
+ reason = "Tests that inject a generic Java Object are not supported by
the test suite for remotes")
Review comment:
Nice. We can use same reason for at rest of the places.
##########
File path:
gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/GherkinTestRunner.cs
##########
@@ -85,6 +85,8 @@ public class GherkinTestRunner
"g_V_hasXperson_name_markoX_bothXknowsX_groupCount_byXvaluesXnameX_foldX",
IgnoreReason.ArrayKeysInMapNotAssertingInGherkin
},
+ {"g_V_properties_order", IgnoreReason.NoReason},
Review comment:
Please add a reason for Ignore here. A comment should be sufficient.
##########
File path:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/Graph.java
##########
@@ -61,13 +61,13 @@
/**
* Configuration key used by {@link GraphFactory}} to determine which
graph to instantiate.
*/
- public static final String GRAPH = "gremlin.graph";
+ String GRAPH = "gremlin.graph";
/**
* This should only be used by providers to create keys, labels, etc. in a
namespace safe from users.
* Users are not allowed to generate property keys, step labels, etc. that
are key'd "hidden".
*/
- public static class Hidden {
+ class Hidden {
Review comment:
Restricting the scope is a backward incompatible change since vendors
might be extending this class. Please revert back to public scope.
Same for other changes in this file.
##########
File path:
tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/io/graphson/AbstractTinkerGraphGraphSONTranslatorProvider.java
##########
@@ -242,12 +242,24 @@ public GraphTraversalSource traversal(final Graph graph) {
return g.withStrategies(new TranslationStrategy(g, new
GraphSONTranslator<>(JavaTranslator.of(g), version), true));
}
+ @Graph.OptOut(
+ test =
"org.apache.tinkerpop.gremlin.process.traversal.step.OrderabilityTest",
+ method = "g_inject_order",
+ reason = "GraphSONv2 does not properly round trip Maps and Sets")
Review comment:
This might not be important for 3.6.x since GraphSonV3 is supposed to be
used here but we still haven't deprecated graphsonv2 hence, it requires some
attention. Please create a JIRA for this. We will have to investigate what's
wrong with graphsonv2 and document the shortcoming.
##########
File path:
gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/jsr223/GroovyTranslatorProvider.java
##########
@@ -215,6 +215,10 @@
test =
"org.apache.tinkerpop.gremlin.process.traversal.step.map.WriteTest",
method = "*",
reason = "read and write tests don't translate locally well because of
calling iterate() inside read()/write() add a none()")
[email protected](
+ test =
"org.apache.tinkerpop.gremlin.process.traversal.step.OrderabilityTest",
+ method = "g_inject_order_with_unknown_type",
+ reason = "Cannot round trip a generic Java Object")
Review comment:
Maybe update the reason here to say something about how this doesn't
work for remote. To a layman code reader, this leaves a cliffhanger about why
cannot be roundtrip generic objects here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
