From 715ce7d7a4350ca44c4c2591fffc593280f256a5 Mon Sep 17 00:00:00 2001 From: Mike Gray Date: Fri, 16 Feb 2024 22:41:36 -0600 Subject: [PATCH 1/7] feat: populate default settings, warn user if key missing --- __init__.py | 19 +++++++++++++--- test/__init__.py | 0 test/test_main.py | 58 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 test/__init__.py create mode 100644 test/test_main.py diff --git a/__init__.py b/__init__.py index 472e172..b3c4fea 100644 --- a/__init__.py +++ b/__init__.py @@ -5,6 +5,11 @@ from ovos_workshop.skills.fallback import FallbackSkill +DEFAULT_SETTINGS = { + "persona": "You are a helpful voice assistant with a friendly tone and fun sense of humor. You respond in 40 words or fewer.", + "model": "gpt-3.5-turbo", +} + class ChatGPTSkill(FallbackSkill): sessions = {} @@ -18,6 +23,8 @@ def runtime_requirements(self): ) def initialize(self): + self.settings.merge(DEFAULT_SETTINGS, new_only=True) + self.unconfigured_message = f"ChatGPT not configured yet, please set your API key in {self.settings_path}" self.add_event("speak", self.handle_speak) self.add_event("recognizer_loop:utterance", self.handle_utterance) self.register_fallback(self.ask_chatgpt, 85) @@ -25,7 +32,11 @@ def initialize(self): @property def chat(self): """created fresh to allow key/url rotation when settings.json is edited""" - return OpenAIPersonaSolver(config=self.settings) + try: + return OpenAIPersonaSolver(config=self.settings) + except Exception as err: + self.log.error(err) + return None def handle_utterance(self, message): utt = message.data.get("utterances")[0] @@ -75,13 +86,15 @@ def _async_ask(self, message): for utt in self.chat.stream_utterances(utterance): answered = True self.speak(utt) - except: # speak error on any network issue / no credits etc - pass + except Exception as err: # speak error on any network issue / no credits etc + self.log.error(err) + self.speak_dialog("gpt_error") if not answered: self.speak_dialog("gpt_error") def ask_chatgpt(self, message): if "key" not in self.settings: + self.log.error(self.unconfigured_message) return False # ChatGPT not configured yet utterance = message.data["utterance"] self.speak_dialog("asking") diff --git a/test/__init__.py b/test/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/test/test_main.py b/test/test_main.py new file mode 100644 index 0000000..65b6984 --- /dev/null +++ b/test/test_main.py @@ -0,0 +1,58 @@ +from time import sleep +from unittest.mock import Mock, MagicMock +import pytest +from ovos_utils.fakebus import FakeBus + +from skill_ovos_fallback_chatgpt import ChatGPTSkill, DEFAULT_SETTINGS + + +class TestChatGPTSkill: + bus = FakeBus() + bus.emitter = bus.ee + bus.run_forever() + + def test_default_no_key(self): + skill = ChatGPTSkill(bus=self.bus, skill_id="test") + skill.speak = Mock() + skill.speak_dialog = Mock() + skill.play_audio = Mock() + skill.log = MagicMock() + while not skill.is_fully_initialized: + sleep(0.5) + assert not skill.settings.get("key") + skill.ask_chatgpt("Will my test pass?") + skill.log.error.assert_called() + skill.speak_dialog.assert_not_called() # no key, we log an error before speaking ever happens + assert skill.settings.get("persona") == DEFAULT_SETTINGS["persona"] + assert skill.settings.get("model") == DEFAULT_SETTINGS["model"] + + def test_default_with_key(self): + skill = ChatGPTSkill(bus=self.bus, skill_id="test", settings={"key": "test"}) + skill.speak = Mock() + skill.speak_dialog = Mock() + skill.play_audio = Mock() + while not skill.is_fully_initialized: + sleep(0.5) + assert skill.settings.get("key") == "test" + assert skill.settings.get("persona") == DEFAULT_SETTINGS["persona"] + assert skill.settings.get("model") == DEFAULT_SETTINGS["model"] + + def test_overriding_all_settings(self): + skill = ChatGPTSkill(bus=self.bus, skill_id="test", settings={ + "key": "test", + "persona": "I am a test persona", + "model": "gpt-4-nitro" + }) + skill.speak = Mock() + skill.speak_dialog = Mock() + skill.play_audio = Mock() + while not skill.is_fully_initialized: + sleep(0.5) + assert skill.settings.get("key") == "test" + assert skill.settings.get("persona") == "I am a test persona" + assert skill.settings.get("model") == "gpt-4-nitro" + assert skill.settings.get("persona") != DEFAULT_SETTINGS["persona"] + assert skill.settings.get("model") != DEFAULT_SETTINGS["model"] + +if __name__ == "__main__": + pytest.main() From 5941bd72a383ecb473cc876becaf3ce20e750705 Mon Sep 17 00:00:00 2001 From: Mike Gray Date: Fri, 16 Feb 2024 22:43:39 -0600 Subject: [PATCH 2/7] satisfy our pipeline overlords --- test/{ => unittests}/__init__.py | 0 test/{ => unittests}/test_main.py | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename test/{ => unittests}/__init__.py (100%) rename test/{ => unittests}/test_main.py (100%) diff --git a/test/__init__.py b/test/unittests/__init__.py similarity index 100% rename from test/__init__.py rename to test/unittests/__init__.py diff --git a/test/test_main.py b/test/unittests/test_main.py similarity index 100% rename from test/test_main.py rename to test/unittests/test_main.py From f87381289a261214031168b3d6b67b73a14101cd Mon Sep 17 00:00:00 2001 From: Mike Gray Date: Fri, 16 Feb 2024 22:50:58 -0600 Subject: [PATCH 3/7] bump unit tests --- .github/workflows/unit_tests.yml | 52 ++++++++++++++++---------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 0322f31..6648c9c 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -4,30 +4,30 @@ on: branches: - dev paths-ignore: - - 'version.py' - - 'requirements.txt' - - 'examples/**' - - '.github/**' - - '.gitignore' - - 'LICENSE' - - 'CHANGELOG.md' - - 'MANIFEST.in' - - 'readme.md' - - 'scripts/**' + - "version.py" + - "requirements.txt" + - "examples/**" + - ".github/**" + - ".gitignore" + - "LICENSE" + - "CHANGELOG.md" + - "MANIFEST.in" + - "readme.md" + - "scripts/**" push: branches: - master paths-ignore: - - 'version.py' - - 'requirements.txt' - - 'examples/**' - - '.github/**' - - '.gitignore' - - 'LICENSE' - - 'CHANGELOG.md' - - 'MANIFEST.in' - - 'readme.md' - - 'scripts/**' + - "version.py" + - "requirements.txt" + - "examples/**" + - ".github/**" + - ".gitignore" + - "LICENSE" + - "CHANGELOG.md" + - "MANIFEST.in" + - "readme.md" + - "scripts/**" workflow_dispatch: jobs: @@ -35,12 +35,12 @@ jobs: strategy: max-parallel: 2 matrix: - python-version: [ 3.7, 3.8, 3.9, "3.10" ] + python-version: [3.8, 3.9, "3.10", "3.11"] runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Set up python ${{ matrix.python-version }} - uses: actions/setup-python@v2 + uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} - name: Install System Dependencies @@ -57,17 +57,17 @@ jobs: sudo apt install libfann-dev - name: Install ovos dependencies run: | - pip install ovos-plugin-manager ovos-core[skills_lgpl]>=0.0.6a21 + pip install ovos-plugin-manager ovos-core[skills_lgpl]>=0.0.7 - name: Install core repo run: | pip install . - name: Run unittests run: | - pytest --cov=ovos-skill-template-repo --cov-report xml test/unittests + pytest --cov=. --cov-report xml test/unittests # NOTE: additional pytest invocations should also add the --cov-append flag # or they will overwrite previous invocations' coverage reports # (for an example, see OVOS Skill Manager's workflow) - name: Upload coverage env: CODECOV_TOKEN: ${{secrets.CODECOV_TOKEN}} - uses: codecov/codecov-action@v2 + uses: codecov/codecov-action@v4 From f2958e061d4bdabcb08848dcd7901a79c1f6abe7 Mon Sep 17 00:00:00 2001 From: Mike Gray Date: Fri, 16 Feb 2024 23:03:25 -0600 Subject: [PATCH 4/7] another bump --- .github/workflows/unit_tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 6648c9c..8a48502 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -57,7 +57,7 @@ jobs: sudo apt install libfann-dev - name: Install ovos dependencies run: | - pip install ovos-plugin-manager ovos-core[skills_lgpl]>=0.0.7 + pip install --pre ovos-plugin-manager ovos-core[skills_lgpl]>=0.0.8 - name: Install core repo run: | pip install . From 130ab7b0c92ba8c07b58f55a9f004a4fd0e747bf Mon Sep 17 00:00:00 2001 From: Mike Gray Date: Sat, 17 Feb 2024 10:18:54 -0600 Subject: [PATCH 5/7] address feedback --- .github/workflows/unit_tests.yml | 1 + __init__.py | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 8a48502..4d19572 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -61,6 +61,7 @@ jobs: - name: Install core repo run: | pip install . + pip list - name: Run unittests run: | pytest --cov=. --cov-report xml test/unittests diff --git a/__init__.py b/__init__.py index b3c4fea..c249b3f 100644 --- a/__init__.py +++ b/__init__.py @@ -88,7 +88,6 @@ def _async_ask(self, message): self.speak(utt) except Exception as err: # speak error on any network issue / no credits etc self.log.error(err) - self.speak_dialog("gpt_error") if not answered: self.speak_dialog("gpt_error") From 6c08fbd9b66ddadba264f8cf36dd1a9666c1026b Mon Sep 17 00:00:00 2001 From: Mike Gray Date: Sat, 17 Feb 2024 11:06:26 -0600 Subject: [PATCH 6/7] pull in version of workshop with fix for unittests --- .github/workflows/unit_tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 4d19572..1091516 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -61,6 +61,7 @@ jobs: - name: Install core repo run: | pip install . + pip install --pre -U ovos-workshop # 0.0.16a5 required for the class to load properly in unit tests pip list - name: Run unittests run: | From 2f1f10153e975b99fca93c145b12246f01dd45c8 Mon Sep 17 00:00:00 2001 From: Mike Gray Date: Sat, 17 Feb 2024 11:50:17 -0600 Subject: [PATCH 7/7] revamp tests --- __init__.py | 3 +- test/unittests/test_main.py | 98 ++++++++++++++++++++++--------------- 2 files changed, 59 insertions(+), 42 deletions(-) diff --git a/__init__.py b/__init__.py index c249b3f..4886497 100644 --- a/__init__.py +++ b/__init__.py @@ -24,7 +24,6 @@ def runtime_requirements(self): def initialize(self): self.settings.merge(DEFAULT_SETTINGS, new_only=True) - self.unconfigured_message = f"ChatGPT not configured yet, please set your API key in {self.settings_path}" self.add_event("speak", self.handle_speak) self.add_event("recognizer_loop:utterance", self.handle_utterance) self.register_fallback(self.ask_chatgpt, 85) @@ -93,7 +92,7 @@ def _async_ask(self, message): def ask_chatgpt(self, message): if "key" not in self.settings: - self.log.error(self.unconfigured_message) + self.log.error("ChatGPT not configured yet, please set your API key in %s", self.settings.path) return False # ChatGPT not configured yet utterance = message.data["utterance"] self.speak_dialog("asking") diff --git a/test/unittests/test_main.py b/test/unittests/test_main.py index 65b6984..6982536 100644 --- a/test/unittests/test_main.py +++ b/test/unittests/test_main.py @@ -1,58 +1,76 @@ +import shutil +from os import environ, getenv +from os.path import dirname, join +from threading import Event from time import sleep -from unittest.mock import Mock, MagicMock +from unittest import TestCase +from unittest.mock import MagicMock, Mock + import pytest from ovos_utils.fakebus import FakeBus +from skill_ovos_fallback_chatgpt import DEFAULT_SETTINGS, ChatGPTSkill -from skill_ovos_fallback_chatgpt import ChatGPTSkill, DEFAULT_SETTINGS +class TestChatGPTSkill(TestCase): + # Define test directories + test_fs = join(dirname(__file__), "skill_fs") + data_dir = join(test_fs, "data") + conf_dir = join(test_fs, "config") + environ["XDG_DATA_HOME"] = data_dir + environ["XDG_CONFIG_HOME"] = conf_dir -class TestChatGPTSkill: bus = FakeBus() bus.emitter = bus.ee + bus.connected_event = Event() + bus.connected_event.set() bus.run_forever() + test_skill_id = 'test_skill.test' + + skill = None + + @classmethod + def setUpClass(cls) -> None: + # Get test skill + cls.skill = ChatGPTSkill(skill_id=cls.test_skill_id, bus=cls.bus) + # Override speak and speak_dialog to test passed arguments + cls.skill.speak = Mock() + cls.skill.speak_dialog = Mock() + + def setUp(self): + self.skill.speak.reset_mock() + self.skill.speak_dialog.reset_mock() + self.skill.play_audio = Mock() + self.skill.log = MagicMock() + + @classmethod + def tearDownClass(cls) -> None: + shutil.rmtree(cls.test_fs) def test_default_no_key(self): - skill = ChatGPTSkill(bus=self.bus, skill_id="test") - skill.speak = Mock() - skill.speak_dialog = Mock() - skill.play_audio = Mock() - skill.log = MagicMock() - while not skill.is_fully_initialized: - sleep(0.5) - assert not skill.settings.get("key") - skill.ask_chatgpt("Will my test pass?") - skill.log.error.assert_called() - skill.speak_dialog.assert_not_called() # no key, we log an error before speaking ever happens - assert skill.settings.get("persona") == DEFAULT_SETTINGS["persona"] - assert skill.settings.get("model") == DEFAULT_SETTINGS["model"] + assert not self.skill.settings.get("key") + self.skill.ask_chatgpt("Will my test pass?") + self.skill.log.error.assert_called() + self.skill.speak_dialog.assert_not_called() # no key, we log an error before speaking ever happens + assert self.skill.settings.get("persona") == DEFAULT_SETTINGS["persona"] + assert self.skill.settings.get("model") == DEFAULT_SETTINGS["model"] def test_default_with_key(self): - skill = ChatGPTSkill(bus=self.bus, skill_id="test", settings={"key": "test"}) - skill.speak = Mock() - skill.speak_dialog = Mock() - skill.play_audio = Mock() - while not skill.is_fully_initialized: - sleep(0.5) - assert skill.settings.get("key") == "test" - assert skill.settings.get("persona") == DEFAULT_SETTINGS["persona"] - assert skill.settings.get("model") == DEFAULT_SETTINGS["model"] + self.skill.settings["key"] = "test" + self.skill.settings.store() + assert self.skill.settings.get("key") == "test" + assert self.skill.settings.get("persona") == DEFAULT_SETTINGS["persona"] + assert self.skill.settings.get("model") == DEFAULT_SETTINGS["model"] def test_overriding_all_settings(self): - skill = ChatGPTSkill(bus=self.bus, skill_id="test", settings={ - "key": "test", - "persona": "I am a test persona", - "model": "gpt-4-nitro" - }) - skill.speak = Mock() - skill.speak_dialog = Mock() - skill.play_audio = Mock() - while not skill.is_fully_initialized: - sleep(0.5) - assert skill.settings.get("key") == "test" - assert skill.settings.get("persona") == "I am a test persona" - assert skill.settings.get("model") == "gpt-4-nitro" - assert skill.settings.get("persona") != DEFAULT_SETTINGS["persona"] - assert skill.settings.get("model") != DEFAULT_SETTINGS["model"] + self.skill.settings["key"] = "test" + self.skill.settings["persona"] = "I am a test persona" + self.skill.settings["model"] = "gpt-4-nitro" + self.skill.settings.store() + assert self.skill.settings.get("key") == "test" + assert self.skill.settings.get("persona") == "I am a test persona" + assert self.skill.settings.get("model") == "gpt-4-nitro" + assert self.skill.settings.get("persona") != DEFAULT_SETTINGS["persona"] + assert self.skill.settings.get("model") != DEFAULT_SETTINGS["model"] if __name__ == "__main__": pytest.main()