FlorianHockmann commented on PR #2565:
URL: https://github.com/apache/tinkerpop/pull/2565#issuecomment-2079581200

   The problem is however that the Gherkin tests already include this step but 
they already passed before my change here as @spmallette noted in the issue 
description.
   This got me curious however as I was wondering how this could actually work 
without an overload taking a dictionary in C# since I couldn't get it to work 
in C# without my change here. (I did write a test case when I implemented this. 
I only deleted it before committing to not impact other tests.)
   
   Stephens assumption was:
   
   > It works because it probably piggybacks on property(object?, object?, 
objects[]?) which has all nullable arguments.
   
   Turns out that this is not the case. The `DotNetTranslator` produces a C# 
traversal with individual `Property` steps for each map entry: 
https://github.com/apache/tinkerpop/blob/e3ced38b2108f3edb29f11fbfa4f6950427bcbd9/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs#L539
   
   So, as a next attempt I wanted to change the translator to instead produce a 
C# traversal using the new `Dictionary` overload.
   But then I learned that that's now even possible since Gremlin-Java already 
produces Bytecode without this overload:
   
https://github.com/apache/tinkerpop/blob/e3ced38b2108f3edb29f11fbfa4f6950427bcbd9/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java#L3221-L3234
   
   and the translator only gets the Bytecode produced from the traversal.
   
   So I don't see any elegant way to test this using Gherkin tests without 
changing the Gremlin-Java overload (which is a bigger change than what makes 
sense here in my opinion). We could add a hard-coded translation for one 
feature test to ensure that the traversal in C# will use the new overload, but 
I don't really like maintaining such hard-coded translations.
   
   However, this got me to the idea to instead use the translation from C# to 
Groovy for a test. I just pushed an updated commit with such a test. The test 
does not compile without the added overload and the translation also includes 
the successful construction of Bytecode from the traversal as the translator 
also uses the Bytecode.
   
   


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