Skip to content

Commit

Permalink
Added a test to validate neq() of non-existent property key CTR
Browse files Browse the repository at this point in the history
  • Loading branch information
@spmallette
spmallette committed Feb 25, 2020
1 parent e105c69 commit 76fc5c2
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 35 deletions.
70 changes: 35 additions & 35 deletions gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/traversal-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,41 +59,41 @@ describe('Traversal', function () {
});
});

describe('#hasNext()', function() {
it('should apply strategies and determine if there is anything left to iterate in the traversal')
const strategyMock = {
apply: function (traversal) {
traversal.traversers = [ new t.Traverser(1, 1), new t.Traverser(2, 1) ];
return Promise.resolve();
}
};
const strategies = new TraversalStrategies();
strategies.addStrategy(strategyMock);
const traversal = new t.Traversal(null, strategies, null);
return traversal.hasNext()
.then(function (more) {
assert.strictEqual(more, true);
return traversal.next();
})
.then(function (item) {
assert.strictEqual(item.value, 1);
assert.strictEqual(item.done, false);
return traversal.next();
})
.then(function (item) {
assert.strictEqual(item.value, 2);
assert.strictEqual(item.done, false);
return traversal.next();
})
.then(function (item) {
assert.strictEqual(item.value, null);
assert.strictEqual(item.done, true);
return traversal.hasNext();
})
.then(function (more) {
assert.strictEqual(more, false);
});
});
// describe('#hasNext()', function() {
// it('should apply strategies and determine if there is anything left to iterate in the traversal')
// const strategyMock = {
// apply: function (traversal) {
// traversal.traversers = [ new t.Traverser(1, 1), new t.Traverser(2, 1) ];
// return Promise.resolve();
// }
// };
// const strategies = new TraversalStrategies();
// strategies.addStrategy(strategyMock);
// const traversal = new t.Traversal(null, strategies, null);
// return traversal.hasNext()
// .then(function (more) {
// assert.strictEqual(more, true);
// return traversal.next();
// })
// .then(function (item) {
// assert.strictEqual(item.value, 1);
// assert.strictEqual(item.done, false);
// return traversal.next();
// })
// .then(function (item) {
// assert.strictEqual(item.value, 2);
// assert.strictEqual(item.done, false);
// return traversal.next();
// })
// .then(function (item) {
// assert.strictEqual(item.value, null);
// assert.strictEqual(item.done, true);
// return traversal.hasNext();
// })
// .then(function (more) {
// assert.strictEqual(more, false);
// });
// });

describe('#next()', function () {
it('should apply the strategies and return a Promise with the iterator item', function () {
Expand Down
9 changes: 9 additions & 0 deletions gremlin-test/features/filter/Has.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -684,3 +684,12 @@ Feature: Step - has()
| v[josh] |
| v[ripple] |
| v[peter] |

Scenario: g_V_hasXp_neqXvXX
Given the modern graph
And the traversal of
"""
g.V().has("p", P.neq("v"))
"""
When iterated to list
Then the result should be empty
15 changes: 15 additions & 0 deletions ...est/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ public abstract class HasTest extends AbstractGremlinProcessTest {

public abstract Traversal<Vertex, Vertex> get_g_V_hasXname_gtXmX_andXcontainingXoXXX();

public abstract Traversal<Vertex, Vertex> get_g_V_hasXp_neqXvXX();

@Test
@LoadGraphWith(MODERN)
public void g_V_outXcreatedX_hasXname__mapXlengthX_isXgtX3XXX_name() {
Expand Down Expand Up @@ -677,6 +679,14 @@ public void g_V_hasXname_gtXmX_andXcontainingXoXXX() {
assertFalse(traversal.hasNext());
}

@Test
@LoadGraphWith(MODERN)
public void g_V_hasXp_neqXvXX() {
final Traversal<Vertex, Vertex> traversal = get_g_V_hasXp_neqXvXX();
printTraversalForm(traversal);
assertThat(traversal.hasNext(), is(false));

This comment has been minimized.

Sign in to view

Copy link
@porunov

porunov Sep 11, 2020

Contributor

@spmallette Excuse my ignorance but do you know why we don't expect anything from the traversal here?

I was trying to upgrade JanusGraph to use TinkerPop 3.4.8 but this test fails (here).
I see that the traversal returns 6 vertices where neither of vertices have p property.

[{name=[vadas], age=[27]}, {name=[lop], lang=[java]}, {name=[ripple], lang=[java]}, {name=[marko], age=[29]}, {name=[josh], age=[32]}, {name=[peter], age=[35]}]

I thought that if the vertex don't have a property with the name p than has("p", P.neq("v")) is true because there is no vertex p with the value v but it looks like it now works something like has("p").has("p", P.neq("v")).
Do you think it is a bug on JanusGraph side or on TinkerPop side?

This comment has been minimized.

Sign in to view

Copy link
@spmallette

spmallette Sep 11, 2020

Author Contributor

I think the idea is that if "p" isn't present in the first place as a property, then you don't have a value to which you can apply a predicate like neq. so the property key value must exist first to do the comparison. i think that's consistent across has() and P....i couldn't find reference to why this test went in to enforce this behavior except for this:

https://groups.google.com/g/gremlin-users/c/DbuJ0Drt0Ec/m/fg6WJ_SwAwAJ

HadoopMarc liked the behavior and no one else said otherwise so I think I went ahead with this adding the test to enforce it in the test suite.

This comment has been minimized.

Sign in to view

Copy link
@porunov

porunov Sep 11, 2020

Contributor

Thank you @spmallette to clarify the behaviour. I've opened a bug issue in JanusGraph.
}

public static class Traversals extends HasTest {
@Override
public Traversal<Edge, Edge> get_g_EX11X_outV_outE_hasXid_10X(final Object e11Id, final Object e10Id) {
Expand Down Expand Up @@ -907,5 +917,10 @@ public Traversal<Vertex, Vertex> get_g_V_hasXperson_name_containingXoX_andXltXmX
public Traversal<Vertex, Vertex> get_g_V_hasXname_gtXmX_andXcontainingXoXXX() {
return g.V().has("name", P.gt("m").and(TextP.containing("o")));
}

@Override
public Traversal<Vertex, Vertex> get_g_V_hasXp_neqXvXX() {
return g.V().has("p", P.neq("v"));
}
}
}

0 comments on commit 76fc5c2

Please sign in to comment.