PYTHON-5073 Convert test.test_connection_monitoring to async by sleepyStick · Pull Request #2087 · mongodb/mongo-python-driver

sleepyStick · 2025-01-28T23:18:07Z

Convert test.test_connection_monitoring to async

NoahStapp · 2025-01-29T13:20:17Z

test/asynchronous/test_connection_monitoring.py

+    #
+    async def test_1_client_connection_pool_options(self):
+        client = await self.async_rs_or_single_client(**self.POOL_OPTIONS)
+        self.addAsyncCleanup(client.close)


Our helper methods like self.async_rs_or_single_client call self.addAsyncCleanup(client.close) for us, so we don't need to do it ourselves.

NoahStapp · 2025-01-29T13:27:50Z

test/asynchronous/test_connection_monitoring.py

+    def start(self, op):
+        """Run the 'start' thread operation."""
+        target = op["target"]
+        thread = SpecRunnerThread(target)


There's a separate ticket to create an asynchronous version of SpecRunnerThread using asyncio tasks: https://jira.mongodb.org/browse/PYTHON-4864. Until we implement that ticket, we don't want to use a thread-based SpecRunnerThread for asynchronous tests due to the poor performance and inconsistent behavior of mixing threads and asyncio.

Makes sense, until that ticket is complete, what should I do for the tests? (or is this going to need to wait for that ticket to be completed?)

wait ignore me, my brain is finally waking up and realizing that's what the last comment was for HAHA

NoahStapp · 2025-01-29T13:29:28Z

test/asynchronous/test_connection_monitoring.py

+
+class AsyncTestCMAP(AsyncIntegrationTest):
+    # Location of JSON test specifications.
+    TEST_PATH = os.path.join(os.path.dirname(os.path.realpath(__file__)), "connection_monitoring")


We need to update this path to reflect the asynchronous subfolder inside test. The pattern is the same across all asynchronous tests that use a TEST_PATH:

if _IS_SYNC: _TEST_PATH = os.path.join(pathlib.Path(__file__).resolve().parent, <module_name>) else: _TEST_PATH = os.path.join(pathlib.Path(__file__).resolve().parent.parent, <module_name>)

NoahStapp · 2025-01-29T13:49:28Z

test/asynchronous/test_connection_monitoring.py

+        return [scenario_def]
+
+
+test_creator = CMAPSpecTestCreator(create_test, AsyncTestCMAP, AsyncTestCMAP.TEST_PATH)


To only run the SpecRunnerThread tests on the sync API:

if _IS_SYNC: test_creator = CMAPSpecTestCreator(create_test, AsyncTestCMAP, AsyncTestCMAP.TEST_PATH) test_creator.create_tests()

NoahStapp

Great work, just one minor cleanup!

NoahStapp · 2025-01-29T18:43:18Z

test/asynchronous/test_connection_monitoring.py

    # Location of JSON test specifications.
-    TEST_PATH = os.path.join(os.path.dirname(os.path.realpath(__file__)), "connection_monitoring")
+    if _IS_SYNC:
+        TEST_PATH = os.path.join(pathlib.Path(__file__).resolve().parent, "connection_monitoring")


We can do Path instead of pathlib.Path and change the import to from pathlib import Path for clarity here.

NoahStapp

Looks great, let's wait to merge this until #2094 is merged so we can add in the tests that rely on SpecRunnerThread too.

NoahStapp · 2025-02-05T19:38:13Z

test/asynchronous/utils_spec_runner.py


                # Construct test from scenario.
-                for test_def in self.tests(scenario_def):
+                for test_def in await self.tests(scenario_def):


self.tests is not asynchronous, so no await needed here.

okay i thought it was really weird too, but without the await i got the following error:

