move sync logic into dedicated function and update test

raman325 · raman325 · commit c1c9345bee6a · 2020-10-31T16:32:26.000-04:00
diff --git a/custom_components/pyscript/__init__.py b/custom_components/pyscript/__init__.py
@@ -30,6 +30,8 @@
     DOMAIN,
     FOLDER,
     LOGGER_PATH,
+    REQUIREMENTS_FILE,
+    REQUIREMENTS_PATHS,
     SERVICE_JUPYTER_KERNEL_START,
     UNSUB_LISTENERS,
 )
@@ -147,7 +149,7 @@ async def async_setup_entry(hass, config_entry):
 
     State.set_pyscript_config(config_entry.data)
 
-    await hass.async_add_executor_job(install_requirements, hass)
+    await install_requirements(hass)
     await load_scripts(hass, config_entry.data)
 
     async def reload_scripts_handler(call):
@@ -165,7 +167,7 @@ async def reload_scripts_handler(call):
 
         await unload_scripts(global_ctx_only=global_ctx_only)
 
-        await hass.async_add_executor_job(install_requirements, hass)
+        await install_requirements(hass)
         await load_scripts(hass, config_entry.data, global_ctx_only=global_ctx_only)
 
         start_global_contexts(global_ctx_only=global_ctx_only)
@@ -266,55 +268,71 @@ async def unload_scripts(global_ctx_only=None, unload_all=False):
         await GlobalContextMgr.delete(global_ctx_name)
 
 
+@bind_hass
+def load_all_requirement_lines(hass, requirements_paths, requirements_file):
+    """Load all lines from requirements_file located in requirements_paths."""
+    all_requirements = {}
+    for root in requirements_paths:
+        for requirements_path in glob.glob(os.path.join(hass.config.path(FOLDER), root, requirements_file)):
+            with open(requirements_path, "r") as requirements_fp:
+                all_requirements[requirements_path] = requirements_fp.readlines()
+
+    return all_requirements
+
+
 @bind_hass
 async def install_requirements(hass):
     """Install missing requirements from requirements.txt."""
-    for root in ("", "apps/*", "modules/*"):
-        for requirements_path in glob.glob(os.path.join(hass.config.path(FOLDER), root, "requirements.txt")):
-            with open(requirements_path, "r") as requirements_file:
-                requirements_to_install = []
-                for pkg in requirements_file.readlines():
-                    # Remove inline comments which are accepted by pip but not by Home
-                    # Assistant's installation method.
-                    # https://rosettacode.org/wiki/Strip_comments_from_a_string#Python
-                    i = pkg.find("#")
-                    if i >= 0:
-                        pkg = pkg[:i]
-                    pkg = pkg.strip()
-
-                    try:
-                        # Attempt to get version of package. Do nothing if it's found since
-                        # we want to use the version that's already installed to be safe
-                        requirement = pkg_resources.Requirement.parse(pkg)
-                        requirement_installed_version = installed_version(requirement.project_name)
-
-                        if requirement_installed_version in requirement:
-                            _LOGGER.debug("`%s` already found", requirement.project_name)
-                        else:
-                            _LOGGER.warning(
-                                (
-                                    "`%s` already found but found version `%s` does not"
-                                    " match requirement. Keeping found version."
-                                ),
-                                requirement.project_name,
-                                requirement_installed_version,
-                            )
-                    except PackageNotFoundError:
-                        # Since package wasn't found, add it to installation list
-                        _LOGGER.debug("%s not found, adding it to package installation list", pkg)
-                        requirements_to_install.append(pkg)
-                    except ValueError:
-                        # Not valid requirements line so it can be skipped
-                        _LOGGER.debug("Ignoring `%s` because it is not a valid package", pkg)
-                if requirements_to_install:
-                    _LOGGER.info(
-                        "Installing the following packages from %s: %s",
-                        requirements_path,
-                        ", ".join(requirements_to_install),
-                    )
-                    await async_process_requirements(hass, DOMAIN, requirements_to_install)
+    all_requirements = await hass.async_add_executor_job(
+        load_all_requirement_lines, hass, REQUIREMENTS_PATHS, REQUIREMENTS_FILE
+    )
+    requirements_to_install = []
+    for requirements_path, pkg_lines in all_requirements.items():
+        for pkg in pkg_lines:
+            # Remove inline comments which are accepted by pip but not by Home
+            # Assistant's installation method.
+            # https://rosettacode.org/wiki/Strip_comments_from_a_string#Python
+            i = pkg.find("#")
+            if i >= 0:
+                pkg = pkg[:i]
+            pkg = pkg.strip()
+
+            if not pkg:
+                continue
+
+            try:
+                # Attempt to get version of package. Do nothing if it's found since
+                # we want to use the version that's already installed to be safe
+                requirement = pkg_resources.Requirement.parse(pkg)
+                requirement_installed_version = installed_version(requirement.project_name)
+
+                if requirement_installed_version in requirement:
+                    _LOGGER.debug("`%s` already found", requirement.project_name)
                 else:
