Merge pull request #690 from TotallyNotRobots/expand-coverage

Add more tests for db access

Author: linuxdaemon

.devcontainer/devcontainer.json

@@ -37,7 +37,6 @@
             ],
             "settings": {
                 "python.pythonPath": "/usr/local/bin/python",
-                "python.testing.pytestArgs": ["--no-cov"],
                 "terminal.integrated.profiles.linux": {
                     "zsh": {
                         "path": "/usr/bin/zsh"

.vscode/settings.json

@@ -4,6 +4,11 @@
     ],
     "python.testing.unittestEnabled": false,
     "python.testing.pytestEnabled": true,
-    "coverage-gutters.showLineCoverage": true,
-    "coverage-gutters.showRulerCoverage": true
+    "python.analysis.enablePytestSupport": true,
+    "[python]": {
+        "diffEditor.ignoreTrimWhitespace": false,
+        "editor.formatOnType": true,
+        "editor.wordBasedSuggestions": "off",
+        "editor.defaultFormatter": "ms-python.black-formatter"
+    }
 }

cloudbot/bot.py

@@ -29,25 +29,28 @@
 logger = logging.getLogger("cloudbot")
 
 
+class AbstractBot:
+    def __init__(self, *, config: Config) -> None:
+        self.config = config
+
+
 class BotInstanceHolder:
-    def __init__(self):
-        self._instance = None
+    def __init__(self) -> None:
+        self._instance: Optional[AbstractBot] = None
 
-    def get(self):
-        # type: () -> CloudBot
+    def get(self) -> Optional[AbstractBot]:
         return self._instance
 
-    def set(self, value):
-        # type: (CloudBot) -> None
+    def set(self, value: Optional[AbstractBot]) -> None:
         self._instance = value
 
     @property
-    def config(self):
-        # type: () -> Config
-        if not self.get():
+    def config(self) -> Config:
+        instance = self.get()
+        if not instance:
             raise ValueError("No bot instance available")
 
-        return self.get().config
+        return instance.config
 
 
 # Store a global instance of the bot to allow easier access to global data
@@ -88,7 +91,7 @@ def get_cmd_regex(event):
     return cmd_re
 
 
-class CloudBot:
+class CloudBot(AbstractBot):
     def __init__(
         self,
         *,
@@ -127,7 +130,7 @@ def __init__(
             self.data_path.mkdir(parents=True)
 
         # set up config
-        self.config = Config(self)
+        super().__init__(config=Config(self))
         logger.debug("Config system initialised.")
 
         self.executor = ThreadPoolExecutor(

plugins/librefm.py

@@ -84,7 +84,7 @@ def librefm(text, nick, db, event):
 
     if (
         "track" not in response["recenttracks"]
-        or response["recenttracks"]["track"]
+        or not response["recenttracks"]["track"]
     ):
         return f'No recent tracks for user "{user}" found.'
 
@@ -243,7 +243,7 @@ def toptrack(text, nick):
         return "Error: {}.".format(data["message"])
 
     out = f"{username}'s favorite songs: "
-    for r in range(5):
+    for r in range(min(5, len(data["toptracks"]["track"]))):
         track_name = data["toptracks"]["track"][r]["name"]
         artist_name = data["toptracks"]["track"][r]["artist"]["name"]
         play_count = data["toptracks"]["track"][r]["playcount"]

plugins/quote.py

@@ -9,6 +9,7 @@
     String,
     Table,
     func,
+    inspect,
     not_,
     select,
 )
@@ -44,8 +45,9 @@ def migrate_table(db, logger):
         Column("deleted", String(5), default=0),
         PrimaryKeyConstraint("chan", "nick", "time"),
     )
+    inspector = inspect(db.bind)
 
-    if not old_table.exists():
+    if not inspector.has_table(old_table.name):
         database.metadata.remove(old_table)
         return
 
@@ -106,19 +108,6 @@ def add_quote(db, chan, target, sender, message):
     return "Quote added."
 
 
-def del_quote(db, nick, msg):
-    """Deletes a quote from a nick"""
-    query = (
-        qtable.update()
-        .where(qtable.c.chan == 1)
-        .where(qtable.c.nick == nick.lower())
-        .where(qtable.c.msg == msg)
-        .values(deleted=True)
-    )
-    db.execute(query)
-    db.commit()
-
-
 def get_quote_num(num, count, name):
     """Returns the quote number to fetch from the DB"""
     if num:  # Make sure num is a number if it isn't false

tests/conftest.py

@@ -59,13 +59,14 @@ def mock_db(tmp_path):
 
 
 @pytest.fixture()
-def mock_bot_factory(event_loop, tmp_path):
+def mock_bot_factory(event_loop, tmp_path, unset_bot):
     instances: List[MockBot] = []
 
     def _factory(*args, **kwargs):
         kwargs.setdefault("loop", event_loop)
         kwargs.setdefault("base_dir", tmp_path)
         _bot = MockBot(*args, **kwargs)
+        bot.set(_bot)
         instances.append(_bot)
         return _bot
 

tests/core_tests/test_bot.py

@@ -4,6 +4,7 @@
 import pytest
 from sqlalchemy import Column, String, Table
 
+import cloudbot.bot
 from cloudbot import hook
 from cloudbot.bot import CloudBot, clean_name, get_cmd_regex
 from cloudbot.event import Event, EventType
@@ -15,6 +16,12 @@
 from tests.util.mock_db import MockDB
 
 
+def test_no_instance_config(unset_bot):
+    cloudbot.bot.bot.set(None)
+    with pytest.raises(ValueError):
+        _ = cloudbot.bot.bot.config
+
+
 @pytest.mark.asyncio()
 async def test_migrate_db(
     mock_db, mock_bot_factory, event_loop, mock_requests, tmp_path