Code review comment for lp:~athomason/libmemcached/test-big-binary-mget

Trond Norbye wrote:
> Review: Needs Fixing
> 1) The test case doesn't use the memcached servers started by the test suite, but tries to connect to localhost:11211. You should connect to the servers started by the test suite instead.
Actually not, it's 11221, the first one the tests start. I didn't see an
obvious bit of what I wanted to do elsewhere in the tests, but point taken.
> If you want to limit the number of servers being used, you should create a clone with memcached_clone and set memc_clone->number_of_hosts to the number of hosts you want (BUT ADD A COMMENT THAT THIS IS A DIRTY HACK JUST USED IN THE TEST FRAMEWORK AND SHOULDN'T BE USED OUTSIDE THE TEST FRAMEWORK!!)
>
No need to yell at your own suggestion ;). The number of servers is
actually irrelevant, I just need a fresh memcached_st. memcached_clone
does seem simpler and more correct, so I'll use that.
> 2) Test cases should return TEST_SUCCESS/TEST_FAIL and not MEMCACHED_SUCCESS (different enums)
>
Sure. To prevent future annoyances I'd suggest cleaning up the rest of
the tests to do the same since newbies like me are likely to just copy
surrounding examples :).
> 3) You can use calloc instead of malloc followed by a memset call, and you can assign a void* to everything according to C99 standard so you don't need any casting there...
>
Similarly, that's code just borrowed from user_supplied_bug3. I'll fix
up all the malloc/memset instances that I see.