Skip to content

Commit 408639c

Browse files
aclark4lifeaclark4life
committed
PYTHON-5782 Noah feedback
- 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
1 parent 06ade41 commit 408639c

2 files changed

Lines changed: 171 additions & 23 deletions

File tree

‎pymongo/message.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -652,7 +652,7 @@ def _raise_document_too_large(operation: str, doc_size: int, max_size: int) -> N
652652
else:
653653
# There's nothing intelligent we can say
654654
# about size for update and delete
655-
raise DocumentTooLarge(f"{operation!r} command document too large")
655+
raise DocumentTooLarge(f"{operation} command document too large")
656656

657657

658658
# From the Client Side Encryption spec:

‎test/test_message.py‎

Lines changed: 170 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ def test_op_msg_max_doc_size_zero_without_docs(self):
154154
self.assertEqual(max_doc_size, 0)
155155

156156
def test_op_msg_max_doc_size_matches_largest_encoded_doc(self):
157-
docs = [{"_id": 1, "x": 2}, {"_id": 3, "x": 4}]
157+
docs = [{"_id": 1}, {"_id": 2, "data": "a" * 100}]
158158
cmd: dict = {"insert": "col", "documents": docs}
159159
max_doc_size = _op_msg(0, cmd, "testdb", None, _OPTS)[3]
160160
self.assertEqual(max_doc_size, max(len(encode(d)) for d in docs))
@@ -199,103 +199,251 @@ def test_raise_document_too_large_insert_includes_sizes(self):
199199
self.assertIn("1000000", msg)
200200

201201
def test_raise_document_too_large_update_generic_message(self):
202-
with self.assertRaises(DocumentTooLarge) as ctx:
202+
with self.assertRaisesRegex(DocumentTooLarge, "update command document too large"):
203203
_raise_document_too_large("update", 2_000_000, 1_000_000)
204-
self.assertIn("update", str(ctx.exception))
205204

206205
# _gen_find_command
207206

208207
def test_find_basic(self):
209-
cmd = _gen_find_command("col", {}, None, 0, 0, None, None, ReadConcern())
208+
cmd = _gen_find_command(
209+
"col",
210+
{},
211+
projection=None,
212+
skip=0,
213+
limit=0,
214+
batch_size=None,
215+
options=None,
216+
read_concern=ReadConcern(),
217+
)
210218
self.assertEqual(cmd["find"], "col")
211219
self.assertEqual(cmd["filter"], {})
212220

213221
def test_find_with_projection(self):
214-
cmd = _gen_find_command("col", {}, {"x": 1}, 0, 0, None, None, ReadConcern())
222+
cmd = _gen_find_command(
223+
"col",
224+
{},
225+
projection={"x": 1},
226+
skip=0,
227+
limit=0,
228+
batch_size=None,
229+
options=None,
230+
read_concern=ReadConcern(),
231+
)
215232
self.assertEqual(cmd["projection"], {"x": 1})
216233

217234
def test_find_with_skip(self):
218-
cmd = _gen_find_command("col", {}, None, 5, 0, None, None, ReadConcern())
235+
cmd = _gen_find_command(
236+
"col",
237+
{},
238+
projection=None,
239+
skip=5,
240+
limit=0,
241+
batch_size=None,
242+
options=None,
243+
read_concern=ReadConcern(),
244+
)
219245
self.assertEqual(cmd["skip"], 5)
220246

221247
def test_find_with_positive_limit(self):
222-
cmd = _gen_find_command("col", {}, None, 0, 10, None, None, ReadConcern())
248+
cmd = _gen_find_command(
249+
"col",
250+
{},
251+
projection=None,
252+
skip=0,
253+
limit=10,
254+
batch_size=None,
255+
options=None,
256+
read_concern=ReadConcern(),
257+
)
223258
self.assertEqual(cmd["limit"], 10)
224259
self.assertNotIn("singleBatch", cmd)
225260