-                    _LOGGER.debug("All packages in %s are already available", requirements_path)
+                    _LOGGER.warning(
+                        (
+                            "`%s` already found but found version `%s` does not"
+                            " match requirement. Keeping found version."
+                        ),
+                        requirement.project_name,
+                        requirement_installed_version,
+                    )
+            except PackageNotFoundError:
+                # Since package wasn't found, add it to installation list
+                _LOGGER.debug("%s not found, adding it to package installation list", pkg)
+                requirements_to_install.append(pkg)
+            except ValueError:
+                # Not valid requirements line so it can be skipped
+                _LOGGER.debug("Ignoring `%s` because it is not a valid package", pkg)
+        if requirements_to_install:
+            _LOGGER.info(
+                "Installing the following packages from %s: %s",
+                requirements_path,
+                ", ".join(requirements_to_install),
+            )
+            await async_process_requirements(hass, DOMAIN, requirements_to_install)
+        else:
+            _LOGGER.debug("All packages in %s are already available", requirements_path)
 
 
 @bind_hass
diff --git a/custom_components/pyscript/const.py b/custom_components/pyscript/const.py
@@ -14,6 +14,9 @@
 
 LOGGER_PATH = "custom_components.pyscript"
 
+REQUIREMENTS_FILE = "requirements.txt"
+REQUIREMENTS_PATHS = ("", "apps/*", "modules/*")
+
 ALLOWED_IMPORTS = {
     "black",
     "cmath",
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -0,0 +1,10 @@
+"""Test configuration for pyscript."""
+from pytest import fixture
+from pytest_homeassistant_custom_component.async_mock import patch
+
+
+@fixture(autouse=True)
+def bypass_package_install_fixture():
+    """Bypass package installation."""
+    with patch("custom_components.pyscript.async_process_requirements"):
+        yield
diff --git a/tests/test_decorator_errors.py b/tests/test_decorator_errors.py
@@ -23,6 +23,16 @@ async def setup_script(hass, notify_q, now, source):
         "custom_components.pyscript.trigger.dt_now", return_value=now
     ), patch(
         "homeassistant.config.load_yaml_config_file", return_value={}
+    ), patch(
+        "custom_components.pyscript.load_all_requirement_lines",
+        return_value={
+            "/some/config/dir/pyscript/requirements.txt": [
+                "pytube==9.7.0\n",
+                "# another test comment\n",
+                "pykakasi==2.0.1 # test comment\n",
+                "\n",
+            ]
+        },
     ):
         assert await async_setup_component(hass, "pyscript", {DOMAIN: {}})
 
diff --git a/tests/test_function.py b/tests/test_function.py
@@ -109,6 +109,16 @@ async def setup_script(hass, notify_q, now, source):
         "custom_components.pyscript.trigger.dt_now", return_value=now
     ), patch(
         "homeassistant.config.load_yaml_config_file", return_value={DOMAIN: {CONF_ALLOW_ALL_IMPORTS: True}}
+    ), patch(
+        "custom_components.pyscript.load_all_requirement_lines",
+        return_value={
+            "/some/config/dir/pyscript/requirements.txt": [
+                "pytube==9.7.0\n",
+                "# another test comment\n",
+                "pykakasi==2.0.1 # test comment\n",
+                "\n",
+            ]
+        },
     ):
         assert await async_setup_component(hass, "pyscript", {DOMAIN: {CONF_ALLOW_ALL_IMPORTS: True}})
 
