Cole-Greer commented on code in PR #3055:
URL: https://github.com/apache/tinkerpop/pull/3055#discussion_r1964439585


##########
gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/MergeEdge.feature:
##########
@@ -947,4 +947,45 @@ Feature: Step - mergeE()
     And the graph should return 7 for count of "g.E()"
     And the graph should return 7 for count of "g.E().hasNot(\"created\")"
     And the graph should return 1 for count of 
"g.V().has(\"person\",\"name\",\"marko\").outE(\"knows\").hasNot(\"created\").inV().has(\"person\",\"name\",\"vadas\")"
-    And the graph should return 1 for count of 
"g.V().has(\"person\",\"name\",\"vadas\").outE(\"self\").hasNot('weight').inV().has(\"person\",\"name\",\"vadas\")"
\ No newline at end of file
+    And the graph should return 1 for count of 
"g.V().has(\"person\",\"name\",\"vadas\").outE(\"self\").hasNot('weight').inV().has(\"person\",\"name\",\"vadas\")"
+
+  @AllowNullPropertyValues
+  Scenario: 
g_mergeEXlabel_knows_out_marko_in_vadasX_optionXonMatch_weight_nullX_allowed
+    Given the empty graph
+    And the graph initializer of
+      """
+      g.addV("person").property("name", "marko").as("a").
+        addV("person").property("name", "vadas").as("b").
+        addE("knows").from("a").to("b").property("weight",1.0d)
+      """
+    And using the parameter xx1 defined as "m[{\"t[label]\": \"knows\", 
\"D[OUT]\":\"v[marko]\", \"D[IN]\":\"v[vadas]\"}]"
+    And using the parameter xx2 defined as "m[{\"weight\":null}]"
+    And the traversal of
+      """
+      g.mergeE(xx1).option(Merge.onMatch,xx2)
+      """
+    When iterated to list
+    Then the result should have a count of 1
+    And the graph should return 2 for count of "g.V()"
+    And the graph should return 1 for count of "g.E().hasLabel(\"knows\")"
+    And the graph should return 1 for count of 
"g.E().hasLabel(\"knows\").has(\"weight\",null)"
+
+  Scenario: 
g_mergeEXlabel_knows_out_marko_in_vadasX_optionXonMatch_weight_nullX
+    Given the empty graph
+    And the graph initializer of
+      """
+      g.addV("person").property("name", "marko").as("a").
+        addV("person").property("name", "vadas").as("b").
+        addE("knows").from("a").to("b").property("weight",1.0d)
+      """
+    And using the parameter xx1 defined as "m[{\"t[label]\": \"knows\", 
\"D[OUT]\":\"v[marko]\", \"D[IN]\":\"v[vadas]\"}]"
+    And using the parameter xx2 defined as "m[{\"weight\":null}]"
+    And the traversal of
+      """
+      g.mergeE(xx1).option(Merge.onMatch,xx2)
+      """
+    When iterated to list
+    Then the result should have a count of 1
+    And the graph should return 2 for count of "g.V()"
+    And the graph should return 1 for count of "g.E().hasLabel(\"knows\")"
+    And the graph should return 0 for count of 
"g.E().hasLabel(\"knows\").has(\"weight\")"

Review Comment:
   I'm going to double down and say [that 
scenario](https://github.com/apache/tinkerpop/blob/master/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/AddVertex.feature#L151)
 also needs a new tag for skipping. Unless I'm misunderstanding something, a 
TinkerGraph configured to allow null properties will fail the last assertion in 
that test (`And the graph should return 2 for count of 
"g.V().properties(\"name\")"`)
   
   ```
   gremlin> config = new 
MapConfiguration(["gremlin.tinkergraph.allowNullPropertyValues": true])
   ==>MapConfiguration [map={gremlin.tinkergraph.allowNullPropertyValues=true}, 
trimmingDisabled=false]
   gremlin> graph = new TinkerGraph(config)
   ==>tinkergraph[vertices:0 edges:0]
   gremlin> g = graph.traversal()
   
   ==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard]
   gremlin> g.addV("person").property("name", "marko").property("age", 
29).as("marko").
   ......1>         addV("person").property("name", "vadas").property("age", 
27).as("vadas").
   ......2>         addV("software").property("name", "lop").property("lang", 
"java").as("lop").
   ......3>         addV("person").property("name","josh").property("age", 
32).as("josh").
   ......4>         addV("software").property("name", 
"ripple").property("lang", "java").as("ripple").
   ......5>         addV("person").property("name", "peter").property("age", 
35).as('peter').
   ......6>         addE("knows").from("marko").to("vadas").property("weight", 
0.5d).
   ......7>         addE("knows").from("marko").to("josh").property("weight", 
1.0d).
   ......8>         addE("created").from("marko").to("lop").property("weight", 
0.4d).
   ......9>         
addE("created").from("josh").to("ripple").property("weight", 1.0d).
   .....10>         addE("created").from("josh").to("lop").property("weight", 
0.4d).
   .....11>         addE("created").from("peter").to("lop").property("weight", 
0.2d)
   ==>e[23][15-created->6]
   
   gremlin> g.V().hasLabel("person").property(single, "name", null)
   ==>v[0]
   ==>v[3]
   ==>v[9]
   ==>v[15]
   
   gremlin> g.V().properties("name")
   ==>vp[name->null]
   ==>vp[name->null]
   ==>vp[name->lop]
   ==>vp[name->null]
   ==>vp[name->ripple]
   ==>vp[name->null]
   ```



-- 
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]

Reply via email to