You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by spmallette <gi...@git.apache.org> on 2015/10/24 12:43:06 UTC
[GitHub] incubator-tinkerpop pull request: TINKERPOP3-903 - amend a test
GitHub user spmallette opened a pull request:
https://github.com/apache/incubator-tinkerpop/pull/120
TINKERPOP3-903 - amend a test
Add a test to validate that results of all sizes stream properly.
This is an amendment to
https://issues.apache.org/jira/browse/TINKERPOP3-903
which was already merged and closed. As @rjbriody was describing the problem he was seeing, it would be cool if he could review this test to see if it is sufficient for covering the problems he had. I'd like to have more tests in this area if he can think of holes in this one. We can vote to merge once we get his :+1:
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-tinkerpop TINKERPOP3-903
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-tinkerpop/pull/120.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #120
----
commit 173228e776ea88b5856d0f02cd4c2070e8f5463c
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2015-10-24T10:38:35Z
TINKERPOP3-903 Add a test to validate that results of all sizes stream properly.
This is an amendment to TINKERPOP-903 which was already merged and closed.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP3-903 - amend a test
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-tinkerpop/pull/120
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP3-903 - amend a test
Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/120#issuecomment-151214202
Ok, there's nothing else wrong with the test, I just wanted to point out, that it *probably* does a lot of unnecessary stuff. If it's needed, cool.
Vote: +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP3-903 - amend a test
Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/120#issuecomment-150831242
Looks like the script does a lot of things that are not really required for the test. I think we should save some CPU cycles wherever we can, since the tests already take long enough.
```groovy
def rand = new Random()
def ii = 0
def ids = [graph.addVertex().id()]
while (ii++ < size) {
def v = graph.addVertex()
def u = graph.vertices(ids.get(rand.nextInt(ids.size()))).next()
v.addEdge('linked', u)
ids += [v, u]*.id()
}
g.V()
```
In my opinion that should be sufficient.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP3-903 - amend a test
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/120#issuecomment-151346232
VOTE +1.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP3-903 - amend a test
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/120#issuecomment-151341707
I like that @dkuppitz looked at this with a discerning eye and questioned the motives of the test, but I guess i could have CTR'd this one. As we're already in the vote, @okram can you just have a look real quick and vote?
Since @rjbriody is good with the test and it passes nicely:
VOTE: +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP3-903 - amend a test
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/120#issuecomment-151000777
I'll let @rjbriody say if your change is ok @dkuppitz but I think having properties were an important part of what was required in the test. Note that this test only runs on TinkerGraph in the integration tests for gremlin server so it's pretty fast (seconds).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---