[GitHub] tinkerpop issue #535: TINKERPOP-1601 LazyBarrierStrategy should not check is...

2017-01-18 Thread okram
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/535 Either works, but I'm leaning more towards (a). I would do something like: Assume the traversal: `g.V().out().values('name').count()` The test algorithm would: 1. I

[GitHub] tinkerpop issue #535: TINKERPOP-1601 LazyBarrierStrategy should not check is...

2017-01-18 Thread dkuppitz
Github user dkuppitz commented on the issue: https://github.com/apache/tinkerpop/pull/535 So do you suggest a) to remove the assertions that check the metrics for specific steps or b) to find the indexes of all steps that were present before strategies were applied? --- If your proj

[GitHub] tinkerpop issue #535: TINKERPOP-1601 LazyBarrierStrategy should not check is...

2017-01-18 Thread okram
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/535 This is not the way that this should be done. We shouldn't remove any strategies. What we should do is generalize the `ProfileTests` so they are not so specific about step indexes and the like. For

[GitHub] tinkerpop issue #535: TINKERPOP-1601 LazyBarrierStrategy should not check is...

2017-01-17 Thread dkuppitz
Github user dkuppitz commented on the issue: https://github.com/apache/tinkerpop/pull/535 The setup method now removes `LazyBarrierStrategy` and all provider specific strategies. We can't remove all strategies, since certain test suites use a different set of extra strategies that are

[GitHub] tinkerpop issue #535: TINKERPOP-1601 LazyBarrierStrategy should not check is...

2017-01-17 Thread okram
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/535 I was thinking more about this specific problem. The real problem is `ProfileTest`, not `LazyBarrierStrategy`. We need to generalize all the `ProfileTest` test cases such that they not be concerned

[GitHub] tinkerpop issue #535: TINKERPOP-1601 LazyBarrierStrategy should not check is...

2017-01-17 Thread dkuppitz
Github user dkuppitz commented on the issue: https://github.com/apache/tinkerpop/pull/535 That was my plan. kinda. I was going to open a new ticket to get rid of `is.testing` altogether. --- If your project is set up for it, you can reply to this email and have your reply appear on G

[GitHub] tinkerpop issue #535: TINKERPOP-1601 LazyBarrierStrategy should not check is...

2017-01-17 Thread okram
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/535 There are numerous areas where `is.testing` is used for strategies. We shouldn't just do this for one of them and push a PR. We should overhaul the entire system so we don't have some parts of the c