From 6538c9ffd86cf1b35a88675d32948b390037dd64 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Sat, 31 Oct 2020 16:49:42 -0400 Subject: [PATCH 1/2] run installed_version as executor job --- custom_components/pyscript/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/custom_components/pyscript/__init__.py b/custom_components/pyscript/__init__.py index a194b1c..c5b8eb5 100644 --- a/custom_components/pyscript/__init__.py +++ b/custom_components/pyscript/__init__.py @@ -307,7 +307,9 @@ async def install_requirements(hass): # 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) + requirement_installed_version = await hass.async_add_executor_job( + installed_version, requirement.project_name + ) if requirement_installed_version in requirement: _LOGGER.debug("`%s` already found", requirement.project_name) From 4babcf53d3211e458aca8fb233b0cc5063635c2c Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Sat, 31 Oct 2020 17:45:37 -0400 Subject: [PATCH 2/2] refactor functions, fix existing tests, and add additional tests --- custom_components/pyscript/__init__.py | 44 +++++++++------- tests/test_decorator_errors.py | 11 +--- tests/test_function.py | 11 +--- tests/test_init.py | 69 +++++++++++++++++++------- tests/test_jupyter.py | 11 +--- tests/test_unique.py | 11 +--- 6 files changed, 83 insertions(+), 74 deletions(-) diff --git a/custom_components/pyscript/__init__.py b/custom_components/pyscript/__init__.py index c5b8eb5..b386ba1 100644 --- a/custom_components/pyscript/__init__.py +++ b/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. @@ -307,9 +303,7 @@ async def install_requirements(hass): # 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 = await hass.async_add_executor_job( - installed_version, requirement.project_name - ) + requirement_installed_version = installed_version(requirement.project_name) if requirement_installed_version in requirement: _LOGGER.debug("`%s` already found", requirement.project_name) @@ -325,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", diff --git a/tests/test_decorator_errors.py b/tests/test_decorator_errors.py index fd48499..2601435 100644 --- a/tests/test_decorator_errors.py +++ b/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: {}}) diff --git a/tests/test_function.py b/tests/test_function.py index 153a7c9..3f73ece 100644 --- a/tests/test_function.py +++ b/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}}) diff --git a/tests/test_init.py b/tests/test_init.py index cef329c..640b681 100644 --- a/tests/test_init.py +++ b/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.", + ) diff --git a/tests/test_jupyter.py b/tests/test_jupyter.py index 0c71459..2a4fbbf 100644 --- a/tests/test_jupyter.py +++ b/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: {}}) diff --git a/tests/test_unique.py b/tests/test_unique.py index f1536ce..15cf75c 100644 --- a/tests/test_unique.py +++ b/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: {}})