Covering functions that make network requests

30 views
Skip to first unread message

Iskandar Pashayev

unread,
Jan 23, 2019, 11:02:40 AM1/23/19
to biocommons-devs
I've been looking into the cli module to increase its test coverage, and had a question about covering the `pull`/`list-remote-instances` functions.  Those functions make network requests through the `rsync` executable. A really simple way to cover them would be to just send a request over the network to the default remote host (dl.biocommons.org), but that doesn't seem like good practice.

I was thinking of making a local `test_remote_data` directory and then launching a subprocess during the test that could serve it over localhost.

Do you have any tips/suggestions for testing these functions? How would the test remote directory have to be set up to interface correctly?

Reece Hart

unread,
Jan 23, 2019, 12:58:45 PM1/23/19
to Iskandar Pashayev, biocommons-devs
Iskandar-

Thanks for your enthusiasm!

You're completely correct: it's hard to mock those aspects of the cli.

Your proposal to include a test directory would work and be easy to implement. The only downside is that it would require that rsync exists on the test host. I presume that it exists on travis-ci, but I'm unsure.

An alternative that might be even easier is to mock _get_remote_instances() using https://pypi.org/project/pytest-mock/. The mocked version would return a list of directories, just like the live version does. A mocked version will do nothing to actually test that function, of course, but would allow callers to be tested.

Also, we should prioritize based on perceived risk and effort to test. I'm not committed to 100% test coverage (after removing the py2compat stuff).

-Reece



--
gitter room: https://gitter.im/biocommons/devs
---
You received this message because you are subscribed to the Google Groups "biocommons-devs" group.
To unsubscribe from this group and stop receiving emails from it, send an email to biocommons-de...@googlegroups.com.
To post to this group, send email to biocomm...@googlegroups.com.
Visit this group at https://groups.google.com/group/biocommons-devs.
To view this discussion on the web visit https://groups.google.com/d/msgid/biocommons-devs/8e813c36-ee77-4747-8ad1-68f46cf86405%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Iskandar Pashayev

unread,
Jan 29, 2019, 3:30:23 AM1/29/19
to biocommons-devs
Thanks for the suggestion Reece.

I was able to mock _get_remote_instances using pytest-mock, but unfortunately it doesn't gain substantial coverage. It can only cover list-remote-instances; the pull function relies on executing rsync.

I also had some questions about covering the descriptive functions. These are functions like show_status, list_local_instances, list_remote_instances. Should the tests be checking what these functions print to standard output?


On Wednesday, January 23, 2019 at 9:58:45 AM UTC-8, Reece Hart wrote:
Iskandar-

Thanks for your enthusiasm!

You're completely correct: it's hard to mock those aspects of the cli.

Your proposal to include a test directory would work and be easy to implement. The only downside is that it would require that rsync exists on the test host. I presume that it exists on travis-ci, but I'm unsure.

An alternative that might be even easier is to mock _get_remote_instances() using https://pypi.org/project/pytest-mock/. The mocked version would return a list of directories, just like the live version does. A mocked version will do nothing to actually test that function, of course, but would allow callers to be tested.

Also, we should prioritize based on perceived risk and effort to test. I'm not committed to 100% test coverage (after removing the py2compat stuff).

-Reece



On Wed, Jan 23, 2019 at 8:02 AM Iskandar Pashayev <ispas...@gmail.com> wrote:
I've been looking into the cli module to increase its test coverage, and had a question about covering the `pull`/`list-remote-instances` functions.  Those functions make network requests through the `rsync` executable. A really simple way to cover them would be to just send a request over the network to the default remote host (dl.biocommons.org), but that doesn't seem like good practice.

I was thinking of making a local `test_remote_data` directory and then launching a subprocess during the test that could serve it over localhost.

Do you have any tips/suggestions for testing these functions? How would the test remote directory have to be set up to interface correctly?

--
gitter room: https://gitter.im/biocommons/devs
---
You received this message because you are subscribed to the Google Groups "biocommons-devs" group.
To unsubscribe from this group and stop receiving emails from it, send an email to biocommons-devs+unsubscribe@googlegroups.com.
To post to this group, send email to biocommons-devs@googlegroups.com.

Reece Hart

unread,
Feb 1, 2019, 6:54:12 PM2/1/19
to Iskandar Pashayev, biocommons-devs
No, tests shouldn't test output unless it's critical. The most important thing to check is function. For example, I think it would be straightforward to init a directory, load some sample sequences, export sequences (hmm... to file?). 

I think it might be more useful to talk about other areas of code to work on. I'll bring ideas to the next meeting.

-Reece

On Tue, Jan 29, 2019 at 12:30 AM Iskandar Pashayev <ispas...@gmail.com> wrote:
Thanks for the suggestion Reece.

I was able to mock _get_remote_instances using pytest-mock, but unfortunately it doesn't gain substantial coverage. It can only cover list-remote-instances; the pull function relies on executing rsync.

I also had some questions about covering the descriptive functions. These are functions like show_status, list_local_instances, list_remote_instances. Should the tests be checking what these functions print to standard output?


On Wednesday, January 23, 2019 at 9:58:45 AM UTC-8, Reece Hart wrote:
Iskandar-

Thanks for your enthusiasm!

You're completely correct: it's hard to mock those aspects of the cli.

Your proposal to include a test directory would work and be easy to implement. The only downside is that it would require that rsync exists on the test host. I presume that it exists on travis-ci, but I'm unsure.

An alternative that might be even easier is to mock _get_remote_instances() using https://pypi.org/project/pytest-mock/. The mocked version would return a list of directories, just like the live version does. A mocked version will do nothing to actually test that function, of course, but would allow callers to be tested.

Also, we should prioritize based on perceived risk and effort to test. I'm not committed to 100% test coverage (after removing the py2compat stuff).

-Reece



On Wed, Jan 23, 2019 at 8:02 AM Iskandar Pashayev <ispas...@gmail.com> wrote:
I've been looking into the cli module to increase its test coverage, and had a question about covering the `pull`/`list-remote-instances` functions.  Those functions make network requests through the `rsync` executable. A really simple way to cover them would be to just send a request over the network to the default remote host (dl.biocommons.org), but that doesn't seem like good practice.

I was thinking of making a local `test_remote_data` directory and then launching a subprocess during the test that could serve it over localhost.

Do you have any tips/suggestions for testing these functions? How would the test remote directory have to be set up to interface correctly?

--
gitter room: https://gitter.im/biocommons/devs
---
You received this message because you are subscribed to the Google Groups "biocommons-devs" group.
To unsubscribe from this group and stop receiving emails from it, send an email to biocommons-de...@googlegroups.com.
To post to this group, send email to biocomm...@googlegroups.com.

--
gitter room: https://gitter.im/biocommons/devs
---
You received this message because you are subscribed to the Google Groups "biocommons-devs" group.
To unsubscribe from this group and stop receiving emails from it, send an email to biocommons-de...@googlegroups.com.
To post to this group, send email to biocomm...@googlegroups.com.
Reply all
Reply to author
Forward
0 new messages