Merge pull request #184 from HackSoc/pymongo4

Upgrade to pymongo 4.x

requirements.txt

@@ -6,6 +6,7 @@ aioresponses==0.7.3
 pytest-cov
 asynctest==0.13.0
 aiofastforward==0.0.24
+time-machine==2.6.0
 mongomock
 
 # Requirements for documentation

setup.py

@@ -16,7 +16,7 @@
     install_requires=[
         'click>=6.2,<7.0',
         'straight.plugin==1.4.0-post-1',
-        'pymongo>=3.6.0',
+        'pymongo>=4.0.1',
         'requests>=2.9.1,<3.0.0',
         'lxml>=2.3.5',
         'aiogoogle>=0.1.13',

src/csbot/plugins/last.py

@@ -24,7 +24,9 @@ def last(self, nick, channel=None, msgtype=None):
         if msgtype is not None:
             search['type'] = msgtype
 
-        return self.db.find_one(search, sort=[('when', pymongo.DESCENDING)])
+        # Additional sorting by _id to make sort order stable for messages that arrive in the same millisecond
+        # (which sometimes happens during tests).
+        return self.db.find_one(search, sort=[('when', pymongo.DESCENDING), ('_id', pymongo.DESCENDING)])
 
     def last_message(self, nick, channel=None):
         """Get the last message sent by a nick, optionally filtering
@@ -104,8 +106,7 @@ def _schedule_update(self, e, query, update):
 
     @Plugin.hook('last.update')
     def _apply_update(self, e):
-        self.db.remove(e['query'])
-        self.db.insert(e['update'])
+        self.db.replace_one(e['query'], e['update'], upsert=True)
 
     @Plugin.command('seen', help=('seen nick [type]: show the last thing'
                                   ' said by a nick in this channel, optionally'

src/csbot/plugins/termdates.py

@@ -196,8 +196,16 @@ def termdates_set(self, e):
         # Save to the database. As we don't touch the _id attribute in this
         # method, this will cause `save` to override the previously-loaded
         # entry (if there is one).
-        self.db_terms.save(self.terms)
-        self.db_weeks.save(self.weeks)
+        if '_id' in self.terms:
+            self.db_terms.replace_one({'_id': self.terms['_id']}, self.terms, upsert=True)
+        else:
+            res = self.db_terms.insert_one(self.terms)
+            self.terms['_id'] = res.inserted_id
+        if '_id' in self.weeks:
+            self.db_weeks.replace_one({'_id': self.weeks['_id']}, self.weeks, upsert=True)
+        else:
+            res = self.db_weeks.insert_one(self.weeks)
+            self.weeks['_id'] = res.inserted_id
 
         # Finally, we're initialised!
         self.initialised = True

tests/conftest.py

@@ -1,3 +1,4 @@
+from __future__ import annotations
 import asyncio
 from textwrap import dedent
 from unittest import mock
@@ -124,16 +125,51 @@ def receive(self, lines):
             lines = [lines]
         return [self.client.line_received(line) for line in lines]
 
-    def assert_sent(self, lines):
+    def assert_sent(self, matchers, *, any_order=False, reset_mock=True):
         """Check that a list of (unicode) strings have been sent.
 
         Resets the mock so the next call will not contain what was checked by
         this call.
         """
-        if isinstance(lines, str):
-            lines = [lines]
-        self.client.send_line.assert_has_calls([mock.call(line) for line in lines])
-        self.client.send_line.reset_mock()
+        sent_lines = [args[0] for name, args, kwargs in self.client.send_line.mock_calls]
+
+        if callable(matchers) or isinstance(matchers, str):
+            matchers = [matchers]
+        matchers = [LineMatcher.equals(matcher) if not callable(matcher) else matcher
+                    for matcher in matchers]
+
+        if not matchers:
+            pass
+        elif any_order:
+            for matcher in matchers:
+                assert any(matcher(line) for line in sent_lines), f"sent line not found: {matcher}"
+        else:
+            # Find the start of the matching run of sent messages
+            start = 0
+            while start < len(sent_lines) and not matchers[0](sent_lines[start]):
+                start += 1
+            for i, matcher in enumerate(matchers):
+                assert start + i < len(sent_lines), f"no line matching {matcher} in {sent_lines}"
+                assert matcher(sent_lines[start + i]), f"expected {sent_lines[start + i]!r} to match {matcher}"
+
+        if reset_mock:
+            self.client.send_line.reset_mock()
+
+
+class LineMatcher:
+    def __init__(self, f, description):
+        self.f = f
+        self.description = description
+
+    def __call__(self, line):
+        return self.f(line)
+
+    def __repr__(self):
+        return self.description
+
+    @classmethod
+    def equals(cls, other):
+        return cls(lambda line: line == other, f"`line == {other!r}`")
 
 
 @pytest.fixture
@@ -170,7 +206,7 @@ def bot_helper_class():
 
 
 @pytest.fixture
-def bot_helper(irc_client, bot_helper_class):
+def bot_helper(irc_client, bot_helper_class) -> BotHelper:
     irc_client.bot_setup()
     return bot_helper_class(irc_client)
 

tests/test_irc.py

@@ -193,62 +193,38 @@ async def test_client_PING(self, fast_forward, run_client):
         run_client.client.send_line.assert_not_called()
         # Advance time, test that a ping was sent
         await fast_forward(4)
-        assert run_client.client.send_line.mock_calls == [
-            mock.call('PING 1'),
-        ]
+        run_client.assert_sent(['PING 1'], reset_mock=False)
         # Advance time again, test that the right number of pings was sent
         await fast_forward(12)
-        assert run_client.client.send_line.mock_calls == [
-            mock.call('PING 1'),
-            mock.call('PING 2'),
-            mock.call('PING 3'),
-            mock.call('PING 4'),
-            mock.call('PING 5'),
-        ]
+        run_client.assert_sent(['PING 1', 'PING 2', 'PING 3', 'PING 4', 'PING 5'], reset_mock=False)
         # Disconnect, advance time, test that no more pings were sent
         run_client.client.disconnect()
         await run_client.client.disconnected.wait()
         await fast_forward(12)
-        assert run_client.client.send_line.mock_calls == [
-            mock.call('PING 1'),
-            mock.call('PING 2'),
-            mock.call('PING 3'),
-            mock.call('PING 4'),
-            mock.call('PING 5'),
-        ]
+        run_client.assert_sent(['PING 1', 'PING 2', 'PING 3', 'PING 4', 'PING 5'], reset_mock=False)
 
     async def test_client_PING_only_when_needed(self, fast_forward, run_client):
         """Check that client PING commands are sent relative to the last received message."""
         run_client.reset_mock()
         run_client.client.send_line.assert_not_called()
         # Advance time to just before the second PING, check that the first PING was sent
         await fast_forward(5)
-        assert run_client.client.send_line.mock_calls == [
-            mock.call('PING 1'),
-        ]
+        run_client.assert_sent(['PING 1'], reset_mock=False)
         # Receive a message, this should reset the PING timer
         run_client.receive(':nick!user@host PRIVMSG #channel :foo')
         # Advance time to just after when the second PING would happen without any messages
         # received, check that still only one PING was sent
         await fast_forward(2)
-        assert run_client.client.send_line.mock_calls == [
-            mock.call('PING 1'),
-        ]
+        run_client.assert_sent(['PING 1'], reset_mock=False)
         # Advance time to 4 seconds after the last message was received, and check that another
         # PING has now been sent
         await fast_forward(2)
-        assert run_client.client.send_line.mock_calls == [
-            mock.call('PING 1'),
-            mock.call('PING 2'),
-        ]
+        run_client.assert_sent(['PING 1', 'PING 2'], reset_mock=False)
         # Disconnect, advance time, test that no more pings were sent
         run_client.client.disconnect()
         await run_client.client.disconnected.wait()
         await fast_forward(12)
-        assert run_client.client.send_line.mock_calls == [
-            mock.call('PING 1'),
-            mock.call('PING 2'),
-        ]
+        run_client.assert_sent(['PING 1', 'PING 2'], reset_mock=False)
 
 
 def test_PING_PONG(irc_client_helper):

tests/test_plugin_last.py

@@ -0,0 +1,143 @@
+import asyncio
+
+import pytest
+
+from csbot.plugins.last import Last
+
+
+pytestmark = [
+    pytest.mark.bot(config="""\
+        ["@bot"]
+        plugins = ["mongodb", "last"]
+
+        [mongodb]
+        mode = "mock"
+    """),
+    pytest.mark.usefixtures("run_client"),
+]
+
+
+def diff_dict(actual: dict, expected: dict) -> dict:
+    """Find items in *expected* that are different at the same keys in *actual*, returning a dict
+    mapping the offending key to a dict with "expected" and "actual" items."""
+    diff = dict()
+    for k, v in expected.items():
+        actual_value = actual.get(k)
+        expected_value = expected.get(k)
+        if actual_value != expected_value:
+            diff[k] = dict(actual=actual_value, expected=expected_value)
+    return diff
+
+
+async def test_message_types(bot_helper):
+    plugin: Last = bot_helper["last"]
+
+    # Starting state: should have no "last message" for a user
+    assert plugin.last("Nick") is None
+    assert plugin.last_message("Nick") is None
+    assert plugin.last_action("Nick") is None
+    assert plugin.last_command("Nick") is None
+
+    # Receive a PRIVMSG from the user
+    await bot_helper.client.line_received(":Nick!~user@hostname PRIVMSG #channel :Example message")
+    # Check that message was recorded correctly
+    assert diff_dict(plugin.last("Nick"), {"nick": "Nick", "message": "Example message"}) == {}
+    # Check that message was only recorded in the correct category
+    assert plugin.last_message("Nick") == plugin.last("Nick")
+    assert not plugin.last_action("Nick") == plugin.last("Nick")
+    assert not plugin.last_command("Nick") == plugin.last("Nick")
+
+    # Receive a CTCP ACTION from the user (inside a PRIVMSG)
+    await bot_helper.client.line_received(":Nick!~user@hostname PRIVMSG #channel :\x01ACTION emotes\x01")
+    # Check that message was recorded correctly
+    assert diff_dict(plugin.last("Nick"), {"nick": "Nick", "message": "emotes"}) == {}
+    # Check that message was only recorded in the correct category
+    assert not plugin.last_message("Nick") == plugin.last("Nick")
+    assert plugin.last_action("Nick") == plugin.last("Nick")
+    assert not plugin.last_command("Nick") == plugin.last("Nick")
+
+    # Receive a bot command from the user (inside a PRIVMSG)
+    await bot_helper.client.line_received(":Nick!~user@hostname PRIVMSG #channel :!help")
+    # Check that message was recorded correctly
+    assert diff_dict(plugin.last("Nick"), {"nick": "Nick", "message": "!help"}) == {}
+    # Check that message was only recorded in the correct category
+    assert not plugin.last_message("Nick") == plugin.last("Nick")
+    assert not plugin.last_action("Nick") == plugin.last("Nick")
+    assert plugin.last_command("Nick") == plugin.last("Nick")
+
+    # Final confirmation that the "message", "action" and "command" message types were all recorded separately
+    assert diff_dict(plugin.last_message("Nick"), {"nick": "Nick", "message": "Example message"}) == {}
+    assert diff_dict(plugin.last_action("Nick"), {"nick": "Nick", "message": "emotes"}) == {}
+    assert diff_dict(plugin.last_command("Nick"), {"nick": "Nick", "message": "!help"}) == {}
+
+    # Also there shouldn't be any records for a different nick
+    assert plugin.last("OtherNick") is None
+
+
+async def test_channel_filter(bot_helper):
+    plugin: Last = bot_helper["last"]
+
+    # Starting state: should have no "last message" for a user
+    assert plugin.last("Nick") is None
+    assert plugin.last("Nick", channel="#a") is None
+    assert plugin.last("Nick", channel="#b") is None
+
+    # Receive a PRIVMSG from the user in #a
+    await bot_helper.client.line_received(":Nick!~user@hostname PRIVMSG #a :Message A")
+    # Check that the message was recorded correctly
+    assert diff_dict(plugin.last("Nick"), {"nick": "Nick", "channel": "#a", "message": "Message A"}) == {}
+    # Check that channel filter applies correctly
+    assert plugin.last("Nick", channel="#a") == plugin.last("Nick")
+    assert not plugin.last("Nick", channel="#b") == plugin.last("Nick")
+
+    # Receive a PRIVMSG from the user in #b
+    await bot_helper.client.line_received(":Nick!~user@hostname PRIVMSG #b :Message B")
+    # Check that the message was recorded correctly
+    assert diff_dict(plugin.last("Nick"), {"nick": "Nick", "channel": "#b", "message": "Message B"}) == {}
+    # Check that channel filter applies correctly
+    assert not plugin.last("Nick", channel="#a") == plugin.last("Nick")
+    assert plugin.last("Nick", channel="#b") == plugin.last("Nick")
+
+    # Final confirmation that the latest message for each channel is stored
+    assert diff_dict(plugin.last("Nick", channel="#a"), {"nick": "Nick", "channel": "#a", "message": "Message A"}) == {}
+    assert diff_dict(plugin.last("Nick", channel="#b"), {"nick": "Nick", "channel": "#b", "message": "Message B"}) == {}
+
+    # Also there shouldn't be any records for a different channel
+    assert plugin.last("Nick", channel="#c") is None
+
+
+async def test_seen_command(bot_helper):
+    bot_helper.reset_mock()
+
+    # !seen for a nick not yet seen
+    await asyncio.wait(bot_helper.receive(":A!~user@hostname PRIVMSG #a :!seen B"))
+    bot_helper.assert_sent("NOTICE #a :Nothing recorded for B")
+
+    # !seen for a nick only seen in a different channel
+    await asyncio.wait(bot_helper.receive(":B!~user@hostname PRIVMSG #b :First message"))
+    await asyncio.wait(bot_helper.receive(":A!~user@hostname PRIVMSG #a :!seen B"))
+    bot_helper.assert_sent("NOTICE #a :Nothing recorded for B")
+
+    # !seen for nick seen in the same channel
+    await asyncio.wait(bot_helper.receive(":A!~user@hostname PRIVMSG #b :!seen B"))
+    bot_helper.assert_sent(lambda line: "<B> First message" in line)
+
+    # Now seen in both channels, !seen should only return the message relating to the current channel
+    await asyncio.wait(bot_helper.receive(":B!~user@hostname PRIVMSG #a :Second message"))
+    await asyncio.wait(bot_helper.receive(":A!~user@hostname PRIVMSG #a :!seen B"))
+    bot_helper.assert_sent(lambda line: "<B> Second message" in line)
+    await asyncio.wait(bot_helper.receive(":A!~user@hostname PRIVMSG #b :!seen B"))
+    bot_helper.assert_sent(lambda line: "<B> First message" in line)
+
+    # !seen on own nick should get the !seen command itself (because it makes more sense than "Nothing recorded")
+    await asyncio.wait(bot_helper.receive(":B!~user@hostname PRIVMSG #a :!seen B"))
+    bot_helper.assert_sent(lambda line: "<B> !seen B" in line)
+
+    # Check different formatting for actions
+    await asyncio.wait(bot_helper.receive(":B!~user@hostname PRIVMSG #a :\x01ACTION does something\x01"))
+    await asyncio.wait(bot_helper.receive(":A!~user@hostname PRIVMSG #a :!seen B"))
+    bot_helper.assert_sent(lambda line: "* B does something" in line)
+
+    # Error when bad message type is specified
+    await asyncio.wait(bot_helper.receive(":A!~user@hostname PRIVMSG #a :!seen B foobar"))
+    bot_helper.assert_sent("NOTICE #a :Bad filter: foobar. Accepted are \"message\", \"command\", and \"action\".")

tests/test_plugin_linkinfo.py

@@ -249,9 +249,7 @@ async def handler(url, **kwargs):
         event.set()
         await asyncio.wait(futures, timeout=0.1)
         assert all(f.done() for f in futures)
-        bot_helper.client.send_line.assert_has_calls([
-            mock.call('NOTICE #channel :foo'),
-        ])
+        bot_helper.assert_sent('NOTICE #channel :foo')
 
     async def test_non_blocking_command(self, event_loop, bot_helper, aioresponses):
         bot_helper.reset_mock()
@@ -281,7 +279,5 @@ async def handler(url, **kwargs):
         event.set()
         await asyncio.wait(futures, timeout=0.1)
         assert all(f.done() for f in futures)
-        bot_helper.client.send_line.assert_has_calls([
-            mock.call('NOTICE #channel :Error: Content-Type not HTML-ish: '
-                      'application/octet-stream (http://example.com/)'),
-        ])
+        bot_helper.assert_sent('NOTICE #channel :Error: Content-Type not HTML-ish: '
+                               'application/octet-stream (http://example.com/)')