Unit tests Recipe.download_file() partly (#1952)

Follow up of #1946.
Increases coverage by testing part of `Recipe.download_file()` handling
https schema. Next up would be to handle more schemas in the tests.
Also addressed @opacam comments from previous pull requests.

Author: AndreMiras

pythonforandroid/toolchain.py

@@ -743,12 +743,11 @@ def _read_configuration():
     def recipes(self, args):
         """
         Prints recipes basic info, e.g.
-        ```
-        python3      3.7.1
-            depends: ['hostpython3', 'sqlite3', 'openssl', 'libffi']
-            conflicts: ['python2']
-            optional depends: ['sqlite3', 'libffi', 'openssl']
-        ```
+        .. code-block:: bash
+            python3      3.7.1
+                depends: ['hostpython3', 'sqlite3', 'openssl', 'libffi']
+                conflicts: ['python2']
+                optional depends: ['sqlite3', 'libffi', 'openssl']
         """
         ctx = self.ctx
         if args.compact:

tests/test_recipe.py

@@ -1,4 +1,5 @@
 import os
+import pytest
 import types
 import unittest
 import warnings
@@ -20,6 +21,10 @@ def patch_logger_debug():
     return patch_logger('debug')
 
 
+def patch_urlretrieve():
+    return mock.patch('pythonforandroid.recipe.urlretrieve')
+
+
 class DummyRecipe(Recipe):
     pass
 
@@ -94,21 +99,34 @@ def test_download_if_necessary(self):
             recipe.download_if_necessary()
         assert m_download.call_args_list == []
 
-    def test_download(self):
+    def test_download_url_not_set(self):
         """
-        Verifies the actual download gets triggered when the URL is set.
+        Verifies that no download happens when URL is not set.
         """
-        # test with no URL set
         recipe = DummyRecipe()
         with patch_logger_info() as m_info:
             recipe.download()
         assert m_info.call_args_list == [
             mock.call('Skipping test_recipe download as no URL is set')]
-        # when the URL is set `Recipe.download_file()` should be called
+
+    @staticmethod
+    def get_dummy_python_recipe_for_download_tests():
+        """
+        Helper method for creating a test recipe used in download tests.
+        """
+        recipe = DummyRecipe()
         filename = 'Python-3.7.4.tgz'
         url = 'https://www.python.org/ftp/python/3.7.4/{}'.format(filename)
         recipe._url = url
         recipe.ctx = Context()
+        return recipe, filename
+
+    def test_download_url_is_set(self):
+        """
+        Verifies the actual download gets triggered when the URL is set.
+        """
+        recipe, filename = self.get_dummy_python_recipe_for_download_tests()
+        url = recipe.url
         with (
                 patch_logger_debug()) as m_debug, (
                 mock.patch.object(Recipe, 'download_file')) as m_download_file, (
@@ -122,3 +140,39 @@ def test_download(self):
                 'Downloading test_recipe from '
                 'https://www.python.org/ftp/python/3.7.4/Python-3.7.4.tgz')]
         assert m_touch.call_count == 1
+
+    def test_download_file_scheme_https(self):
+        """
+        Verifies `urlretrieve()` is being called on https downloads.
+        """
+        recipe, filename = self.get_dummy_python_recipe_for_download_tests()
+        url = recipe.url
+        with (
+                patch_urlretrieve()) as m_urlretrieve, (
+                tempfile.TemporaryDirectory()) as temp_dir:
+            recipe.ctx.setup_dirs(temp_dir)
+            assert recipe.download_file(url, filename) == filename
+        assert m_urlretrieve.call_args_list == [
+            mock.call(url, filename, mock.ANY)
+        ]
+
+    def test_download_file_scheme_https_oserror(self):
+        """
+        Checks `urlretrieve()` is being retried on `OSError`.
+        After a number of retries the exception is re-reaised.
+        """
+        recipe, filename = self.get_dummy_python_recipe_for_download_tests()
+        url = recipe.url
+        with (
+                patch_urlretrieve()) as m_urlretrieve, (
+                mock.patch('pythonforandroid.recipe.time.sleep')) as m_sleep, (
+                pytest.raises(OSError)), (
+                tempfile.TemporaryDirectory()) as temp_dir:
+            recipe.ctx.setup_dirs(temp_dir)
+            m_urlretrieve.side_effect = OSError
+            assert recipe.download_file(url, filename) == filename
+        retry = 5
+        expected_call_args_list = [mock.call(url, filename, mock.ANY)] * retry
+        assert m_urlretrieve.call_args_list == expected_call_args_list
+        expected_call_args_list = [mock.call(1)] * (retry - 1)
+        assert m_sleep.call_args_list == expected_call_args_list