226261
def test_find_with_negative_limit_sets_single_batch(self):
227-
cmd = _gen_find_command("col", {}, None, 0, -5, None, None, ReadConcern())
262+
cmd = _gen_find_command(
263+
"col",
264+
{},
265+
projection=None,
266+
skip=0,
267+
limit=-5,
268+
batch_size=None,
269+
options=None,
270+
read_concern=ReadConcern(),
271+
)
228272
self.assertEqual(cmd["limit"], 5)
229273
self.assertTrue(cmd["singleBatch"])
230274

231275
def test_find_batch_size_adjusted_when_equal_to_limit(self):
232-
cmd = _gen_find_command("col", {}, None, 0, 10, 10, None, ReadConcern())
276+
cmd = _gen_find_command(
277+
"col",
278+
{},
279+
projection=None,
280+
skip=0,
281+
limit=10,
282+
batch_size=10,
283+
options=None,
284+
read_concern=ReadConcern(),
285+
)
233286
self.assertEqual(cmd["batchSize"], 11)
234287

235288
def test_find_batch_size_not_adjusted_when_different(self):
236289
# Covers the False branch of `if limit == batch_size:` — distinct from the True branch above.
237-
cmd = _gen_find_command("col", {}, None, 0, 10, 5, None, ReadConcern())
290+
cmd = _gen_find_command(
291+
"col",
292+
{},
293+
projection=None,
294+
skip=0,
295+
limit=10,
296+
batch_size=5,
297+
options=None,
298+
read_concern=ReadConcern(),
299+
)
238300
self.assertEqual(cmd["batchSize"], 5)
239301

240302
def test_find_read_concern_level_included(self):
241-
cmd = _gen_find_command("col", {}, None, 0, 0, None, None, ReadConcern("majority"))
303+
cmd = _gen_find_command(
304+
"col",
305+
{},
306+
projection=None,
307+
skip=0,
308+
limit=0,
309+
batch_size=None,
310+
options=None,
311+
read_concern=ReadConcern("majority"),
312+
)
242313
self.assertEqual(cmd["readConcern"], {"level": "majority"})
243314

244315
def test_find_query_with_dollar_query_modifier(self):
245316
spec = {"$query": {"x": 1}, "$orderby": {"x": 1}}
246-
cmd = _gen_find_command("col", spec, None, 0, 0, None, None, ReadConcern())
317+
cmd = _gen_find_command(
318+
"col",
319+
spec,
320+
projection=None,
321+
skip=0,
322+
limit=0,
323+
batch_size=None,
324+
options=None,
325+
read_concern=ReadConcern(),
326+
)
247327
self.assertIn("sort", cmd)
248328
self.assertNotIn("$orderby", cmd)
249329
self.assertNotIn("$query", cmd)
250330

251331
def test_find_allow_disk_use(self):
252332
cmd = _gen_find_command(
253-
"col", {}, None, 0, 0, None, None, ReadConcern(), allow_disk_use=True
333+
"col",
334+
{},
335+
projection=None,
336+
skip=0,
337+
limit=0,
338+
batch_size=None,
339+
options=None,
340+
read_concern=ReadConcern(),
341+
allow_disk_use=True,
254342
)
255343
self.assertTrue(cmd["allowDiskUse"])
256344

257345
def test_find_collation(self):
258346
cmd = _gen_find_command(
259-
"col", {}, None, 0, 0, None, None, ReadConcern(), collation={"locale": "en"}
347+
"col",
348+
{},
349+
projection=None,
350+
skip=0,
351+
limit=0,
352+
batch_size=None,
353+
options=None,
354+
read_concern=ReadConcern(),
355+
collation={"locale": "en"},
260356
)
261357
self.assertEqual(cmd["collation"]["locale"], "en")
262358

