diff --git a/ovos_core/skill_installer.py b/ovos_core/skill_installer.py index 4d3716ba089..51324818b40 100644 --- a/ovos_core/skill_installer.py +++ b/ovos_core/skill_installer.py @@ -37,11 +37,11 @@ def shutdown(self): pass def play_error_sound(self): - snd = Configuration().get("sounds", {}).get("pip_error", "snd/error.mp3") + snd = self.config.get("sounds", {}).get("pip_error", "snd/error.mp3") self.bus.emit(Message("mycroft.audio.play_sound", {"uri": snd})) def play_success_sound(self): - snd = Configuration().get("sounds", {}).get("pip_success", "snd/acknowledge.mp3") + snd = self.config.get("sounds", {}).get("pip_success", "snd/acknowledge.mp3") self.bus.emit(Message("mycroft.audio.play_sound", {"uri": snd})) def pip_install(self, packages: list, @@ -81,7 +81,9 @@ def pip_install(self, packages: list, proc = Popen(pip_command, stdout=PIPE, stderr=PIPE) pip_code = proc.wait() if pip_code != 0: - stderr = proc.stderr.read().decode() + stderr = proc.stderr + if stderr: + stderr = stderr.read().decode() self.play_error_sound() raise RuntimeError(stderr) @@ -198,7 +200,7 @@ def handle_install_python(self, message: Message): self.bus.emit(message.reply("ovos.pip.install.failed", {"error": InstallError.DISABLED.value})) return - pkgs = message.data["packages"] + pkgs = message.data.get("packages") if pkgs: if self.pip_install(pkgs): self.bus.emit(message.reply("ovos.pip.install.complete")) @@ -216,7 +218,7 @@ def handle_uninstall_python(self, message: Message): self.bus.emit(message.reply("ovos.pip.uninstall.failed", {"error": InstallError.DISABLED.value})) return - pkgs = message.data["packages"] + pkgs = message.data.get("packages") if pkgs: if self.pip_uninstall(pkgs): self.bus.emit(message.reply("ovos.pip.uninstall.complete")) diff --git a/test/unittests/skills/test_skill_installer.py b/test/unittests/skills/test_skill_installer.py new file mode 100644 index 00000000000..2450a0217f4 --- /dev/null +++ b/test/unittests/skills/test_skill_installer.py @@ -0,0 +1,220 @@ +from unittest.mock import Mock +import pytest +from ovos_bus_client import Message +from ..mocks import MessageBusMock +from ovos_core.skill_installer import SkillsStore + + +@pytest.fixture(scope="function", autouse=True) +def skills_store(request): + config = getattr(request, 'param', {}) + return SkillsStore(bus=MessageBusMock(), config=config) + + +def test_shutdown(skills_store): + assert skills_store.shutdown() is None + + +def test_play_error_sound(skills_store): + skills_store.play_error_sound() + assert skills_store.bus.message_data[-1] == { + "uri": "snd/error.mp3" + } + assert skills_store.bus.message_types[-1] == "mycroft.audio.play_sound" + + +@pytest.mark.parametrize("skills_store", [{"sounds": {"pip_error": "snd/custom_error.mp3"}}], indirect=True) +def test_play_error_sound_custom(skills_store): + skills_store.play_error_sound() + assert skills_store.bus.message_data[-1] == { + "uri": "snd/custom_error.mp3" + } + assert skills_store.bus.message_types[-1] == "mycroft.audio.play_sound" + + +def test_play_success_sound(skills_store): + skills_store.play_success_sound() + assert skills_store.bus.message_data[-1] == { + "uri": "snd/acknowledge.mp3" + } + assert skills_store.bus.message_types[-1] == "mycroft.audio.play_sound" + + + +@pytest.mark.parametrize("skills_store", [{"sounds": {"pip_success": "snd/custom_success.mp3"}}], indirect=True) +def test_play_success_sound_custom(skills_store): + skills_store.play_success_sound() + assert skills_store.bus.message_data[-1] == { + "uri": "snd/custom_success.mp3" + } + assert skills_store.bus.message_types[-1] == "mycroft.audio.play_sound" + + +def test_pip_install_no_packages(skills_store): + # TODO: This method should be refactored in 0.1.0 for easier unit testing + skills_store.play_error_sound = Mock() + res = skills_store.pip_install([]) + assert res is False + skills_store.play_error_sound.assert_called_once() + + +def test_pip_install_no_constraints(skills_store): + skills_store.play_error_sound = Mock() + res = skills_store.pip_install(["foo", "bar"], constraints="not/real") + assert res is False + skills_store.play_error_sound.assert_called_once() + +def test_pip_install_happy_path(): + # TODO: This method should be refactored in 0.1.0 for easier unit testing + assert True + + +def test_pip_uninstall_no_packages(skills_store): + # TODO: This method should be refactored in 0.1.0 for easier unit testing + skills_store.play_error_sound = Mock() + res = skills_store.pip_uninstall([]) + assert res is False + skills_store.play_error_sound.assert_called_once() + + +def test_pip_uninstall_no_constraints(skills_store): + skills_store.play_error_sound = Mock() + res = skills_store.pip_uninstall(["foo", "bar"], constraints="not/real") + assert res is False + skills_store.play_error_sound.assert_called_once() + +def test_pip_uninstall_happy_path(): + # TODO: This method should be refactored in 0.1.0 for easier unit testing + assert True + + +def test_validate_skill(skills_store): + assert skills_store.validate_skill("https://github.com/openvoiceos/skill-foo") is True + assert skills_store.validate_skill("https://gitlab.com/foo/skill-bar") is False + assert skills_store.validate_skill("literally-anything-else") is False + +@pytest.mark.parametrize('skills_store', [{"allow_pip": False}], indirect=True) +def test_handle_install_skill_not_allowed(skills_store): + skills_store.play_error_sound = Mock() + skills_store.validate_skill = Mock() + skills_store.handle_install_skill(Message(msg_type="test", data={})) + skills_store.play_error_sound.assert_called_once() + assert skills_store.bus.message_types[-1] == "ovos.skills.install.failed" + assert skills_store.bus.message_data[-1] == {"error": "pip disabled in mycroft.conf"} + skills_store.validate_skill.assert_not_called() + + +@pytest.mark.parametrize('skills_store', [{"allow_pip": True}], indirect=True) +def test_handle_install_skill_not_from_github(skills_store): + skills_store.play_error_sound = Mock() + skills_store.handle_install_skill(Message(msg_type="test", data={"url": "beautifulsoup4"})) + skills_store.play_error_sound.assert_called_once() + assert skills_store.bus.message_types[-1] == "ovos.skills.install.failed" + assert skills_store.bus.message_data[-1] == {"error": "skill url validation failed"} + + +@pytest.mark.parametrize('skills_store', [{"allow_pip": True}], indirect=True) +def test_handle_install_skill_from_github(skills_store): + skills_store.play_error_sound = Mock() + skills_store.pip_install = Mock(return_value=True) + skills_store.handle_install_skill(Message(msg_type="test", data={"url": "https://github.com/OpenVoiceOS/skill-foo"})) + skills_store.play_error_sound.assert_not_called() + skills_store.pip_install.assert_called_once_with(["git+https://github.com/OpenVoiceOS/skill-foo"]) + assert skills_store.bus.message_types[-1] == "ovos.skills.install.complete" + assert skills_store.bus.message_data[-1] == {} + + +@pytest.mark.parametrize('skills_store', [{"allow_pip": True}], indirect=True) +def test_handle_install_skill_from_github_failure(skills_store): + skills_store.play_error_sound = Mock() + skills_store.pip_install = Mock(return_value=False) + skills_store.handle_install_skill(Message(msg_type="test", data={"url": "https://github.com/OpenVoiceOS/skill-foo"})) + skills_store.play_error_sound.assert_not_called() + skills_store.pip_install.assert_called_once_with(["git+https://github.com/OpenVoiceOS/skill-foo"]) + assert skills_store.bus.message_types[-1] == "ovos.skills.install.failed" + + +@pytest.mark.parametrize('skills_store', [{"allow_pip": False}], indirect=True) +def test_handle_uninstall_skill_not_allowed(skills_store): + skills_store.play_error_sound = Mock() + skills_store.handle_uninstall_skill(Message(msg_type="test", data={})) + skills_store.play_error_sound.assert_called_once() + assert skills_store.bus.message_types[-1] == "ovos.skills.uninstall.failed" + assert skills_store.bus.message_data[-1] == {"error": "pip disabled in mycroft.conf"} + + +@pytest.mark.parametrize('skills_store', [{"allow_pip": True}], indirect=True) +def test_handle_uninstall_skill(skills_store): + skills_store.play_error_sound = Mock() + skills_store.handle_uninstall_skill(Message(msg_type="test", data={})) + skills_store.play_error_sound.assert_called_once() + assert skills_store.bus.message_types[-1] == "ovos.skills.uninstall.failed" + assert skills_store.bus.message_data[-1] == {"error": "not implemented"} + + +@pytest.mark.parametrize('skills_store', [{"allow_pip": False}], indirect=True) +def test_handle_install_python_not_allowed(skills_store): + skills_store.play_error_sound = Mock() + skills_store.pip_install = Mock() + skills_store.handle_install_python(Message(msg_type="test", data={})) + skills_store.play_error_sound.assert_called_once() + assert skills_store.bus.message_types[-1] == "ovos.pip.install.failed" + assert skills_store.bus.message_data[-1] == {"error": "pip disabled in mycroft.conf"} + skills_store.pip_install.assert_not_called() + + +@pytest.mark.parametrize('skills_store', [{"allow_pip": True}], indirect=True) +def test_handle_install_python_no_packages(skills_store): + skills_store.pip_install = Mock() + skills_store.handle_install_python(Message(msg_type="test", data={})) + assert skills_store.bus.message_types[-1] == "ovos.pip.install.failed" + assert skills_store.bus.message_data[-1] == {"error": "no packages to install"} + skills_store.pip_install.assert_not_called() + + +@pytest.mark.parametrize('skills_store', [{"allow_pip": True}], indirect=True) +def test_handle_install_python_success(skills_store): + skills_store.play_error_sound = Mock() + skills_store.pip_install = Mock() + packages = ["requests", "fastapi"] + skills_store.handle_install_python(Message(msg_type="test", data={"packages": packages})) + skills_store.play_error_sound.assert_not_called() + skills_store.pip_install.assert_called_once_with(packages) + assert skills_store.bus.message_types[-1] == "ovos.pip.install.complete" + assert skills_store.bus.message_data[-1] == {} + + +@pytest.mark.parametrize('skills_store', [{"allow_pip": False}], indirect=True) +def test_handle_uninstall_python_not_allowed(skills_store): + skills_store.play_error_sound = Mock() + skills_store.pip_uninstall = Mock() + skills_store.handle_uninstall_python(Message(msg_type="test", data={})) + skills_store.play_error_sound.assert_called_once() + assert skills_store.bus.message_types[-1] == "ovos.pip.uninstall.failed" + assert skills_store.bus.message_data[-1] == {"error": "pip disabled in mycroft.conf"} + skills_store.pip_uninstall.assert_not_called() + + +@pytest.mark.parametrize('skills_store', [{"allow_pip": True}], indirect=True) +def test_handle_uninstall_python_no_packages(skills_store): + skills_store.pip_uninstall = Mock() + skills_store.handle_uninstall_python(Message(msg_type="test", data={})) + assert skills_store.bus.message_types[-1] == "ovos.pip.uninstall.failed" + assert skills_store.bus.message_data[-1] == {"error": "no packages to install"} + skills_store.pip_uninstall.assert_not_called() + + +@pytest.mark.parametrize('skills_store', [{"allow_pip": True}], indirect=True) +def test_handle_uninstall_python_success(skills_store): + skills_store.play_error_sound = Mock() + skills_store.pip_uninstall = Mock() + packages = ["requests", "fastapi"] + skills_store.handle_uninstall_python(Message(msg_type="test", data={"packages": packages})) + skills_store.play_error_sound.assert_not_called() + skills_store.pip_uninstall.assert_called_once_with(packages) + assert skills_store.bus.message_types[-1] == "ovos.pip.uninstall.complete" + assert skills_store.bus.message_data[-1] == {} + + +if __name__ == "__main__": + pytest.main()