diff --git a/tests/test_init.py b/tests/test_init.py
@@ -32,6 +32,16 @@ async def setup_script(hass, notify_q, now, source):
         "custom_components.pyscript.trigger.dt_now", return_value=now
     ), patch(
         "homeassistant.config.load_yaml_config_file", return_value={}
+    ), patch(
+        "custom_components.pyscript.load_all_requirement_lines",
+        return_value={
+            "/some/config/dir/pyscript/requirements.txt": [
+                "pytube==9.7.0\n",
+                "# another test comment\n",
+                "pykakasi==2.0.1 # test comment\n",
+                "\n",
+            ]
+        },
     ):
         assert await async_setup_component(hass, "pyscript", {DOMAIN: {}})
 
@@ -446,6 +456,16 @@ def func5(var_name=None, value=None):
             "custom_components.pyscript.trigger.dt_now", return_value=now
         ), patch(
             "homeassistant.config.load_yaml_config_file", return_value={}
+        ), patch(
+            "custom_components.pyscript.load_all_requirement_lines",
+            return_value={
+                "/some/config/dir/pyscript/requirements.txt": [
+                    "pytube==9.7.0\n",
+                    "# another test comment\n",
+                    "pykakasi==2.0.1 # test comment\n",
+                    "\n",
+                ]
+            },
         ):
             reload_param = {}
             if i % 2 == 1:
@@ -486,17 +506,11 @@ async def test_misc_errors(hass, caplog):
 
 async def test_install_requirements(hass):
     """Test install_requirements function."""
-    requirements = """
-pytube==9.7.0
-# another test comment
-pykakasi==2.0.1 # test comment
-
-"""
-
     with patch("custom_components.pyscript.async_hass_config_yaml", return_value={}), patch(
-        "custom_components.pyscript.open", mock_open(read_data=requirements), create=True,
-    ), patch("custom_components.pyscript.async_process_requirements") as install_requirements:
+        "custom_components.pyscript.async_process_requirements"
+    ) as install_requirements:
         await setup_script(hass, None, dt(2020, 7, 1, 11, 59, 59, 999999), "")
+        assert install_requirements.called
         assert install_requirements.call_args[0][2] == ["pytube==9.7.0", "pykakasi==2.0.1"]
         install_requirements.reset_mock()
         # Because in tests, packages are not installed, we fake that they are
diff --git a/tests/test_jupyter.py b/tests/test_jupyter.py
@@ -117,6 +117,16 @@ async def setup_script(hass, now, source, no_connect=False):
         "custom_components.pyscript.trigger.dt_now", return_value=now
     ), patch(
         "homeassistant.config.load_yaml_config_file", return_value={}
+    ), patch(
+        "custom_components.pyscript.load_all_requirement_lines",
+        return_value={
+            "/some/config/dir/pyscript/requirements.txt": [
+                "pytube==9.7.0\n",
+                "# another test comment\n",
+                "pykakasi==2.0.1 # test comment\n",
+                "\n",
+            ]
+        },
     ):
         assert await async_setup_component(hass, "pyscript", {DOMAIN: {}})
 
diff --git a/tests/test_unique.py b/tests/test_unique.py
@@ -23,6 +23,16 @@ async def setup_script(hass, notify_q, now, source):
         "custom_components.pyscript.trigger.dt_now", return_value=now
     ), patch(
         "homeassistant.config.load_yaml_config_file", return_value={}
+    ), patch(
+        "custom_components.pyscript.load_all_requirement_lines",
+        return_value={
+            "/some/config/dir/pyscript/requirements.txt": [
+                "pytube==9.7.0\n",
+                "# another test comment\n",
+                "pykakasi==2.0.1 # test comment\n",
+                "\n",
+            ]
+        },
     ):
         assert await async_setup_component(hass, "pyscript", {DOMAIN: {}})
 
