Github user spmallette commented on a diff in the pull request:
https://github.com/apache/incubator-tinkerpop/pull/333#discussion_r66292909
--- Diff:
gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/io/IoTest.java
---
@@ -721,14 +732,18 @@ public static void assertCrewGraph(final Graph g1,
final boolean lossyForId) {
}
public static void assertClassicGraph(final Graph g1, final boolean
assertDouble, final boolean lossyForId) {
- assertToyGraph(g1, assertDouble, lossyForId, false);
+ assertToyGraph(g1, assertDouble, lossyForId, false, true);
+ }
+
+ public static void assertNoEdgeGraph(final Graph g1, final boolean
assertDouble, final boolean lossyForId) {
+ assertToyGraph(g1, assertDouble, lossyForId, false, false);
}
public static void assertModernGraph(final Graph g1, final boolean
assertDouble, final boolean lossyForId) {
- assertToyGraph(g1, assertDouble, lossyForId, true);
+ assertToyGraph(g1, assertDouble, lossyForId, true, true);
}
- private static void assertToyGraph(final Graph g1, final boolean
assertDouble, final boolean lossyForId, final boolean assertSpecificLabel) {
+ private static void assertToyGraph(final Graph g1, final boolean
assertDouble, final boolean lossyForId, final boolean assertSpecificLabel,
final boolean assertEdgeLabel) {
--- End diff --
I'm sorry, but I don't think I like that you altered this assertion method.
The toy graph has important meaning in TinkerPop and there really is no valid
version of it that does not have edge labels. It essentially ceases to be the
"toy graph" without those labels. Your test graph for "GraphML with no edges"
really doesn't need to validate the entire toy graph to be effective in what it
is testing. It just needs to validate that a default edge label is used when
the label isn't supplied.
I would much prefer to see you produce a new simple
"tinkerpop-no-edge-labels.xml" (you could even just take the existing toy graph
and strip out all but two vertices and an edge) and just assert that in your
test method itself. I think that would be preferable to altering
`assertToyGraph()` in a way that no other test would really have benefit in
using.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
