Cole-Greer commented on code in PR #3717:
URL: https://github.com/apache/tinkerpop/pull/3717#discussion_r4276147797


##########
gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/CountStrategyTest.java:
##########
@@ -84,15 +84,46 @@ public static Iterable<Object[]> generateTestParameters() {
                     {__.filter(__.out().out().count().is(0)), 
__.not(__.out().out())},
                     {__.local(aggregate("x")).count().is(0).as("a"), 
__.local(aggregate("x")).limit(1).count().is(0).as("a")},
                     {__.out().count().as("a").is(0), 
__.out().limit(1).count().as("a").is(0)},
-                    {__.out().count().is(neq(4)), 
__.out().limit(5).count().is(neq(4))},

Review Comment:
   For each test case we are updating by removing the `limit()`, I think I'd 
like to reintroduce a new case with a similar traversal and predicate, but 
where we still expect a limit to be added. In this case, it could be something 
like this:
   ```
   {__.out().where(__.count().is(neq(4))), 
__.out().where(__.limit(5).count().is(neq(4)))},
   ```
   
   I'd like to retain decent coverage of "positive" cases where CountStrategy 
is still productive following this change.



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