kick: Exhaustive implementation of the Modern spec + honor TARGMAX in testDoubleKickMessages (#100)

Makefile

@@ -22,12 +22,13 @@ BAHAMUT_SELECTORS := \
 
 # testQuitErrors is very flaky
 # AccountTagTestCase.testInvite fails because https://github.com/solanum-ircd/solanum/issues/166
+# testKickDefaultComment fails because it uses the nick of the kickee rather than the kicker.
 CHARYBDIS_SELECTORS := \
 	not Ergo \
 	and not deprecated \
 	and not strict \
-	and not testDoubleKickMessages \
 	and not testQuitErrors \
+	and not testKickDefaultComment \
 	and not (AccountTagTestCase and testInvite) \
 	$(EXTRA_SELECTORS)
 
@@ -59,6 +60,7 @@ INSPIRCD_SELECTORS := \
 # lusers tests fail because they depend on Modern behavior, not just RFC2812 (TODO: update lusers tests to accept RFC2812-compliant implementations)
 # statusmsg tests fail because STATUSMSG is present in ISUPPORT, but it not actually supported as PRIVMSG target
 # testKeyValidation[empty] fails because ircu2 returns ERR_NEEDMOREPARAMS on empty keys: https://github.com/UndernetIRC/ircu2/issues/13
+# testKickDefaultComment fails because it uses the nick of the kickee rather than the kicker.
 IRCU2_SELECTORS := \
 	not Ergo \
 	and not deprecated \
@@ -68,6 +70,7 @@ IRCU2_SELECTORS := \
 	and not lusers \
 	and not statusmsg \
 	and not (testKeyValidation and empty) \
+	and not testKickDefaultComment \
 	$(EXTRA_SELECTORS)
 
 # same justification as ircu2
@@ -82,11 +85,13 @@ SNIRCD_SELECTORS := \
 	$(EXTRA_SELECTORS)
 
 # testListEmpty and testListOne fails because irc2 deprecated LIST
+# testKickDefaultComment fails because it uses the nick of the kickee rather than the kicker.
 IRC2_SELECTORS := \
 	not Ergo \
 	and not deprecated \
 	and not strict \
 	and not testListEmpty and not testListOne \
+	and not testKickDefaultComment \
 	$(EXTRA_SELECTORS)
 
 MAMMON_SELECTORS := \
@@ -112,12 +117,13 @@ LIMNORIA_SELECTORS := \
 	$(EXTRA_SELECTORS)
 
 # testQuitErrors is too flaky for CI
+# testKickDefaultComment fails because solanum uses the nick of the kickee rather than the kicker.
 SOLANUM_SELECTORS := \
 	not Ergo \
 	and not deprecated \
 	and not strict \
-	and not testDoubleKickMessages \
 	and not testQuitErrors \
+	and not testKickDefaultComment \
 	$(EXTRA_SELECTORS)
 
 SOPEL_SELECTORS := \

irctest/server_tests/kick.py

@@ -11,13 +11,17 @@ from irctest.patma import ANYSTR
 
 
 class KickTestCase(cases.BaseServerTestCase):
-    @cases.mark_specifications("RFC1459", "RFC2812")
+    @cases.mark_specifications("RFC1459", "RFC2812", "Modern")
     def testKickSendsMessages(self):
         """“Once a user has joined a channel, he receives information about
         all commands his server receives affecting the channel.  This
         includes […] KICK”
         -- <https://tools.ietf.org/html/rfc1459#section-4.2.1>
         and <https://tools.ietf.org/html/rfc2812#section-3.2.1>
+
+        " If a comment is given, this will be sent instead of the default message,
+        the nickname of the user targeted by the KICK."
+        -- https://github.com/ircdocs/modern-irc/pull/101
         """
         self.connectClient("foo")
         self.joinChannel(1, "#chan")
@@ -49,6 +53,59 @@ class KickTestCase(cases.BaseServerTestCase):
         m = self.getMessage(3)
         self.assertMessageMatch(m, command="KICK", params=["#chan", "bar", "bye"])
 
+    def _testKickNoComment(self, check_default):
+        self.connectClient("foo")
+        self.joinChannel(1, "#chan")
+
+        self.connectClient("bar")
+        self.joinChannel(2, "#chan")
+
+        self.connectClient("baz")
+        self.joinChannel(3, "#chan")
+
+        # TODO: check foo is an operator
+
+        self.getMessages(1)
+        self.getMessages(2)
+        self.getMessages(3)
+        self.sendLine(1, "KICK #chan bar")
+        try:
+            m = self.getMessage(1)
+            if m.command == "482":
+                raise runner.ImplementationChoice(
+                    "Channel creators are not opped by default."
+                )
+            self.assertMessageMatch(m, command="KICK")
+        except client_mock.NoMessageException:
+            # The RFCs do not say KICK must be echoed
+            pass
+        m2 = self.getMessage(2)
+        m3 = self.getMessage(3)
+        if check_default:
+            self.assertMessageMatch(m2, command="KICK", params=["#chan", "bar", "foo"])
+            self.assertMessageMatch(m3, command="KICK", params=["#chan", "bar", "foo"])
+        else:
+            self.assertMessageMatch(m2, command="KICK", params=["#chan", "bar", ANYSTR])
+            self.assertMessageMatch(m3, command="KICK", params=["#chan", "bar", ANYSTR])
+
+    @cases.mark_specifications("RFC2812")
+    def testKickDefaultComment(self):
+        """
+        "If a "comment" is
+        given, this will be sent instead of the default message, the nickname
+        of the user issuing the KICK."
+        -- https://datatracker.ietf.org/doc/html/rfc2812#section-3.2.8
+        """
+        self._testKickNoComment(check_default=True)
+
+    @cases.mark_specifications("Modern")
+    def testKickNoComment(self):
+        """
+        "If no comment is given, the server SHOULD use a default message instead."
+        -- https://github.com/ircdocs/modern-irc/pull/101
+        """
+        self._testKickNoComment(check_default=False)
+
     @cases.mark_specifications("RFC2812")
     def testKickPrivileges(self):
         """Test who has the ability to kick / what error codes are sent
@@ -116,12 +173,37 @@ class KickTestCase(cases.BaseServerTestCase):
         self.assertMessageMatch(m, command="403")
 
     @pytest.mark.parametrize("multiple_targets", [True, False])
-    @cases.mark_specifications("RFC2812")
+    @cases.mark_specifications("RFC2812", "Modern", "ircdocs")
     def testDoubleKickMessages(self, multiple_targets):
         """“The server MUST NOT send KICK messages with multiple channels or
         users to clients.  This is necessarily to maintain backward
         compatibility with old client software.”
         -- https://tools.ietf.org/html/rfc2812#section-3.2.8
+
+        "The server MUST NOT send KICK messages with multiple channels or
+        users to clients.
+        This is necessary to maintain backward compatibility with existing
+        client software."
+        -- https://github.com/ircdocs/modern-irc/pull/101
+
+        "Servers MAY limit the number of target users per `KICK` command
+        via the [`TARGMAX` parameter of `RPL_ISUPPORT`](#targmax-parameter),
+        and silently drop targets if the number of targets exceeds the limit."
+        -- https://github.com/ircdocs/modern-irc/pull/101
+
+        "If the "TARGMAX" parameter is not advertised or a value is not sent
+        then a client SHOULD assume that no commands except the "JOIN" and "PART"
+        commands accept multiple parameters."
+        -- https://defs.ircdocs.horse/defs/isupport.html#targmax
+
+        "If this parameter is not advertised or a value is not sent then a client
+        SHOULD assume that no commands except the `JOIN` and `PART` commands
+        accept multiple parameters."
+        -- https://github.com/ircdocs/modern-irc/pull/113
+
+        "If <limit> is not specified, then there is no maximum number of targets
+        for that command."
+        -- https://modern.ircdocs.horse/#targmax-parameter
         """
         self.connectClient("foo")
         self.joinChannel(1, "#chan")
@@ -135,6 +217,14 @@ class KickTestCase(cases.BaseServerTestCase):
         self.connectClient("qux")
         self.joinChannel(4, "#chan")
 
+        targmax = dict(
+            item.split(":", 1)
+            for item in self.server_support.get("TARGMAX", "").split(",")
+            if item
+        )
+        if targmax.get("KICK", "1") == "1":
+            raise runner.NotImplementedByController("Multi-target KICK")
+
         # TODO: check foo is an operator
 
         # Synchronize
@@ -153,14 +243,12 @@ class KickTestCase(cases.BaseServerTestCase):
                 raise runner.OptionalExtensionNotSupported(
                     "Channel creators are not opped by default."
                 )
-            if m.command in {"401", "403"}:
-                raise runner.NotImplementedByController("Multi-target KICK")
         except client_mock.NoMessageException:
             # The RFCs do not say KICK must be echoed
             pass
 
         mgroup = self.getMessages(4)
-        self.assertGreaterEqual(len(mgroup), 2)
+        self.assertGreaterEqual(len(mgroup), 2, mgroup)
         m1, m2 = mgroup[:2]
 
         self.assertMessageMatch(m1, command="KICK", params=["#chan", ANYSTR, "bye"])