[TINKERPOP-2972] ProjectStep does not allow keys specified in a query #2217
Conversation
Codecov Report
@@ Coverage Diff @@
## 3.6-dev #2217 +/- ##
=============================================
- Coverage 75.23% 75.13% -0.11%
+ Complexity 12278 12264 -14
=============================================
Files 1054 1054
Lines 63294 63296 +2
Branches 6913 6914 +1
=============================================
- Hits 47617 47555 -62
- Misses 13112 13182 +70
+ Partials 2565 2559 -6
... and 11 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
rdtr
force-pushed
the
TINKERPOP-2972
branch
2 times, most recently
from
7cf4a81 to
ce98e49
Compare
|
Thanks for putting this together. Overall the change looks good to me. If possible it would be nice to have a feature test for this just to ensure there are no issues in the GLV's. A good example of feature tests catching errors can be found here: |
rdtr
force-pushed
the
TINKERPOP-2972
branch
3 times, most recently
from
97487f8 to
fdb199a
Compare
|
VOTE +1 |
|
|
||
|
|
||
| Scenario: g_V_projectXa_aX |
There was a problem hiding this comment.
Looks like the Gherkin tests can't handle cases that fast fail on step construction (I believe it needs the step to be successfully constructed before translating it into the GLV tests). I don't think there is any easy work around currently, sorry @rdtr, you might need to remove this test and just have unit tests.
(As an aside, the test is missing a When iterated to list, but that's not really important now that this test won't really work in Gherkin).
There was a problem hiding this comment.
Actually there is a quick workaround I just remembered. You can technically add static translation of this test to all the feature tests in the generate.groovy file of each GLV, but it's ultimately a hack so I'd think it's ok to not include it at this point.
There was a problem hiding this comment.
Sorry I missed that this scenario is incompatible with the feature tests.
VOTE +1 (with failing feature test removed)
There was a problem hiding this comment.
Ok, I deleted the feature test. Please check when you have time, thanks !
rdtr
force-pushed
the
TINKERPOP-2972
branch
from
fdb199a to
c64573f
Compare
ProjectStep should not accept a duplicate key, anyway doing so would not make much sense while the implementation for Graph provider would be complicated because of the logic to determine what is the actual traversal to be mapped to the key.