Uploaded image for project: 'TinkerPop'
  1. TinkerPop
  2. TINKERPOP-985

shouldPersistDataOnClose makes incorrect feature check

        Details

        • Type: Bug
        • Status: Closed
        • Priority: Minor
        • Resolution: Fixed
        • Affects Version/s: 3.0.2-incubating
        • Fix Version/s: 3.1.1-incubating
        • Component/s: test-suite
        • Labels:
          None

          Description

          Lines 1206-1211 in GraphTest.java (gremlin-test):

          final Vertex v = graph.addVertex();
          final Vertex u = graph.addVertex();
          if (graph.features().edge().properties().supportsStringValues())

          { v.property(VertexProperty.Cardinality.single, "name", "marko"); u.property(VertexProperty.Cardinality.single, "name", "pavel"); }

          The check tests for string value properties on edges, but it should be checking string values on properties of vertices.

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            spmallette stephen mallette added a comment -

            yup -that looks like a bug there - do you want to submit a small pull request to fix that? if not, i can just tack that in there. please let me know.

            Show
            spmallette stephen mallette added a comment - yup -that looks like a bug there - do you want to submit a small pull request to fix that? if not, i can just tack that in there. please let me know.
            Hide
            Permalink
            Ellitron Jonathan Ellithorpe added a comment -

            Yup, just submitted a pull request.

            In the commit I included some additional notes about the feature requirements checking for this method (primarily, that the check for ADD_PROPERTY support on vertices is implicit, because the SIMPLE feature set doesn't check for that directly, but rather for being able to add meta properties).

            Also, I just saw in the SIMPLE feature set that a check is made for Graph.Features.VertexPropertyFeatures.FEATURE_STRING_VALUES. Is this if statement check in the unit test code itself even needed then in the first place? If not, perhaps the right thing to do is to delete it all together (in which case you'll want to disregard my PR which simply fixes the if statement check).

            Jonathan

            Show
            Ellitron Jonathan Ellithorpe added a comment - Yup, just submitted a pull request. In the commit I included some additional notes about the feature requirements checking for this method (primarily, that the check for ADD_PROPERTY support on vertices is implicit, because the SIMPLE feature set doesn't check for that directly, but rather for being able to add meta properties). Also, I just saw in the SIMPLE feature set that a check is made for Graph.Features.VertexPropertyFeatures.FEATURE_STRING_VALUES. Is this if statement check in the unit test code itself even needed then in the first place? If not, perhaps the right thing to do is to delete it all together (in which case you'll want to disregard my PR which simply fixes the if statement check). Jonathan
            Hide
            Permalink
            spmallette stephen mallette added a comment -

            the fix for this was merged on this issue: https://github.com/apache/incubator-tinkerpop/pull/152

            Show
            spmallette stephen mallette added a comment - the fix for this was merged on this issue: https://github.com/apache/incubator-tinkerpop/pull/152

              People

              • Assignee:
                spmallette stephen mallette
                Reporter:
                Ellitron Jonathan Ellithorpe
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Development