test_networkutil: Implement tests for download_file

[sonic2kk]

Depends on #529 for the same reason as #535; we need pyfakefs, responses, and pytest-mock,

Overview

This PR implements tests for download_file. We test what I believe are all of the non-happy-path scenarios, and most of the permutations of the happy-path scenario.

Since we're moving more of our ctmods over to use download_file, I think it's useful to have tests for this function to give us confidence that various cases work and behave as expected beyond "it works for other ctmods". 😄

With these tests, we have 88% coverage of networkutil.py (which only has download_file util function at this time) according to pytest-cov.

Implementation

To test the happy path scenarios we make extensive use of PyTest's Parametrize functionality. If I have missed a test scenario, let me know and it can be added.

In order to test downloading a file, we have some steps:

• Use pyfakefs to load a sample file as a PyTest Fixture that can be used across tests. This is used as an example response from a GET request. This also allows download_file to write to a real file at our destination path that we can load and compare against. We can simulate real filesystem operations and compare them against our real file.
  • pyfakefs mocks out the os module but not Pathlib, so it's safe to use Pathlib to load the file.
  • While the FakeFilesystem fixture fs does have some wrapper methods for our os functions, since it mocks out os it's safe to use the regular methods. This is also what our tested components would do anyway :-)
• Use pytest-mock's MockerFixture to force certain operations inside of download_file to raise exceptions. We use this to raise an OSError when calling os.makedirs , and we also use it to spy on our progress_callback to check that it was called a given number of times and with expected arguments (the progress of the download).
• Use responses to mock our GET requests, since we don't want to make any real network calls. In combination with pyfakefs this allows us to load a file and set its contents as the returned body from the request to the given URL we call with download_file.

Structure

For the docstrings in these tests, I decided to experiment with "Given-When-Then" syntax to describe the tests. I'm a fan, and happy to revise other tests to use this structure as well if we want to use it as a go-forward and think it's a useful way to describe our tests!

Concerns

There is duplicated logic to send a mocked request across a couple of test scenarios. For example in test_download_file_cannot_create_destination we have to do the same set up as we do in test_download_file, because we still want to send a request but we want to check that it fails right before trying to write the file.

Maybe this isn't a huge deal, but it is something I wanted to call out.

Future Work

This PR only adds a few test functions, but because we use Parametrize, we end up with this PR effectively adding 19 tests. This on top of our existing tests, and the opened PRs that also use Parametrize, we could end up with a fair amount of tests. It may be worth looking into parallelizing some of our tests.

We should be able to do this fairly easily with pytest-xdist :-)

---

Opening as draft as this PR is dependent on another to be merged, but since it's standalone in a new test_networkutil.py file and since download_file is the only function in that file, I thought it was worth working on in parallel.

As always, all feedback is welcome. Thanks! 😄