Traceback (most recent call last): File "/opt/homebrew/Cellar/python@3.12/3.12.8/Frameworks/Python.framework/Versions/3.12/lib/python3.12/warnings.py", line 555, in _warn_unawaited_coroutine warn(msg, category=RuntimeWarning, stacklevel=2, source=coro) RuntimeWarning: coroutine 'CMAPSpecTestCreator.tests' was never awaited test/asynchronous/test_connection_monitoring.py:None (test/asynchronous/test_connection_monitoring.py) test_connection_monitoring.py:475: in <module> test_creator.create_tests() utils_spec_runner.py:243: in create_tests asyncio.run(self._create_tests()) /opt/homebrew/Cellar/python@3.12/3.12.8/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/runners.py:194: in run return runner.run(main) /opt/homebrew/Cellar/python@3.12/3.12.8/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/runners.py:118: in run return self._loop.run_until_complete(task) /opt/homebrew/Cellar/python@3.12/3.12.8/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/base_events.py:686: in run_until_complete return future.result() utils_spec_runner.py:225: in _create_tests for test_def in self.tests(scenario_def): E TypeError: 'coroutine' object is not iterable```

Ah, found the issue:

class CMAPSpecTestCreator(AsyncSpecTestCreator): async def tests(self, scenario_def): ...

should be

class CMAPSpecTestCreator(AsyncSpecTestCreator): def tests(self, scenario_def): ...

And then await self.tests() can become self.tests()

ahh! good find! thanks!

NoahStapp · 2025-02-06T18:53:41Z

Encryption test failure is due to test_encryption's create_test being async when it should be sync: https://evergreen.mongodb.com/test_log/mongo_python_driver_encryption_rhel8_python3.10_test_4.0_sharded_cluster_auth_ssl_sync_async_patch_3dd44e6e5f89ffc6171c8ffc7ed052b620e35065_67a4fc4e7a1a330007e6f608_25_02_06_18_15_43/0?test_name=4b8a3fb7e7ebd8ee1a0182780cc2ba8a#L0

sleepyStick · 2025-02-11T18:15:59Z

Encryption test failure is due to test_encryption's create_test being async when it should be sync: https://evergreen.mongodb.com/test_log/mongo_python_driver_encryption_rhel8_python3.10_test_4.0_sharded_cluster_auth_ssl_sync_async_patch_3dd44e6e5f89ffc6171c8ffc7ed052b620e35065_67a4fc4e7a1a330007e6f608_25_02_06_18_15_43/0?test_name=4b8a3fb7e7ebd8ee1a0182780cc2ba8a#L0

Should be fixed now :)

sleepyStick added 2 commits January 28, 2025 15:17

Convert test.test_connection_monitoring to async

19896a8

fix

fadef89

sleepyStick requested a review from NoahStapp January 29, 2025 03:30

NoahStapp requested changes Jan 29, 2025

View reviewed changes

sleepyStick changed the title ~~PYTHON-5072 Convert test.test_connection_monitoring to async~~ PYTHON-5073 Convert test.test_connection_monitoring to async Jan 29, 2025

sleepyStick added 3 commits January 29, 2025 09:34

address review comments

f0bb7c8

Merge branch 'master' into PYTHON-5073

58bae37

don't mind me being silly

6a6577a

sleepyStick marked this pull request as ready for review January 29, 2025 17:48

NoahStapp requested changes Jan 29, 2025

View reviewed changes

sleepyStick and others added 2 commits January 29, 2025 11:24

change import

1dbe1f9

Merge branch 'master' into PYTHON-5073

eb74f1b

sleepyStick requested a review from NoahStapp January 29, 2025 20:17

NoahStapp reviewed Jan 29, 2025

View reviewed changes

sleepyStick and others added 3 commits January 29, 2025 13:18

Merge branch 'master' into PYTHON-5073

ad106b2

Merge branch 'master' into PYTHON-5073

7bde85c

run all tests with SpecRunnerTask

0e5df26

sleepyStick requested a review from NoahStapp February 5, 2025 19:30

NoahStapp requested changes Feb 5, 2025

View reviewed changes

sleepyStick and others added 2 commits February 6, 2025 10:13

Merge branch 'master' into PYTHON-5073

b2f8d98

fix tests

8cd9f1a

sleepyStick requested a review from NoahStapp February 6, 2025 18:16

sleepyStick added 2 commits February 6, 2025 11:53

Merge branch 'master' into PYTHON-5073

ebe590f

Merge branch 'master' into PYTHON-5073

6f05489

ShaneHarvey mentioned this pull request Feb 7, 2025

PYTHON-5120 Reduce configureFailPoint duplication in tests #2131

Merged

Merge branch 'master' into PYTHON-5073

469d10b

Merge branch 'master' into PYTHON-5073

695e38a

NoahStapp approved these changes Feb 11, 2025

View reviewed changes

sleepyStick merged commit 0e6aa6f into mongodb:master Feb 11, 2025
45 of 49 checks passed

sleepyStick deleted the PYTHON-5073 branch February 11, 2025 22:01

		return [scenario_def]


		test_creator = CMAPSpecTestCreator(create_test, AsyncTestCMAP, AsyncTestCMAP.TEST_PATH)

Conversation

sleepyStick commented Jan 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NoahStapp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NoahStapp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NoahStapp Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NoahStapp commented Feb 6, 2025

Uh oh!

sleepyStick commented Feb 11, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

NoahStapp Feb 6, 2025 •

edited

Loading