From 751ccae769d6a951e2f7b8511f0068ab7b8a5911 Mon Sep 17 00:00:00 2001 From: Shane Harvey Date: Fri, 31 Jan 2025 10:08:28 -0800 Subject: [PATCH 1/2] PYTHON-5120 Reduce configureFailPoint duplication in tests --- test/__init__.py | 17 +++++++++++------ test/asynchronous/__init__.py | 17 +++++++++++------ test/asynchronous/test_transactions.py | 7 +------ test/asynchronous/unified_format.py | 8 ++------ test/asynchronous/utils_spec_runner.py | 9 ++------- test/test_connection_monitoring.py | 7 +------ test/test_transactions.py | 7 +------ test/unified_format.py | 8 ++------ test/utils_spec_runner.py | 9 ++------- 9 files changed, 33 insertions(+), 56 deletions(-) diff --git a/test/__init__.py b/test/__init__.py index b49eee99ac..f5036858a1 100644 --- a/test/__init__.py +++ b/test/__init__.py @@ -933,17 +933,22 @@ def assertEqualCommand(self, expected, actual, msg=None): def assertEqualReply(self, expected, actual, msg=None): self.assertEqual(sanitize_reply(expected), sanitize_reply(actual), msg) + @staticmethod + def configure_fail_point(client, command_args, off=False): + cmd = {"configureFailPoint": "failCommand"} + cmd.update(command_args) + if off: + cmd["mode"] = "off" + cmd.pop("data", None) + client.admin.command(cmd) + @contextmanager def fail_point(self, command_args): - cmd_on = SON([("configureFailPoint", "failCommand")]) - cmd_on.update(command_args) - client_context.client.admin.command(cmd_on) + self.configure_fail_point(client_context.client, command_args) try: yield finally: - client_context.client.admin.command( - "configureFailPoint", cmd_on["configureFailPoint"], mode="off" - ) + self.configure_fail_point(client_context.client, command_args, off=True) @contextmanager def fork( diff --git a/test/asynchronous/__init__.py b/test/asynchronous/__init__.py index a6ba29baaa..eaefff1b6f 100644 --- a/test/asynchronous/__init__.py +++ b/test/asynchronous/__init__.py @@ -935,17 +935,22 @@ def assertEqualCommand(self, expected, actual, msg=None): def assertEqualReply(self, expected, actual, msg=None): self.assertEqual(sanitize_reply(expected), sanitize_reply(actual), msg) + @staticmethod + async def configure_fail_point(client, command_args, off=False): + cmd = {"configureFailPoint": "failCommand"} + cmd.update(command_args) + if off: + cmd["mode"] = "off" + cmd.pop("data", None) + await client.admin.command(cmd) + @asynccontextmanager async def fail_point(self, command_args): - cmd_on = SON([("configureFailPoint", "failCommand")]) - cmd_on.update(command_args) - await async_client_context.client.admin.command(cmd_on) + await self.configure_fail_point(async_client_context.client, command_args) try: yield finally: - await async_client_context.client.admin.command( - "configureFailPoint", cmd_on["configureFailPoint"], mode="off" - ) + await self.configure_fail_point(async_client_context.client, command_args, off=True) @contextmanager def fork( diff --git a/test/asynchronous/test_transactions.py b/test/asynchronous/test_transactions.py index d11d0a9776..5f75746a4d 100644 --- a/test/asynchronous/test_transactions.py +++ b/test/asynchronous/test_transactions.py @@ -410,15 +410,10 @@ async def asyncSetUp(self) -> None: for address in async_client_context.mongoses: self.mongos_clients.append(await self.async_single_client("{}:{}".format(*address))) - async def _set_fail_point(self, client, command_args): - cmd = {"configureFailPoint": "failCommand"} - cmd.update(command_args) - await client.admin.command(cmd) - async def set_fail_point(self, command_args): clients = self.mongos_clients if self.mongos_clients else [self.client] for client in clients: - await self._set_fail_point(client, command_args) + await self.configure_fail_point(client, command_args) @async_client_context.require_transactions async def test_callback_raises_custom_error(self): diff --git a/test/asynchronous/unified_format.py b/test/asynchronous/unified_format.py index 52d964eb3e..fdf82c34b1 100644 --- a/test/asynchronous/unified_format.py +++ b/test/asynchronous/unified_format.py @@ -1008,12 +1008,8 @@ async def __set_fail_point(self, client, command_args): if not async_client_context.test_commands_enabled: self.skipTest("Test commands must be enabled") - cmd_on = SON([("configureFailPoint", "failCommand")]) - cmd_on.update(command_args) - await client.admin.command(cmd_on) - self.addAsyncCleanup( - client.admin.command, "configureFailPoint", cmd_on["configureFailPoint"], mode="off" - ) + await self.configure_fail_point(client, command_args) + self.addAsyncCleanup(self.configure_fail_point, client, command_args, off=True) async def _testOperation_failPoint(self, spec): await self.__set_fail_point( diff --git a/test/asynchronous/utils_spec_runner.py b/test/asynchronous/utils_spec_runner.py index b79e5258b5..c1461d2bbf 100644 --- a/test/asynchronous/utils_spec_runner.py +++ b/test/asynchronous/utils_spec_runner.py @@ -265,15 +265,10 @@ async def asyncSetUp(self) -> None: async def asyncTearDown(self) -> None: self.knobs.disable() - async def _set_fail_point(self, client, command_args): - cmd = SON([("configureFailPoint", "failCommand")]) - cmd.update(command_args) - await client.admin.command(cmd) - async def set_fail_point(self, command_args): clients = self.mongos_clients if self.mongos_clients else [self.client] for client in clients: - await self._set_fail_point(client, command_args) + await self.configure_fail_point(client, command_args) async def targeted_fail_point(self, session, fail_point): """Run the targetedFailPoint test operation. @@ -282,7 +277,7 @@ async def targeted_fail_point(self, session, fail_point): """ clients = {c.address: c for c in self.mongos_clients} client = clients[session._pinned_address] - await self._set_fail_point(client, fail_point) + await self.configure_fail_point(client, fail_point) self.addAsyncCleanup(self.set_fail_point, {"mode": "off"}) def assert_session_pinned(self, session): diff --git a/test/test_connection_monitoring.py b/test/test_connection_monitoring.py index 05411d17ba..06b1b2c2ec 100644 --- a/test/test_connection_monitoring.py +++ b/test/test_connection_monitoring.py @@ -204,15 +204,10 @@ def check_error(self, actual, expected): self.check_object(actual, expected) self.assertIn(message, str(actual)) - def _set_fail_point(self, client, command_args): - cmd = SON([("configureFailPoint", "failCommand")]) - cmd.update(command_args) - client.admin.command(cmd) - def set_fail_point(self, command_args): if not client_context.supports_failCommand_fail_point: self.skipTest("failCommand fail point must be supported") - self._set_fail_point(self.client, command_args) + self.configure_fail_point(self.client, command_args) def run_scenario(self, scenario_def, test): """Run a CMAP spec test.""" diff --git a/test/test_transactions.py b/test/test_transactions.py index 949b88e60b..7a8dcd0f00 100644 --- a/test/test_transactions.py +++ b/test/test_transactions.py @@ -402,15 +402,10 @@ def setUp(self) -> None: for address in client_context.mongoses: self.mongos_clients.append(self.single_client("{}:{}".format(*address))) - def _set_fail_point(self, client, command_args): - cmd = {"configureFailPoint": "failCommand"} - cmd.update(command_args) - client.admin.command(cmd) - def set_fail_point(self, command_args): clients = self.mongos_clients if self.mongos_clients else [self.client] for client in clients: - self._set_fail_point(client, command_args) + self.configure_fail_point(client, command_args) @client_context.require_transactions def test_callback_raises_custom_error(self): diff --git a/test/unified_format.py b/test/unified_format.py index 372eb8abba..111067cfcf 100644 --- a/test/unified_format.py +++ b/test/unified_format.py @@ -999,12 +999,8 @@ def __set_fail_point(self, client, command_args): if not client_context.test_commands_enabled: self.skipTest("Test commands must be enabled") - cmd_on = SON([("configureFailPoint", "failCommand")]) - cmd_on.update(command_args) - client.admin.command(cmd_on) - self.addCleanup( - client.admin.command, "configureFailPoint", cmd_on["configureFailPoint"], mode="off" - ) + self.configure_fail_point(client, command_args) + self.addCleanup(self.configure_fail_point, client, command_args, off=True) def _testOperation_failPoint(self, spec): self.__set_fail_point( diff --git a/test/utils_spec_runner.py b/test/utils_spec_runner.py index 4508502cd0..ac48bc1cb4 100644 --- a/test/utils_spec_runner.py +++ b/test/utils_spec_runner.py @@ -265,15 +265,10 @@ def setUp(self) -> None: def tearDown(self) -> None: self.knobs.disable() - def _set_fail_point(self, client, command_args): - cmd = SON([("configureFailPoint", "failCommand")]) - cmd.update(command_args) - client.admin.command(cmd) - def set_fail_point(self, command_args): clients = self.mongos_clients if self.mongos_clients else [self.client] for client in clients: - self._set_fail_point(client, command_args) + self.configure_fail_point(client, command_args) def targeted_fail_point(self, session, fail_point): """Run the targetedFailPoint test operation. @@ -282,7 +277,7 @@ def targeted_fail_point(self, session, fail_point): """ clients = {c.address: c for c in self.mongos_clients} client = clients[session._pinned_address] - self._set_fail_point(client, fail_point) + self.configure_fail_point(client, fail_point) self.addCleanup(self.set_fail_point, {"mode": "off"}) def assert_session_pinned(self, session): From ba715b1119bf1e27a07bbe76f12d86e1a287835d Mon Sep 17 00:00:00 2001 From: Shane Harvey Date: Tue, 25 Feb 2025 15:08:38 -0800 Subject: [PATCH 2/2] PYTHON-5120 Fix async test_connection_monitoring.py --- test/asynchronous/test_connection_monitoring.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/asynchronous/test_connection_monitoring.py b/test/asynchronous/test_connection_monitoring.py index a68b2a90cb..cdf4887ba3 100644 --- a/test/asynchronous/test_connection_monitoring.py +++ b/test/asynchronous/test_connection_monitoring.py @@ -211,15 +211,10 @@ def check_error(self, actual, expected): self.check_object(actual, expected) self.assertIn(message, str(actual)) - async def _set_fail_point(self, client, command_args): - cmd = SON([("configureFailPoint", "failCommand")]) - cmd.update(command_args) - await client.admin.command(cmd) - async def set_fail_point(self, command_args): if not async_client_context.supports_failCommand_fail_point: self.skipTest("failCommand fail point must be supported") - await self._set_fail_point(self.client, command_args) + await self.configure_fail_point(self.client, command_args) async def run_scenario(self, scenario_def, test): """Run a CMAP spec test."""