-
Notifications
You must be signed in to change notification settings - Fork 1.1k
PYTHON-5114 Test suite reduce killAllSessions calls #2721
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
Changes from 2 commits
c2052a9
8f23842
c0ac732
dd08d2d
77182fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -621,11 +621,14 @@ async def setup_scenario(self, scenario_def): | |
| async def run_scenario(self, scenario_def, test): | ||
| self.maybe_skip_scenario(test) | ||
|
|
||
| # Kill all sessions before and after each test to prevent an open | ||
| # Kill all sessions after each test with transactions to prevent an open | ||
|
ShaneHarvey marked this conversation as resolved.
Outdated
|
||
| # transaction (from a test failure) from blocking collection/database | ||
| # operations during test set up and tear down. | ||
| await self.kill_all_sessions() | ||
| self.addAsyncCleanup(self.kill_all_sessions) | ||
| for op in test["operations"]: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we have this in both
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a lot of duplication between the two. The legacy class should actually never be running any tests with transactions.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we safely remove this code then?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now yes? But it may be safer to keep it until we delete the remaining legacy tests. Just in case one of them is updated to include transaction operations.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair enough. Can you open a ticket to track that final deletion?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh looking at this again the SpecRunner class is unused so I removed it.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you split the removal into a separate PR for better separation of concerns?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| name = op["name"] | ||
| if name == "startTransaction" or name == "withTransaction": | ||
| self.addAsyncCleanup(self.kill_all_sessions) | ||
| break | ||
| await self.setup_scenario(scenario_def) | ||
| database_name = self.get_scenario_db_name(scenario_def) | ||
| collection_name = self.get_scenario_coll_name(scenario_def) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.