Refactor logic for processing requirements.txt so all I/O operations happen in a single function

custom_components/pyscript/__init__.py

@@ -272,25 +272,21 @@ async def unload_scripts(global_ctx_only=None, unload_all=False):
 
 
 @bind_hass
-def load_all_requirement_lines(hass, requirements_paths, requirements_file):
-    """Load all lines from requirements_file located in requirements_paths."""
-    all_requirements = {}
+def process_all_requirements(hass, requirements_paths, requirements_file):
+    """
+    Load all lines from requirements_file located in requirements_paths.
+
+    Returns files and a list of packages, if any, that need to be installed.
+    """
+    all_requirements_to_process = {}
     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
-
+                all_requirements_to_process[requirements_path] = requirements_fp.readlines()
 
-@bind_hass
-async def install_requirements(hass):
-    """Install missing requirements from requirements.txt."""
-    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():
+    all_requirements_to_install = {}
+    for requirements_path, pkg_lines in all_requirements_to_process.items():
+        all_requirements_to_install[requirements_path] = []
         for pkg in pkg_lines:
             # Remove inline comments which are accepted by pip but not by Home
             # Assistant's installation method.
@@ -323,10 +319,22 @@ async def install_requirements(hass):
             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)
+                all_requirements_to_install[requirements_path].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)
+
+    return all_requirements_to_install
+
+
+@bind_hass
+async def install_requirements(hass):
+    """Install missing requirements from requirements.txt."""
+    all_requirements = await hass.async_add_executor_job(
+        process_all_requirements, hass, REQUIREMENTS_PATHS, REQUIREMENTS_FILE
+    )
+
+    for requirements_path, requirements_to_install in all_requirements.items():
         if requirements_to_install:
             _LOGGER.info(
                 "Installing the following packages from %s: %s",

tests/test_decorator_errors.py

@@ -24,15 +24,8 @@ async def setup_script(hass, notify_q, now, source):
     ), 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",
-            ]
-        },
+        "custom_components.pyscript.process_all_requirements",
+        return_value={"/some/config/dir/pyscript/requirements.txt": ["pytube==2.0.1", "pykakasi==2.0.1"]},
     ):
         assert await async_setup_component(hass, "pyscript", {DOMAIN: {}})
 

tests/test_function.py

@@ -111,15 +111,8 @@ async def setup_script(hass, notify_q, now, source):
     ), 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",
-            ]
-        },
+        "custom_components.pyscript.process_all_requirements",
+        return_value={"/some/config/dir/pyscript/requirements.txt": ["pytube==2.0.1", "pykakasi==2.0.1"]},
     ):
         assert await async_setup_component(hass, "pyscript", {DOMAIN: {CONF_ALLOW_ALL_IMPORTS: True}})
 

tests/test_init.py

@@ -4,7 +4,8 @@
 from datetime import datetime as dt
 import pathlib
 
-from custom_components.pyscript.const import DOMAIN
+from custom_components.pyscript import process_all_requirements
+from custom_components.pyscript.const import DOMAIN, REQUIREMENTS_FILE, REQUIREMENTS_PATHS
 from custom_components.pyscript.event import Event
 from custom_components.pyscript.function import Function
 from custom_components.pyscript.global_ctx import GlobalContextMgr
@@ -33,15 +34,8 @@ async def setup_script(hass, notify_q, now, source):
     ), 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",
-            ]
-        },
+        "custom_components.pyscript.process_all_requirements",
+        return_value={"/some/config/dir/pyscript/requirements.txt": ["pytube==2.0.1", "pykakasi==2.0.1"]},
     ):
         assert await async_setup_component(hass, "pyscript", {DOMAIN: {}})
 
@@ -457,14 +451,9 @@ def func5(var_name=None, value=None):
         ), patch(
             "homeassistant.config.load_yaml_config_file", return_value={}
         ), patch(
-            "custom_components.pyscript.load_all_requirement_lines",
+            "custom_components.pyscript.process_all_requirements",
             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",
-                ]
+                "/some/config/dir/pyscript/requirements.txt": ["pytube==2.0.1", "pykakasi==2.0.1"]
             },
         ):
             reload_param = {}
@@ -511,10 +500,52 @@ async def test_install_requirements(hass):
     ) 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"]
+        assert install_requirements.call_args[0][2] == ["pytube==2.0.1", "pykakasi==2.0.1"]
         install_requirements.reset_mock()
         # Because in tests, packages are not installed, we fake that they are
         # installed so we can test that we don't attempt to install them
-        with patch("custom_components.pyscript.installed_version", return_value="2.0.1"):
+        with patch("custom_components.pyscript.installed_version", return_value="2.0.1"), patch(
+            "custom_components.pyscript.process_all_requirements",
+            return_value={"/some/config/dir/pyscript/requirements.txt": []},
+        ):
             await hass.services.async_call("pyscript", "reload", {}, blocking=True)
             assert not install_requirements.called
+
+
+async def test_process_requirements(hass, caplog):
+    """Test process requirements function."""
+    requirements = [
+        "/some/config/dir/pyscript/requirements.txt",
+    ]
+
+    source = """
+# Comment
+pytube==2.0.1
+>
+pkakasi==2.0.1 # Comment
+
+
+"""
+
+    with patch("custom_components.pyscript.glob.iglob", return_value=requirements), patch(
+        "custom_components.pyscript.open", mock_open(read_data=source), create=True
+    ):
+        all_requirements = await hass.async_add_executor_job(
+            process_all_requirements, hass, REQUIREMENTS_PATHS, REQUIREMENTS_FILE
+        )
+        assert requirements[0] in all_requirements
+        assert all_requirements[requirements[0]] == ["pytube==2.0.1", "pkakasi==2.0.1"]
+
+        with patch("custom_components.pyscript.installed_version") as installed_version:
+            installed_version.side_effect = ["2.0.1", "1.0.0"]
+            all_requirements = await hass.async_add_executor_job(
+                process_all_requirements, hass, REQUIREMENTS_PATHS, REQUIREMENTS_FILE
+            )
+            assert requirements[0] in all_requirements
+            assert all_requirements[requirements[0]] == []
+
+    assert caplog.record_tuples[0] == (
+        "custom_components.pyscript",
+        30,
+        "`pkakasi` already found but found version `1.0.0` does not match requirement. Keeping found version.",
+    )

tests/test_jupyter.py

@@ -118,15 +118,8 @@ async def setup_script(hass, now, source, no_connect=False):
     ), 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",
-            ]
-        },
+        "custom_components.pyscript.process_all_requirements",
+        return_value={"/some/config/dir/pyscript/requirements.txt": ["pytube==2.0.1", "pykakasi==2.0.1"]},
     ):
         assert await async_setup_component(hass, "pyscript", {DOMAIN: {}})
 

tests/test_unique.py

@@ -24,15 +24,8 @@ async def setup_script(hass, notify_q, now, source):
     ), 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",
-            ]
-        },
+        "custom_components.pyscript.process_all_requirements",
+        return_value={"/some/config/dir/pyscript/requirements.txt": ["pytube==2.0.1", "pykakasi==2.0.1"]},
     ):
         assert await async_setup_component(hass, "pyscript", {DOMAIN: {}})