Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PYTHON-5101 Convert test.test_server_selection_in_window to async #2119

Merged
merged 20 commits into from
Feb 11, 2025

Conversation

sleepyStick
Copy link
Contributor

No description provided.

Copy link

There is an existing patch(es) for this commit SHA:

Please note that the status that is posted is not in the context of this PR but rather the (latest) existing patch and that may affect some tests that may depend on the particular PR. If your tests do not rely on any PR-specific values (like base or head branch name) then your tests will report the same status. If you would like a patch to run in the context of this PR and abort the other(s), comment 'evergreen retry'.

@sleepyStick sleepyStick marked this pull request as ready for review February 6, 2025 02:16
@sleepyStick sleepyStick requested a review from NoahStapp February 6, 2025 02:16

class TestAllScenarios(unittest.IsolatedAsyncioTestCase):
def run_scenario(self, scenario_def):
topology = create_topology(scenario_def)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to make an async_create_topology here to create a pymongo.asynchronous.topology.Topology instance. Our current create_topology method creates a synchronous one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh i see! I ended up making an async version of the utils_selection_tests.py file? thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To minimize code duplication, only classes/functions that differ between async and sync should be in synchro'd files. I'd make a separate utils_selection_tests_shared.py file for all of the code that's the same between both and keep only the code that differs in utils_selection_tests.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good, done!

"appName": "loadBalancingTest",
},
}
with self.fail_point(delay_finds):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
with self.fail_point(delay_finds):
async with self.fail_point(delay_finds):

@sleepyStick sleepyStick requested a review from NoahStapp February 6, 2025 20:19
@sleepyStick sleepyStick requested a review from NoahStapp February 6, 2025 22:02
Copy link
Contributor

@NoahStapp NoahStapp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same test_encryption issue is present here too, we need to update test_encryption.create_tests to not use an async method.

@sleepyStick
Copy link
Contributor Author

The same test_encryption issue is present here too, we need to update test_encryption.create_tests to not use an async method.

Yeahhhh i see that now, okay I fixed it here, I'll assume this PR will be merged first and then pull it from master into the other one

@sleepyStick sleepyStick requested a review from NoahStapp February 7, 2025 15:36
@sleepyStick sleepyStick requested a review from NoahStapp February 7, 2025 16:04
@sleepyStick sleepyStick requested a review from NoahStapp February 8, 2025 01:20
@sleepyStick sleepyStick merged commit 13fa361 into mongodb:master Feb 11, 2025
44 of 46 checks passed
@sleepyStick sleepyStick deleted the PYTHON-5101 branch February 11, 2025 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants