From 2f106aa02adc157ef730ce49012da7e7f3b05b54 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 28 Mar 2023 08:35:31 -0400 Subject: [PATCH 01/11] use foolscap.reconnector.ReconnectionInfo where one is required It's *right* there. Just use it! --- src/allmydata/test/test_connection_status.py | 22 +++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/allmydata/test/test_connection_status.py b/src/allmydata/test/test_connection_status.py index 2bd8bf6ab..f6b36d5ba 100644 --- a/src/allmydata/test/test_connection_status.py +++ b/src/allmydata/test/test_connection_status.py @@ -1,21 +1,13 @@ """ Tests for allmydata.util.connection_status. - -Port to Python 3. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals - -from future.utils import PY2 -if PY2: - from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 import mock from twisted.trial import unittest +from foolscap.reconnector import ReconnectionInfo + from ..util import connection_status class Status(unittest.TestCase): @@ -33,7 +25,7 @@ class Status(unittest.TestCase): ci.connectionHandlers = {"h1": "hand1"} ci.winningHint = "h1" ci.establishedAt = 120 - ri = mock.Mock() + ri = ReconnectionInfo() ri.state = "connected" ri.connectionInfo = ci rc = mock.Mock @@ -51,7 +43,7 @@ class Status(unittest.TestCase): ci.connectionHandlers = {"h1": "hand1"} ci.winningHint = "h1" ci.establishedAt = 120 - ri = mock.Mock() + ri = ReconnectionInfo() ri.state = "connected" ri.connectionInfo = ci rc = mock.Mock @@ -70,7 +62,7 @@ class Status(unittest.TestCase): ci.listenerStatus = ("listener1", "successful") ci.winningHint = None ci.establishedAt = 120 - ri = mock.Mock() + ri = ReconnectionInfo() ri.state = "connected" ri.connectionInfo = ci rc = mock.Mock @@ -87,7 +79,7 @@ class Status(unittest.TestCase): ci = mock.Mock() ci.connectorStatuses = {"h1": "st1", "h2": "st2"} ci.connectionHandlers = {"h1": "hand1"} - ri = mock.Mock() + ri = ReconnectionInfo() ri.state = "connecting" ri.connectionInfo = ci rc = mock.Mock @@ -104,7 +96,7 @@ class Status(unittest.TestCase): ci = mock.Mock() ci.connectorStatuses = {"h1": "st1", "h2": "st2"} ci.connectionHandlers = {"h1": "hand1"} - ri = mock.Mock() + ri = ReconnectionInfo() ri.state = "waiting" ri.lastAttempt = 10 ri.nextAttempt = 20 From e2c6cc49d5e97627f4979ff4329c8cf3360f010e Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 28 Mar 2023 08:37:22 -0400 Subject: [PATCH 02/11] use foolscap.info.ConnectionInfo where one is required It's *right* there. Just use it! --- src/allmydata/test/test_connection_status.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/allmydata/test/test_connection_status.py b/src/allmydata/test/test_connection_status.py index f6b36d5ba..ba57f8aee 100644 --- a/src/allmydata/test/test_connection_status.py +++ b/src/allmydata/test/test_connection_status.py @@ -7,6 +7,7 @@ import mock from twisted.trial import unittest from foolscap.reconnector import ReconnectionInfo +from foolscap.info import ConnectionInfo from ..util import connection_status @@ -20,7 +21,7 @@ class Status(unittest.TestCase): "h2": "st2"}) def test_reconnector_connected(self): - ci = mock.Mock() + ci = ConnectionInfo() ci.connectorStatuses = {"h1": "st1"} ci.connectionHandlers = {"h1": "hand1"} ci.winningHint = "h1" @@ -38,7 +39,7 @@ class Status(unittest.TestCase): self.assertEqual(cs.last_received_time, 123) def test_reconnector_connected_others(self): - ci = mock.Mock() + ci = ConnectionInfo() ci.connectorStatuses = {"h1": "st1", "h2": "st2"} ci.connectionHandlers = {"h1": "hand1"} ci.winningHint = "h1" @@ -56,7 +57,7 @@ class Status(unittest.TestCase): self.assertEqual(cs.last_received_time, 123) def test_reconnector_connected_listener(self): - ci = mock.Mock() + ci = ConnectionInfo() ci.connectorStatuses = {"h1": "st1", "h2": "st2"} ci.connectionHandlers = {"h1": "hand1"} ci.listenerStatus = ("listener1", "successful") @@ -76,7 +77,7 @@ class Status(unittest.TestCase): self.assertEqual(cs.last_received_time, 123) def test_reconnector_connecting(self): - ci = mock.Mock() + ci = ConnectionInfo() ci.connectorStatuses = {"h1": "st1", "h2": "st2"} ci.connectionHandlers = {"h1": "hand1"} ri = ReconnectionInfo() @@ -93,7 +94,7 @@ class Status(unittest.TestCase): self.assertEqual(cs.last_received_time, 123) def test_reconnector_waiting(self): - ci = mock.Mock() + ci = ConnectionInfo() ci.connectorStatuses = {"h1": "st1", "h2": "st2"} ci.connectionHandlers = {"h1": "hand1"} ri = ReconnectionInfo() From 6b7ea29d887150a8e041fbb0fe835b75ca76d565 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 28 Mar 2023 08:40:25 -0400 Subject: [PATCH 03/11] use foolscap.reconnector.Reconnector where one is required Unfortunately we need to touch a private attribute directly to shove our expected info into it. This isn't so bad though. Foolscap isn't moving much and we're not touching anything complex, just setting a simple model attribute. --- src/allmydata/test/test_connection_status.py | 23 ++++++++++---------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/allmydata/test/test_connection_status.py b/src/allmydata/test/test_connection_status.py index ba57f8aee..3456a61f0 100644 --- a/src/allmydata/test/test_connection_status.py +++ b/src/allmydata/test/test_connection_status.py @@ -6,11 +6,17 @@ import mock from twisted.trial import unittest -from foolscap.reconnector import ReconnectionInfo +from foolscap.reconnector import ReconnectionInfo, Reconnector from foolscap.info import ConnectionInfo from ..util import connection_status +def reconnector(info: ReconnectionInfo) -> Reconnector: + rc = Reconnector(None, None, (), {}) + rc._reconnectionInfo = info + return rc + + class Status(unittest.TestCase): def test_hint_statuses(self): ncs = connection_status._hint_statuses(["h2","h1"], @@ -29,8 +35,7 @@ class Status(unittest.TestCase): ri = ReconnectionInfo() ri.state = "connected" ri.connectionInfo = ci - rc = mock.Mock - rc.getReconnectionInfo = mock.Mock(return_value=ri) + rc = reconnector(ri) cs = connection_status.from_foolscap_reconnector(rc, 123) self.assertEqual(cs.connected, True) self.assertEqual(cs.summary, "Connected to h1 via hand1") @@ -47,8 +52,7 @@ class Status(unittest.TestCase): ri = ReconnectionInfo() ri.state = "connected" ri.connectionInfo = ci - rc = mock.Mock - rc.getReconnectionInfo = mock.Mock(return_value=ri) + rc = reconnector(ri) cs = connection_status.from_foolscap_reconnector(rc, 123) self.assertEqual(cs.connected, True) self.assertEqual(cs.summary, "Connected to h1 via hand1") @@ -66,8 +70,7 @@ class Status(unittest.TestCase): ri = ReconnectionInfo() ri.state = "connected" ri.connectionInfo = ci - rc = mock.Mock - rc.getReconnectionInfo = mock.Mock(return_value=ri) + rc = reconnector(ri) cs = connection_status.from_foolscap_reconnector(rc, 123) self.assertEqual(cs.connected, True) self.assertEqual(cs.summary, "Connected via listener (listener1)") @@ -83,8 +86,7 @@ class Status(unittest.TestCase): ri = ReconnectionInfo() ri.state = "connecting" ri.connectionInfo = ci - rc = mock.Mock - rc.getReconnectionInfo = mock.Mock(return_value=ri) + rc = reconnector(ri) cs = connection_status.from_foolscap_reconnector(rc, 123) self.assertEqual(cs.connected, False) self.assertEqual(cs.summary, "Trying to connect") @@ -102,8 +104,7 @@ class Status(unittest.TestCase): ri.lastAttempt = 10 ri.nextAttempt = 20 ri.connectionInfo = ci - rc = mock.Mock - rc.getReconnectionInfo = mock.Mock(return_value=ri) + rc = reconnector(ri) with mock.patch("time.time", return_value=12): cs = connection_status.from_foolscap_reconnector(rc, 5) self.assertEqual(cs.connected, False) From 32cd54501d0f92de067c39b4178814e438bb49a4 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 28 Mar 2023 08:52:31 -0400 Subject: [PATCH 04/11] Pass a time function instead of patching the global --- src/allmydata/test/test_connection_status.py | 3 +-- src/allmydata/util/connection_status.py | 5 +++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/allmydata/test/test_connection_status.py b/src/allmydata/test/test_connection_status.py index 3456a61f0..6e258294b 100644 --- a/src/allmydata/test/test_connection_status.py +++ b/src/allmydata/test/test_connection_status.py @@ -105,8 +105,7 @@ class Status(unittest.TestCase): ri.nextAttempt = 20 ri.connectionInfo = ci rc = reconnector(ri) - with mock.patch("time.time", return_value=12): - cs = connection_status.from_foolscap_reconnector(rc, 5) + cs = connection_status.from_foolscap_reconnector(rc, 5, time=lambda: 12) self.assertEqual(cs.connected, False) self.assertEqual(cs.summary, "Reconnecting in 8 seconds (last attempt 2s ago)") diff --git a/src/allmydata/util/connection_status.py b/src/allmydata/util/connection_status.py index 0e8595e81..e9c2c1388 100644 --- a/src/allmydata/util/connection_status.py +++ b/src/allmydata/util/connection_status.py @@ -16,6 +16,7 @@ if PY2: import time from zope.interface import implementer from ..interfaces import IConnectionStatus +from foolscap.reconnector import Reconnector @implementer(IConnectionStatus) class ConnectionStatus(object): @@ -50,7 +51,7 @@ def _hint_statuses(which, handlers, statuses): non_connected_statuses["%s%s" % (hint, handler_dsc)] = dsc return non_connected_statuses -def from_foolscap_reconnector(rc, last_received): +def from_foolscap_reconnector(rc: Reconnector, last_received: int, time=time.time) -> ConnectionStatus: ri = rc.getReconnectionInfo() # See foolscap/reconnector.py, ReconnectionInfo, for details about possible # states. The returned result is a native string, it seems, so convert to @@ -80,7 +81,7 @@ def from_foolscap_reconnector(rc, last_received): # ci describes the current in-progress attempt summary = "Trying to connect" elif state == "waiting": - now = time.time() + now = time() elapsed = now - ri.lastAttempt delay = ri.nextAttempt - now summary = "Reconnecting in %d seconds (last attempt %ds ago)" % \ From 9a8430c90fcbb879e3e8e1b32a44c5f7aea8a5c0 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 28 Mar 2023 08:52:44 -0400 Subject: [PATCH 05/11] Remove porting boilerplate --- src/allmydata/util/connection_status.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/allmydata/util/connection_status.py b/src/allmydata/util/connection_status.py index e9c2c1388..d7cf18e1b 100644 --- a/src/allmydata/util/connection_status.py +++ b/src/allmydata/util/connection_status.py @@ -1,18 +1,7 @@ """ Parse connection status from Foolscap. - -Ported to Python 3. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals - -from future.utils import PY2 -if PY2: - from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 - import time from zope.interface import implementer from ..interfaces import IConnectionStatus From 8e63fe2fddc8ca7b20a8a819d56c839546bc30e3 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 28 Mar 2023 08:52:55 -0400 Subject: [PATCH 06/11] Remove the unused mock import --- src/allmydata/test/test_connection_status.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/allmydata/test/test_connection_status.py b/src/allmydata/test/test_connection_status.py index 6e258294b..6c2e170f3 100644 --- a/src/allmydata/test/test_connection_status.py +++ b/src/allmydata/test/test_connection_status.py @@ -2,8 +2,6 @@ Tests for allmydata.util.connection_status. """ -import mock - from twisted.trial import unittest from foolscap.reconnector import ReconnectionInfo, Reconnector From 6d4278b465a4eb2194845884d363d373e84433f3 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 28 Mar 2023 08:53:21 -0400 Subject: [PATCH 07/11] Factor some repetition out of the tests --- src/allmydata/test/test_connection_status.py | 86 ++++++++++---------- src/allmydata/util/connection_status.py | 2 +- 2 files changed, 42 insertions(+), 46 deletions(-) diff --git a/src/allmydata/test/test_connection_status.py b/src/allmydata/test/test_connection_status.py index 6c2e170f3..f415a6ebf 100644 --- a/src/allmydata/test/test_connection_status.py +++ b/src/allmydata/test/test_connection_status.py @@ -2,21 +2,43 @@ Tests for allmydata.util.connection_status. """ -from twisted.trial import unittest +from typing import Optional from foolscap.reconnector import ReconnectionInfo, Reconnector from foolscap.info import ConnectionInfo from ..util import connection_status +from .common import SyncTestCase def reconnector(info: ReconnectionInfo) -> Reconnector: - rc = Reconnector(None, None, (), {}) + rc = Reconnector(None, None, (), {}) # type: ignore[no-untyped-call] rc._reconnectionInfo = info return rc +def connection_info( + statuses: dict[str, str], + handlers: dict[str, str], + winningHint: Optional[str], + establishedAt: Optional[int], +) -> ConnectionInfo: + ci = ConnectionInfo() # type: ignore[no-untyped-call] + ci.connectorStatuses = statuses + ci.connectionHandlers = handlers + ci.winningHint = winningHint + ci.establishedAt = establishedAt + return ci -class Status(unittest.TestCase): - def test_hint_statuses(self): +def reconnection_info( + state: str, + connection_info: ConnectionInfo, +) -> ReconnectionInfo: + ri = ReconnectionInfo() # type: ignore[no-untyped-call] + ri.state = state + ri.connectionInfo = connection_info + return ri + +class Status(SyncTestCase): + def test_hint_statuses(self) -> None: ncs = connection_status._hint_statuses(["h2","h1"], {"h1": "hand1", "h4": "hand4"}, {"h1": "st1", "h2": "st2", @@ -24,15 +46,9 @@ class Status(unittest.TestCase): self.assertEqual(ncs, {"h1 via hand1": "st1", "h2": "st2"}) - def test_reconnector_connected(self): - ci = ConnectionInfo() - ci.connectorStatuses = {"h1": "st1"} - ci.connectionHandlers = {"h1": "hand1"} - ci.winningHint = "h1" - ci.establishedAt = 120 - ri = ReconnectionInfo() - ri.state = "connected" - ri.connectionInfo = ci + def test_reconnector_connected(self) -> None: + ci = connection_info({"h1": "st1"}, {"h1": "hand1"}, "h1", 120) + ri = reconnection_info("connected", ci) rc = reconnector(ri) cs = connection_status.from_foolscap_reconnector(rc, 123) self.assertEqual(cs.connected, True) @@ -41,15 +57,9 @@ class Status(unittest.TestCase): self.assertEqual(cs.last_connection_time, 120) self.assertEqual(cs.last_received_time, 123) - def test_reconnector_connected_others(self): - ci = ConnectionInfo() - ci.connectorStatuses = {"h1": "st1", "h2": "st2"} - ci.connectionHandlers = {"h1": "hand1"} - ci.winningHint = "h1" - ci.establishedAt = 120 - ri = ReconnectionInfo() - ri.state = "connected" - ri.connectionInfo = ci + def test_reconnector_connected_others(self) -> None: + ci = connection_info({"h1": "st1", "h2": "st2"}, {"h1": "hand1"}, "h1", 120) + ri = reconnection_info("connected", ci) rc = reconnector(ri) cs = connection_status.from_foolscap_reconnector(rc, 123) self.assertEqual(cs.connected, True) @@ -58,16 +68,10 @@ class Status(unittest.TestCase): self.assertEqual(cs.last_connection_time, 120) self.assertEqual(cs.last_received_time, 123) - def test_reconnector_connected_listener(self): - ci = ConnectionInfo() - ci.connectorStatuses = {"h1": "st1", "h2": "st2"} - ci.connectionHandlers = {"h1": "hand1"} + def test_reconnector_connected_listener(self) -> None: + ci = connection_info({"h1": "st1", "h2": "st2"}, {"h1": "hand1"}, None, 120) ci.listenerStatus = ("listener1", "successful") - ci.winningHint = None - ci.establishedAt = 120 - ri = ReconnectionInfo() - ri.state = "connected" - ri.connectionInfo = ci + ri = reconnection_info("connected", ci) rc = reconnector(ri) cs = connection_status.from_foolscap_reconnector(rc, 123) self.assertEqual(cs.connected, True) @@ -77,13 +81,9 @@ class Status(unittest.TestCase): self.assertEqual(cs.last_connection_time, 120) self.assertEqual(cs.last_received_time, 123) - def test_reconnector_connecting(self): - ci = ConnectionInfo() - ci.connectorStatuses = {"h1": "st1", "h2": "st2"} - ci.connectionHandlers = {"h1": "hand1"} - ri = ReconnectionInfo() - ri.state = "connecting" - ri.connectionInfo = ci + def test_reconnector_connecting(self) -> None: + ci = connection_info({"h1": "st1", "h2": "st2"}, {"h1": "hand1"}, None, None) + ri = reconnection_info("connecting", ci) rc = reconnector(ri) cs = connection_status.from_foolscap_reconnector(rc, 123) self.assertEqual(cs.connected, False) @@ -93,15 +93,11 @@ class Status(unittest.TestCase): self.assertEqual(cs.last_connection_time, None) self.assertEqual(cs.last_received_time, 123) - def test_reconnector_waiting(self): - ci = ConnectionInfo() - ci.connectorStatuses = {"h1": "st1", "h2": "st2"} - ci.connectionHandlers = {"h1": "hand1"} - ri = ReconnectionInfo() - ri.state = "waiting" + def test_reconnector_waiting(self) -> None: + ci = connection_info({"h1": "st1", "h2": "st2"}, {"h1": "hand1"}, None, None) + ri = reconnection_info("waiting", ci) ri.lastAttempt = 10 ri.nextAttempt = 20 - ri.connectionInfo = ci rc = reconnector(ri) cs = connection_status.from_foolscap_reconnector(rc, 5, time=lambda: 12) self.assertEqual(cs.connected, False) diff --git a/src/allmydata/util/connection_status.py b/src/allmydata/util/connection_status.py index d7cf18e1b..751eee4fe 100644 --- a/src/allmydata/util/connection_status.py +++ b/src/allmydata/util/connection_status.py @@ -31,7 +31,7 @@ class ConnectionStatus(object): last_received_time=None, ) -def _hint_statuses(which, handlers, statuses): +def _hint_statuses(which, handlers, statuses) -> dict[str, str]: non_connected_statuses = {} for hint in which: handler = handlers.get(hint) From 2e6a40294b7c30bb9e000eaa52e9bc00097504a3 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 28 Mar 2023 08:53:37 -0400 Subject: [PATCH 08/11] Crank the type checking ratchet --- mypy.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy.ini b/mypy.ini index 7acc0ddc5..482fd6dd8 100644 --- a/mypy.ini +++ b/mypy.ini @@ -9,7 +9,7 @@ no_implicit_optional = True warn_redundant_casts = True strict_equality = True -[mypy-allmydata.test.cli.wormholetesting] +[mypy-allmydata.test.cli.wormholetesting,allmydata.test.test_connection_status] disallow_any_generics = True disallow_subclassing_any = True disallow_untyped_calls = True From a839ace32ae144457bf2f0c1be6e97903f84e7d3 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 28 Mar 2023 08:53:54 -0400 Subject: [PATCH 09/11] news fragment --- newsfragments/4003.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/4003.minor diff --git a/newsfragments/4003.minor b/newsfragments/4003.minor new file mode 100644 index 000000000..e69de29bb From 3ea9e97606b8605befc8e2b2f1a6342cf47c0336 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 28 Mar 2023 09:01:03 -0400 Subject: [PATCH 10/11] Python 3.8 compatibility --- src/allmydata/test/test_connection_status.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/allmydata/test/test_connection_status.py b/src/allmydata/test/test_connection_status.py index f415a6ebf..da41f5a47 100644 --- a/src/allmydata/test/test_connection_status.py +++ b/src/allmydata/test/test_connection_status.py @@ -2,6 +2,8 @@ Tests for allmydata.util.connection_status. """ +from __future__ import annotations + from typing import Optional from foolscap.reconnector import ReconnectionInfo, Reconnector From ecfa76ac3268d8f23fe374b8f1ae7507e01f0773 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 28 Mar 2023 13:22:08 -0400 Subject: [PATCH 11/11] Python 3.8 compatibility --- src/allmydata/util/connection_status.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/allmydata/util/connection_status.py b/src/allmydata/util/connection_status.py index 751eee4fe..0ccdcd672 100644 --- a/src/allmydata/util/connection_status.py +++ b/src/allmydata/util/connection_status.py @@ -2,6 +2,8 @@ Parse connection status from Foolscap. """ +from __future__ import annotations + import time from zope.interface import implementer from ..interfaces import IConnectionStatus