PYTHON-5782 Coverage increase, remove old code in message.py#2772
PYTHON-5782 Coverage increase, remove old code in message.py#2772aclark4life wants to merge 12 commits into
message.py#2772Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new unit test module to increase coverage of pymongo/message.py by exercising message-building utilities and related pure-Python helper functions without requiring a live MongoDB server.
Changes:
- Adds
test/test_message.pywith unit tests for message helpers (read preference wrapping, command generation, write-result conversion). - Adds tests covering OP_MSG / OP_QUERY / OP_GET_MORE message construction in both compressed and uncompressed paths.
- Adds tests for error conversion and document-too-large error formatting.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
a6c8f8e to
a8d9f92
Compare
4baf2a6 to
7b31987
Compare
message.py
message.pymessage.py
message.pymessage.py
|
Test failures flaky: |
| self.assertEqual(result["upserted"][0]["_id"], 42) | ||
|
|
||
| def test_update_legacy_upsert_id_from_update_doc(self): | ||
| # Pre-2.6 servers omit "upserted"; _id is extracted from the update doc (takes |
There was a problem hiding this comment.
We don't support servers <4.2, so no need to test them. Any code that still makes a distinction should be removed.
| elif operation == "update": | ||
| if "upserted" in result: | ||
| res["upserted"] = [{"index": 0, "_id": result["upserted"]}] | ||
| # Versions of MongoDB before 2.6 don't return the _id for an | ||
| # upsert if _id is not an ObjectId. | ||
| elif result.get("updatedExisting") is False and affected == 1: | ||
| # If _id is in both the update document *and* the query spec | ||
| # the update document _id takes precedence. | ||
| update = command["updates"][0] | ||
| _id = update["u"].get("_id", update["q"].get("_id")) | ||
| res["upserted"] = [{"index": 0, "_id": _id}] | ||
| return res |
There was a problem hiding this comment.
Agreed that the PR should be updated to reflect that there's this minor refactor here.
|
|
||
| # _gen_find_command | ||
|
|
||
| def test_find_basic(self): |
There was a problem hiding this comment.
All of the calls to _gen_find_command and _gen_get_more_command should use keyword args for readability.
| self.assertEqual(max_doc_size, 0) | ||
|
|
||
| def test_op_msg_max_doc_size_matches_largest_encoded_doc(self): | ||
| docs = [{"_id": 1, "x": 2}, {"_id": 3, "x": 4}] |
There was a problem hiding this comment.
These encode to the same size, so the assert would pass regardless of how max_doc_size is set. One of these should be larger than the other to ensure correctness.
| with self.assertRaises(DocumentTooLarge) as ctx: | ||
| _raise_document_too_large("update", 2_000_000, 1_000_000) | ||
| self.assertIn("update", str(ctx.exception)) |
There was a problem hiding this comment.
| with self.assertRaises(DocumentTooLarge) as ctx: | |
| _raise_document_too_large("update", 2_000_000, 1_000_000) | |
| self.assertIn("update", str(ctx.exception)) | |
| with self.assertRaisesRegex(DocumentTooLarge, "update command document too large"): | |
| _raise_document_too_large("update", 2_000_000, 1_000_000) |
message.pymessage.py
message.pymessage.py
- Replace _MockCtx with real ZlibContext(-1) to satisfy the SnappyContext | ZlibContext | ZstdContext union type - Annotate docs list as list to satisfy the Mapping invariance check - Add # type: ignore[arg-type] on _MockConn call sites for _gen_get_more_command (mocking AsyncConnection | Connection fully would require a much heavier stub)
Address Copilot feedback: add tests for legacy upsert _id fallback in _convert_write_result and for the zlib-compressed _op_msg path.
Guard zlib compression test with _have_zlib() skip condition.
- Use keyword args in all _gen_find_command and _gen_get_more_command calls - Fix test_op_msg_max_doc_size_matches_largest_encoded_doc to use docs of different sizes so the max() assertion is meaningful - Use assertRaisesRegex with the exact message in test_raise_document_too_large_update_generic_message - Remove !r from the DocumentTooLarge message in _raise_document_too_large so it matches
| # There's nothing intelligent we can say | ||
| # about size for update and delete | ||
| raise DocumentTooLarge(f"{operation!r} command document too large") | ||
| raise DocumentTooLarge(f"{operation} command document too large") |
PYTHON-5782
Changes in this PR
Adds
test/test_message.pywith unit tests forpymongo/message.py.Also removes a dead code branch in
_convert_write_result()— a legacy compatibility shim for MongoDB < 2.6 that manually reconstructed the upserted_idwhen it wasn't an ObjectId. MongoDB 2.6+ always returns theupsertedfield directly, so this branch was unreachable.Tests cover:
Result / error conversion
_convert_exception()— converts anExceptioninto anerrmsg/errtypefailure document for event publishing_convert_client_bulk_exception()— same as above but also captures the errorcode, used by the client-level bulk write API_convert_write_result()— converts a legacy GLE (Get Last Error) write result into write-command format, handling insert/update/delete/upsert, wtimeout, and errInfo branches_raise_document_too_large()— raisesDocumentTooLargewith a size-aware message for inserts or a generic message for other operationsWire-format message construction
_op_msg()— builds an OP_MSG wire message, injecting$dband$readPreference, and temporarily popping document-sequence fields before encodingCommand builders
_gen_find_command()— builds afindcommand document from a query spec, applying projections, skip/limit, batch size, read concern, collation, and cursor options_gen_get_more_command()— builds agetMorecommand document, conditionally including batch size,maxTimeMS, andcomment(gated on wire version ≥ 9)Read preference
_maybe_add_read_preference()— injects$readPreferenceinto a query spec for non-primary modes, skipping plainsecondaryPreferredwhen no tags or maxStalenessSeconds are setTest Plan
Added unit tests using a real
ZlibContextfor compression paths and aMagicMockconnection for_gen_get_more_command. Focuses on the pure-Python code paths without requiring a live MongoDB server.Checklist
Checklist for Author
Checklist for Reviewer