Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions test/asynchronous/unified_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -1464,11 +1464,6 @@ async def verify_outcome(self, spec):
self.assertListEqual(sorted_expected_documents, actual_documents)

async def run_scenario(self, spec, uri=None):
# Kill all sessions before and after each test to prevent an open
# transaction (from a test failure) from blocking collection/database
# operations during test set up and tear down.
await self.kill_all_sessions()

# Handle flaky tests.
flaky_tests = [
("PYTHON-5170", ".*test_discovery_and_monitoring.*"),
Expand Down Expand Up @@ -1504,6 +1499,15 @@ async def _run_scenario(self, spec, uri=None):
if skip_reason is not None:
raise unittest.SkipTest(f"{skip_reason}")

# Kill all sessions after each test with transactions to prevent an open
Comment thread
ShaneHarvey marked this conversation as resolved.
# transaction (from a test failure) from blocking collection/database
# operations during test set up and tear down.
for op in spec["operations"]:
name = op["name"]
if name == "startTransaction" or name == "withTransaction":
self.addAsyncCleanup(self.kill_all_sessions)
break

# process createEntities
self._uri = uri
self.entity_map = EntityMapUtil(self)
Expand Down
9 changes: 6 additions & 3 deletions test/asynchronous/utils_spec_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
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"]:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we have this in both unified_format and utils_spec_runner?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we safely remove this code then?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fair enough. Can you open a ticket to track that final deletion?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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)
Expand Down
14 changes: 9 additions & 5 deletions test/unified_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -1451,11 +1451,6 @@ def verify_outcome(self, spec):
self.assertListEqual(sorted_expected_documents, actual_documents)

def run_scenario(self, spec, uri=None):
# Kill all sessions before and after each test to prevent an open
# transaction (from a test failure) from blocking collection/database
# operations during test set up and tear down.
self.kill_all_sessions()

# Handle flaky tests.
flaky_tests = [
("PYTHON-5170", ".*test_discovery_and_monitoring.*"),
Expand Down Expand Up @@ -1491,6 +1486,15 @@ def _run_scenario(self, spec, uri=None):
if skip_reason is not None:
raise unittest.SkipTest(f"{skip_reason}")

# Kill all sessions after each test with transactions to prevent an open
Comment thread
ShaneHarvey marked this conversation as resolved.
# transaction (from a test failure) from blocking collection/database
# operations during test set up and tear down.
for op in spec["operations"]:
name = op["name"]
if name == "startTransaction" or name == "withTransaction":
self.addCleanup(self.kill_all_sessions)
break

Comment thread
ShaneHarvey marked this conversation as resolved.
# process createEntities
self._uri = uri
self.entity_map = EntityMapUtil(self)
Expand Down
9 changes: 6 additions & 3 deletions test/utils_spec_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -621,11 +621,14 @@ def setup_scenario(self, scenario_def):
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
Comment thread
ShaneHarvey marked this conversation as resolved.
Outdated
# transaction (from a test failure) from blocking collection/database
# operations during test set up and tear down.
self.kill_all_sessions()
self.addCleanup(self.kill_all_sessions)
for op in test["operations"]:
name = op["name"]
if name == "startTransaction" or name == "withTransaction":
self.addCleanup(self.kill_all_sessions)
break
self.setup_scenario(scenario_def)
database_name = self.get_scenario_db_name(scenario_def)
collection_name = self.get_scenario_coll_name(scenario_def)
Expand Down
Loading