263359
def test_find_options_tailable(self):
264-
cmd = _gen_find_command("col", {}, None, 0, 0, None, 2, ReadConcern())
360+
cmd = _gen_find_command(
361+
"col",
362+
{},
363+
projection=None,
364+
skip=0,
365+
limit=0,
366+
batch_size=None,
367+
options=2,
368+
read_concern=ReadConcern(),
369+
)
265370
self.assertTrue(cmd.get("tailable"))
266371

267372
def test_find_dollar_query_with_explain_removed(self):
268373
spec = {"$query": {"x": 1}, "$explain": 1}
269-
cmd = _gen_find_command("col", spec, None, 0, 0, None, None, ReadConcern())
374+
cmd = _gen_find_command(
375+
"col",
376+
spec,
377+
projection=None,
378+
skip=0,
379+
limit=0,
380+
batch_size=None,
381+
options=None,
382+
read_concern=ReadConcern(),
383+
)
270384
self.assertNotIn("$explain", cmd)
271385

272386
def test_find_dollar_query_with_read_preference_removed(self):
273387
# Covers the separate `if "$readPreference" in cmd:` branch — not entered by test_dollar_query_with_explain_removed.
274388
spec = {"$query": {"x": 1}, "$readPreference": {"mode": "secondary"}}
275-
cmd = _gen_find_command("col", spec, None, 0, 0, None, None, ReadConcern())
389+
cmd = _gen_find_command(
390+
"col",
391+
spec,
392+
projection=None,
393+
skip=0,
394+
limit=0,
395+
batch_size=None,
396+
options=None,
397+
read_concern=ReadConcern(),
398+
)
276399
self.assertNotIn("$readPreference", cmd)
277400

278401
# _gen_get_more_command
279402

280403
def test_get_more_basic(self):
281-
cmd = _gen_get_more_command(12345, "col", None, None, None, self._make_conn())
404+
cmd = _gen_get_more_command(
405+
12345,
406+
"col",
407+
batch_size=None,
408+
max_await_time_ms=None,
409+
comment=None,
410+
conn=self._make_conn(),
411+
)
282412
self.assertEqual(cmd["getMore"], 12345)
283413
self.assertEqual(cmd["collection"], "col")
284414

285415
def test_get_more_with_batch_size(self):
286-
cmd = _gen_get_more_command(1, "col", 100, None, None, self._make_conn())
416+
cmd = _gen_get_more_command(
417+
1, "col", batch_size=100, max_await_time_ms=None, comment=None, conn=self._make_conn()
418+
)
287419
self.assertEqual(cmd["batchSize"], 100)
288420

289421
def test_get_more_with_max_await_time_ms(self):
290-
cmd = _gen_get_more_command(1, "col", None, 500, None, self._make_conn())
422+
cmd = _gen_get_more_command(
423+
1, "col", batch_size=None, max_await_time_ms=500, comment=None, conn=self._make_conn()
424+
)
291425
self.assertEqual(cmd["maxTimeMS"], 500)
292426

293427
def test_get_more_comment_added_on_high_wire_version(self):
294-
cmd = _gen_get_more_command(1, "col", None, None, "my comment", self._make_conn(9))
428+
cmd = _gen_get_more_command(
429+
1,
430+
"col",
431+
batch_size=None,
432+
max_await_time_ms=None,
433+
comment="my comment",
434+
conn=self._make_conn(9),
435+
)
295436
self.assertEqual(cmd["comment"], "my comment")
296437

297438
def test_get_more_comment_not_added_on_low_wire_version(self):
298-
cmd = _gen_get_more_command(1, "col", None, None, "my comment", self._make_conn(8))
439+
cmd = _gen_get_more_command(
440+
1,
441+
"col",
442+
batch_size=None,
443+
max_await_time_ms=None,
444+
comment="my comment",
445+
conn=self._make_conn(8),
446+
)
299447
self.assertNotIn("comment", cmd)
300448

301449

0 commit comments

Comments
 (0)