spmallette commented on code in PR #3211: URL: https://github.com/apache/tinkerpop/pull/3211#discussion_r2378907910
########## gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/filter/TypeOf.feature: ########## @@ -0,0 +1,630 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +@StepClassFilter @StepIs +Feature: Predicate - typeOf() + + Scenario: g_V_valuesXageX_isXtypeOfXGType_INTXX + Given the modern graph + And the traversal of + """ + g.V().values("age").is(P.typeOf(GType.INT)) + """ + When iterated to list + Then the result should be unordered + | result | + | d[29].i | + | d[27].i | + | d[32].i | + | d[35].i | + + Scenario: g_V_valuesXnameX_isXtypeOfXGType_STRINGXX + Given the modern graph + And the traversal of + """ + g.V().values("name").is(P.typeOf(GType.STRING)) + """ + When iterated to list + Then the result should be unordered + | result | + | marko | + | vadas | + | lop | + | josh | + | ripple | + | peter | + + Scenario: g_V_isXtypeOfXGType_VERTEXXX_count + Given the modern graph + And the traversal of + """ + g.V().is(P.typeOf(GType.VERTEX)).count() + """ + When iterated to list + Then the result should be unordered + | result | + | d[6].l | + + Scenario: g_E_isXtypeOfXGType_EDGEXX_count + Given the modern graph + And the traversal of + """ + g.E().is(P.typeOf(GType.EDGE)).count() + """ + When iterated to list + Then the result should be unordered + | result | + | d[6].l | + + Scenario: g_V_propertiesXnameX_isXtypeOfXGType_PROPERTYXX_count + Given the modern graph + And the traversal of + """ + g.V().properties("name").is(P.typeOf(GType.PROPERTY)).count() + """ + When iterated to list + Then the result should be unordered + | result | + | d[6].l | + + Scenario: g_V_valuesXageX_isXtypeOfXGType_NUMBERXX + Given the modern graph + And the traversal of + """ + g.V().values("age").is(P.typeOf(GType.NUMBER)) + """ + When iterated to list + Then the result should be unordered + | result | + | d[29].i | + | d[27].i | + | d[32].i | + | d[35].i | + + Scenario: g_V_valuesXageX_isXtypeOfXGType_BYTEXX Review Comment: testing a lot of negatives here which is good. where are the feature tests for positives for all these types? By defining types for Gremlin in `GType` I feel like we've elevated those selected ones to have special meaning. I understand none of our example graphs have that kind of data at this point, but I think that's a call to expand what our test framework can do. We already have the kitchen sink graph for this sort of thing. I think you should expand that dataset to throw in some data for this. Maybe one vertex label per testable data type (that can be stored as a property), so that you could do `g.V().hasLabel("double", "string").is(typeOf(NUMBER))` and assert the right stuff. I didn't examine this carefully, but i don't see positive tests for GRAPH and TREE. What else is missing? Perhaps it would be wise to break this feature test up into: `TypeOfNumber.feature`, `TypeOfGraph.feature`, etc. for some specific testing of grouped types then keep `TypeOf.feature` as a catch-all for everything else? -- 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]
