From 495ce9c4f0421f40c0f0748289e09e681d5ab26b Mon Sep 17 00:00:00 2001 From: dgw Date: Mon, 4 Sep 2023 00:32:37 -0600 Subject: [PATCH 1/2] test: add capabilities test cases for trailing space handling Some IRCds include trailing space in capability lists, and this is not a spec violation. Default `.split()` behavior in Python should handle it fine, but it's also good to be explicit that we expect the default behavior, through tests. --- test/irc/test_irc_capabilities.py | 68 +++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/test/irc/test_irc_capabilities.py b/test/irc/test_irc_capabilities.py index 2a4e6d042f..d4b64fb84a 100644 --- a/test/irc/test_irc_capabilities.py +++ b/test/irc/test_irc_capabilities.py @@ -83,6 +83,20 @@ def test_capabilities_ls_multiline(mockbot, triggerfactory): assert manager.available == {'away-notify': None, 'account-tag': None} +def test_capabilities_ls_trailing_space(mockbot, triggerfactory: TriggerFactory): + raw = 'CAP * LS :away-notify ' + wrapped = triggerfactory.wrapper(mockbot, raw) + manager = Capabilities() + assert manager.handle_ls(wrapped, wrapped._trigger) + assert manager.is_available('away-notify') + assert not manager.is_enabled('away-notify') + assert manager.available == {'away-notify': None} + assert not manager.enabled + + expected = CapabilityInfo('away-notify', None, True, False) + assert manager.get_capability_info('away-notify') == expected + + def test_capabilities_ack(mockbot, triggerfactory): raw = 'CAP * ACK :away-notify' wrapped = triggerfactory.wrapper(mockbot, raw) @@ -104,6 +118,18 @@ def test_capabilities_ack(mockbot, triggerfactory): assert manager.enabled == frozenset({'away-notify', 'account-tag'}) +def test_capabilities_ack_trailing_space(mockbot, triggerfactory): + raw = 'CAP * ACK :away-notify ' + wrapped = triggerfactory.wrapper(mockbot, raw) + manager = Capabilities() + assert manager.handle_ack(wrapped, wrapped._trigger) == ('away-notify',) + assert not manager.is_available('away-notify'), ( + 'ACK a capability does not update server availability.') + assert manager.is_enabled('away-notify'), ( + 'ACK a capability make it enabled.') + assert manager.enabled == frozenset({'away-notify'}) + + def test_capabilities_ack_multiple(mockbot, triggerfactory): raw = 'CAP * ACK :away-notify account-tag' wrapped = triggerfactory.wrapper(mockbot, raw) @@ -183,6 +209,18 @@ def test_capabilities_nak(mockbot, triggerfactory): assert not manager.enabled +def test_capabilities_nak_trailing_space(mockbot, triggerfactory): + raw = 'CAP * NAK :away-notify ' + wrapped = triggerfactory.wrapper(mockbot, raw) + manager = Capabilities() + assert manager.handle_nak(wrapped, wrapped._trigger) == ('away-notify',) + assert not manager.is_available('away-notify'), ( + 'NAK a capability does not update server availability.') + assert not manager.is_enabled('away-notify'), ( + 'NAK a capability make it not enabled.') + assert not manager.enabled + + def test_capabilities_nack_multiple(mockbot, triggerfactory): raw = 'CAP * NAK :away-notify account-tag' wrapped = triggerfactory.wrapper(mockbot, raw) @@ -274,6 +312,21 @@ def test_capabilities_new(mockbot, triggerfactory): assert not manager.enabled +def test_capabilities_new_trailing_space(mockbot, triggerfactory): + manager = Capabilities() + + # NEW CAP + raw = 'CAP * NEW :echo-message ' + wrapped = triggerfactory.wrapper(mockbot, raw) + assert manager.handle_new(wrapped, wrapped._trigger) == ( + 'echo-message', + ) + assert manager.is_available('echo-message') + assert not manager.is_enabled('echo-message') + assert manager.available == {'echo-message': None} + assert not manager.enabled + + def test_capabilities_new_multiple(mockbot, triggerfactory): manager = Capabilities() @@ -320,6 +373,21 @@ def test_capabilities_del(mockbot, triggerfactory): assert not manager.enabled +def test_capabilities_del_trailing_space(mockbot, triggerfactory): + manager = Capabilities() + + # DEL CAP + raw = 'CAP * DEL :echo-message ' + wrapped = triggerfactory.wrapper(mockbot, raw) + assert manager.handle_del(wrapped, wrapped._trigger) == ( + 'echo-message', + ) + assert not manager.is_available('echo-message') + assert not manager.is_enabled('echo-message') + assert not manager.available + assert not manager.enabled + + def test_capabilities_del_available(mockbot, triggerfactory): manager = Capabilities() From 0c868423337efbcb120251b79b71dd28d42ef185 Mon Sep 17 00:00:00 2001 From: dgw Date: Mon, 4 Sep 2023 00:35:19 -0600 Subject: [PATCH 2/2] test: fix incorrect capability NAK test names It's not much, but it's honest work. I was going to do some type-hint cleanup too, but there are plenty of warnings in the `test/` directory because we haven't been checking it. That sort of work should be done in its own PR(s) rather than as incidental cleanup "because I'm already touching this file". --- test/irc/test_irc_capabilities.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/irc/test_irc_capabilities.py b/test/irc/test_irc_capabilities.py index d4b64fb84a..b24c9aa1f5 100644 --- a/test/irc/test_irc_capabilities.py +++ b/test/irc/test_irc_capabilities.py @@ -221,7 +221,7 @@ def test_capabilities_nak_trailing_space(mockbot, triggerfactory): assert not manager.enabled -def test_capabilities_nack_multiple(mockbot, triggerfactory): +def test_capabilities_nak_multiple(mockbot, triggerfactory): raw = 'CAP * NAK :away-notify account-tag' wrapped = triggerfactory.wrapper(mockbot, raw) manager = Capabilities() @@ -246,7 +246,7 @@ def test_capabilities_nack_multiple(mockbot, triggerfactory): assert not manager.enabled -def test_capabilities_ack_and_nack(mockbot, triggerfactory): +def test_capabilities_ack_and_nak(mockbot, triggerfactory): manager = Capabilities() # ACK a single CAP