Skip to content

PYTHON-3160 Fix MMAPv1 tests #914

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 31, 2022
Merged

PYTHON-3160 Fix MMAPv1 tests #914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9ac70d7
PYTHON-3160 Fix mmapv1 tests
ShaneHarvey Mar 29, 2022
ed6d235
PYTHON-3160 Fix, skip change stream tests on mmapv1
ShaneHarvey Mar 30, 2022
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
1 change: 0 additions & 1 deletion test/sessions/driver-sessions-dirty-session-errors.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,6 @@
"name": "insertOne",
"object": "collection0",
"arguments": {
"session": "session0",
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a bug fix in the spec test that I will upstream.

"document": {
"_id": 2
}
Expand Down
10 changes: 4 additions & 6 deletions test/test_session.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,11 @@ def test_implicit_sessions_checkout(self):
# "To confirm that implicit sessions only allocate their server session after a
# successful connection checkout" test from Driver Sessions Spec.
succeeded = False
lsid_set = set()
failures = 0
for _ in range(5):
listener = EventListener()
client = rs_or_single_client(
event_listeners=[listener], maxPoolSize=1, retryWrites=True
)
client = rs_or_single_client(event_listeners=[listener], maxPoolSize=1)
Copy link
Member Author

Choose a reason for hiding this comment

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

This test doesn't need retryWrite=True (which doesn't work on MMAPv1).

cursor = client.db.test.find({})
ops: List[Tuple[Callable, List[Any]]] = [
(client.db.test.find_one, [{"_id": 1}]),
Expand Down Expand Up @@ -225,16 +224,15 @@ def thread_target(op, *args):
thread.join()
self.assertIsNone(thread.exc)
client.close()
lsid_set = set()
lsid_set.clear()
for i in listener.results["started"]:
if i.command.get("lsid"):
lsid_set.add(i.command.get("lsid")["id"])
if len(lsid_set) == 1:
succeeded = True
else:
failures += 1
print(failures)
self.assertTrue(succeeded)
self.assertTrue(succeeded, lsid_set)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a small improvement to include the last lsid_set in the error output when the test fails.


def test_pool_lifo(self):
# "Pool is LIFO" test from Driver Sessions Spec.
Expand Down
21 changes: 20 additions & 1 deletion test/unified_format.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -766,11 +766,30 @@ def setUp(self):
def maybe_skip_test(self, spec):
# add any special-casing for skipping tests here
if client_context.storage_engine == "mmapv1":
if "Dirty explicit session is discarded" in spec["description"]:
if (
"Dirty explicit session is discarded" in spec["description"]
or "Dirty implicit session is discarded" in spec["description"]
):
raise unittest.SkipTest("MMAPv1 does not support retryWrites=True")
elif "Client side error in command starting transaction" in spec["description"]:
raise unittest.SkipTest("Implement PYTHON-1894")

# Some tests need to be skipped based on the operations they try to run.
for op in spec["operations"]:
name = op["name"]
if name == "count":
self.skipTest("PyMongo does not support count()")
if name == "listIndexNames":
self.skipTest("PyMongo does not support list_index_names()")
if client_context.storage_engine == "mmapv1":
if name == "createChangeStream":
self.skipTest("MMAPv1 does not support change streams")
if name == "withTransaction" or name == "startTransaction":
self.skipTest("MMAPv1 does not support document-level locking")
if not client_context.test_commands_enabled:
if name == "failPoint" or name == "targetedFailPoint":
self.skipTest("Test commands must be enabled to use fail points")
Copy link
Member Author

@ShaneHarvey ShaneHarvey Mar 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The explanation here is that we're trying to skip createChangeStream tests on mmapv1 at run time which would cause problems when the test used both createChangeStream and expectError. The new code checks if we should skip the test before executing it. This also speeds up the test suite a bit since we don't have to run half of a bunch of tests just to find out we can skip them.


def process_error(self, exception, spec):
is_error = spec.get("isError")
is_client_error = spec.get("isClientError")
Expand Down