From 15f9875ae5ad64fd1b74153351b0289ff6514dd8 Mon Sep 17 00:00:00 2001
From: Valentin Lorentz <progval+git@progval.net>
Date: Sun, 4 Jul 2021 17:15:04 +0200
Subject: [PATCH] chathistory: Parametrize tests by subcommand

This means that:

* if one subcommand implementation is buggy, other subcommands are still tested and
  have a chance to pass
* we can exclude known-buggy subcommands from the Makefile
* when a test failure happens, we get much shorter logs (only logs for
  that subcommand's I/O)
---
 Makefile                                 |  5 ++-
 irctest/server_tests/test_chathistory.py | 48 +++++++++++++++++-------
 2 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index ade0f9f..2cb5fc8 100644
--- a/Makefile
+++ b/Makefile
@@ -67,7 +67,8 @@ SOPEL_SELECTORS := \
 # Tests marked with arbitrary_client_tags can't pass because Unreal whitelists which tags it relays
 # Tests marked with react_tag can't pass because Unreal blocks +draft/react https://github.com/unrealircd/unrealircd/pull/149
 # Tests marked with private_chathistory can't pass because Unreal does not implement CHATHISTORY for DMs
-# testChathistory fails: https://bugs.unrealircd.org/view.php?id=5952 and https://bugs.unrealircd.org/view.php?id=5953
+# testChathistory[BETWEEN] fails: https://bugs.unrealircd.org/view.php?id=5952
+# testChathistory[AROUND] fails: https://bugs.unrealircd.org/view.php?id=5953
 UNREALIRCD_SELECTORS := \
 	not Ergo \
 	and not deprecated \
@@ -80,7 +81,7 @@ UNREALIRCD_SELECTORS := \
 	and not arbitrary_client_tags \
 	and not react_tag \
 	and not private_chathistory \
-	and not testChathistory \
+	and not (testChathistory and (between or around)) \
 	$(EXTRA_SELECTORS)
 
 .PHONY: all flakes charybdis ergo inspircd mammon limnoria sopel solanum unrealircd
diff --git a/irctest/server_tests/test_chathistory.py b/irctest/server_tests/test_chathistory.py
index 9181de3..4ca7989 100644
--- a/irctest/server_tests/test_chathistory.py
+++ b/irctest/server_tests/test_chathistory.py
@@ -10,6 +10,8 @@ from irctest.patma import ANYSTR
 CHATHISTORY_CAP = "draft/chathistory"
 EVENT_PLAYBACK_CAP = "draft/event-playback"
 
+# Keep this in sync with validate_chathistory()
+SUBCOMMANDS = ["LATEST", "BEFORE", "AFTER", "BETWEEN", "AROUND"]
 
 MYSQL_PASSWORD = ""
 
@@ -158,7 +160,8 @@ class ChathistoryTestCase(cases.BaseServerTestCase):
         self.assertEqual(len(set(msg.msgid for msg in echo_messages)), num_messages)
         self.assertEqual(len(set(msg.time for msg in echo_messages)), num_messages)
 
-    def testChathistory(self):
+    @pytest.mark.parametrize("subcommand", SUBCOMMANDS)
+    def testChathistory(self, subcommand):
         self.connectClient(
             "bar",
             capabilities=[
@@ -187,9 +190,10 @@ class ChathistoryTestCase(cases.BaseServerTestCase):
             time.sleep(0.002)
 
         self.validate_echo_messages(NUM_MESSAGES, echo_messages)
-        self.validate_chathistory(echo_messages, 1, chname)
+        self.validate_chathistory(subcommand, echo_messages, 1, chname)
 
-    def testChathistoryEventPlayback(self):
+    @pytest.mark.parametrize("subcommand", SUBCOMMANDS)
+    def testChathistoryEventPlayback(self, subcommand):
         self.connectClient(
             "bar",
             capabilities=[
@@ -218,10 +222,11 @@ class ChathistoryTestCase(cases.BaseServerTestCase):
             time.sleep(0.002)
 
         self.validate_echo_messages(NUM_MESSAGES, echo_messages)
-        self.validate_chathistory(echo_messages, 1, chname)
+        self.validate_chathistory(subcommand, echo_messages, 1, chname)
 
+    @pytest.mark.parametrize("subcommand", SUBCOMMANDS)
     @pytest.mark.private_chathistory
-    def testChathistoryDMs(self):
+    def testChathistoryDMs(self, subcommand):
         c1 = "foo" + secrets.token_hex(12)
         c2 = "bar" + secrets.token_hex(12)
         self.controller.registerUser(self, c1, "sesame1")
@@ -272,8 +277,8 @@ class ChathistoryTestCase(cases.BaseServerTestCase):
             time.sleep(0.002)
 
         self.validate_echo_messages(NUM_MESSAGES, echo_messages)
-        self.validate_chathistory(echo_messages, 1, c2)
-        self.validate_chathistory(echo_messages, 2, c1)
+        self.validate_chathistory(subcommand, echo_messages, 1, c2)
+        self.validate_chathistory(subcommand, echo_messages, 2, c1)
 
         c3 = "baz" + secrets.token_hex(12)
         self.connectClient(
@@ -321,9 +326,9 @@ class ChathistoryTestCase(cases.BaseServerTestCase):
         self.assertEqual(results, new_convo)
 
         # additional messages with c3 should not show up in the c1-c2 history:
-        self.validate_chathistory(echo_messages, 1, c2)
-        self.validate_chathistory(echo_messages, 2, c1)
-        self.validate_chathistory(echo_messages, 2, c1.upper())
+        self.validate_chathistory(subcommand, echo_messages, 1, c2)
+        self.validate_chathistory(subcommand, echo_messages, 2, c1)
+        self.validate_chathistory(subcommand, echo_messages, 2, c1.upper())
 
         # regression test for #833
         self.sendLine(3, "QUIT")
@@ -356,9 +361,13 @@ class ChathistoryTestCase(cases.BaseServerTestCase):
         # should get nothing
         self.assertEqual(results, [])
 
-    def validate_chathistory(self, echo_messages, user, chname):
-        INCLUSIVE_LIMIT = len(echo_messages) * 2
+    def validate_chathistory(self, subcommand, echo_messages, user, chname):
+        # Keep this list of subcommands in sync with the SUBCOMMANDS global
+        method = getattr(self, f"_validate_chathistory_{subcommand}")
+        method(echo_messages, user, chname)
 
+    def _validate_chathistory_LATEST(self, echo_messages, user, chname):
+        INCLUSIVE_LIMIT = len(echo_messages) * 2
         self.sendLine(user, "CHATHISTORY LATEST %s * %d" % (chname, INCLUSIVE_LIMIT))
         result = validate_chathistory_batch(self.getMessages(user))
         self.assertEqual(echo_messages, result)
@@ -387,6 +396,8 @@ class ChathistoryTestCase(cases.BaseServerTestCase):
         result = validate_chathistory_batch(self.getMessages(user))
         self.assertEqual(echo_messages[5:], result)
 
+    def _validate_chathistory_BEFORE(self, echo_messages, user, chname):
+        INCLUSIVE_LIMIT = len(echo_messages) * 2
         self.sendLine(
             user,
             "CHATHISTORY BEFORE %s msgid=%s %d"
@@ -411,6 +422,8 @@ class ChathistoryTestCase(cases.BaseServerTestCase):
         result = validate_chathistory_batch(self.getMessages(user))
         self.assertEqual(echo_messages[4:6], result)
 
+    def _validate_chathistory_AFTER(self, echo_messages, user, chname):
+        INCLUSIVE_LIMIT = len(echo_messages) * 2
         self.sendLine(
             user,
             "CHATHISTORY AFTER %s msgid=%s %d"
@@ -434,6 +447,8 @@ class ChathistoryTestCase(cases.BaseServerTestCase):
         result = validate_chathistory_batch(self.getMessages(user))
         self.assertEqual(echo_messages[4:7], result)
 
+    def _validate_chathistory_BETWEEN(self, echo_messages, user, chname):
+        INCLUSIVE_LIMIT = len(echo_messages) * 2
         # BETWEEN forwards and backwards
         self.sendLine(
             user,
@@ -509,7 +524,7 @@ class ChathistoryTestCase(cases.BaseServerTestCase):
         result = validate_chathistory_batch(self.getMessages(user))
         self.assertEqual(echo_messages[-4:-1], result)
 
-        # AROUND
+    def _validate_chathistory_AROUND(self, echo_messages, user, chname):
         self.sendLine(
             user,
             "CHATHISTORY AROUND %s msgid=%s %d" % (chname, echo_messages[7].msgid, 1),
@@ -682,3 +697,10 @@ class ChathistoryTestCase(cases.BaseServerTestCase):
         validate_msg(echo)
         relay = self.getMessage(2)
         validate_msg(relay)
+
+
+assert {f"_validate_chathistory_{cmd}" for cmd in SUBCOMMANDS} == {
+    meth_name
+    for meth_name in dir(ChathistoryTestCase)
+    if meth_name.startswith("_validate_chathistory_")
+}, "ChathistoryTestCase.validate_chathistory and SUBCOMMANDS are out of sync"