Fix MEMCACHED_SRV support #295
Conversation
This did not work in practice due to a mix up of
func example(serverList memcache.ServerList)
and
func example(serverList *memcache.ServerList)
The code used the first case and did not correctly update a given
memcache.ServerList instance, instead a new one was created, updated
then forgot.
Signed-off-by: Peter Marsh <pmarsh@spotify.com>
bfe2e75 to
361e3ed
Compare
Agreed. Is there any kind of test we can add to catch this? |
|
@nezdolik @mattklein123 I do completely agree with you, this code path is tricky to test as it does DNS lookups and I'm really not sure how to go about writing tests for it (I have fairly limited go experience). I guess mocking would be the way, if you have any pointers or links that would be useful I can go away and try to add some. This is currently running in production and does fix the broken code that's currently in main so perhaps it is worth merging? But, genuinely I am happy to write some tests I just need some help :) Edit: Possibly a library like this? |
|
We do have mocking support already used in this library, but I'm not sure exactly the best way to refactor this code to make it mockable. I'm OK merging this for now but if you can try to follow up with testing that would be appreciated. cc @ysawa0 who might have better ideas. |
This did not work in practice due to a mix up of
func example(serverList memcache.ServerList)
and
func example(serverList *memcache.ServerList)
The code used the first case and did not correctly update a given
memcache.ServerList instance, instead a new one was created, updated
then forgot.
Signed-off-by: Peter Marsh <pmarsh@spotify.com>
This did not work in practice due to a mix up of
and
The code used the first case and did not correctly update a given
memcache.ServerList instance, instead a new one was created, updated
then forgot.