Add comprehensive tests for the query-and-write insert command#597
Add comprehensive tests for the query-and-write insert command#597imforster wants to merge 11 commits into
Conversation
7fbe2bc to
b3d793b
Compare
|
🤖 Auto-triaged by documentdb-triage-tool. Applied: Reasoningcomponent from path globs (test-coverage, test-framework); effort from diff stats (2323+33 LOC, 43 files); LLM: Adds 253 new compatibility test cases across 13 files for the insert command, plus a directory rename affecting the query-and-write tree — multi-file, single component, no schema change. If a label is wrong, remove it manually and ping |
Feature: query-and-write insert command Test cases: 253 Docs: https://www.mongodb.com/docs/v8.2/reference/command/insert/ Adds compatibility test coverage for the query-and-write insert command, validating its full surface area - from basic document insertion and implicit collection creation to BSON type preservation, ordered/unordered error handling, _id field semantics, schema validation, and argument type rejection - across document types and edge cases. 253 test cases across 13 files following project test guidelines. Also renames the query-and-write directory tree to use underscores (query_and_write) for valid Python package naming, and renames read-concern and write-concern subdirectories accordingly. Signed-off-by: Ian Forster <forstaia@amazon.com>
b3d793b to
d0a1030
Compare
Replaces custom BaseTestCase subclasses with CommandTestCase across four insert test files, aligning them with the established pattern used elsewhere in the test suite. - test_insert_batch_size_limits: drop BatchLimitTest, convert TESTS -> BATCH_LIMIT_REJECTED_TESTS using CommandTestCase - test_insert_collection_variants: replace three standalone tests with COLLECTION_VARIANT_TESTS (regular, capped, view); keep capped-wraps-old-docs standalone due to per-iteration assertions - test_insert_core_behavior: drop ResponseTest/DocCheckTest, convert RESPONSE_TESTS and DOC_TESTS -> DOC_PRESERVATION_TESTS to CommandTestCase - test_insert_id_handling: drop IdEquivalenceTest, convert NUMERIC_EQUIVALENCE_TESTS, CROSS_TYPE_DISTINCTION_TESTS to CommandTestCase; merge the two standalone rejected-id-type tests into REJECTED_ID_TYPE_TESTS Signed-off-by: Ian Forster <forstaia@amazon.com>
…ion failure Replace assert isinstance(expected, dict) with cast(Dict[str, Any], ...) to satisfy mypy type narrowing without triggering the test format validator which prohibits plain assert statements in test functions. Signed-off-by: Ian Forster <forstaia@amazon.com>
Raw bytes are converted by the driver on read; use Binary to preserve the expected type consistently across all type sample consumers. Signed-off-by: Ian Forster <forstaia@amazon.com>
|
|
||
| # Property [Command Field Rejection]: insert command rejects invalid BSON types for the | ||
| # collection name field, and rejects missing or empty documents array. | ||
| TESTS: list[FieldValidationTest] = [ |
There was a problem hiding this comment.
▎ [Suggestion] Collection-name field rejection matrix duplicates §19 territory. test_insert_argument_handling.py:21-122 runs 15 BSON-type rejection cases for the insert field, asserting INVALID_NAMESPACE_ERROR. Per TEST_COVERAGE.md §19, "wire-protocol namespace validation" is a foundational behavior that should have a centralized test site (currently TBD) with consumers
▎ running only a single representative case. The repo currently doesn't follow this — compact, drop, count, aggregate, renameCollection, etc. all duplicate the matrix — which makes this PR consistent with sibling commands but inconsistent with the documented rule. Either:
▎ 1. Trim to 1-2 representative cases (per §19's stated rule) and accept divergence from sibling commands.
▎ 2. Open a follow-up issue to create the centralized namespace test site, then trim all commands at once. PR #597 inherits the existing convention in the meantime.
▎
There was a problem hiding this comment.
Bumping this to [Required]. The 15 cases still all assert INVALID_NAMESPACE_ERROR for the collection-name field — that's wire-protocol namespace validation, not insert-specific behavior. Per TEST_COVERAGE.md §19 it belongs in a single shared place; per-command duplication of the matrix scales linearly with the number of commands and drifts when the canonical set changes. Please drop this matrix and rely on the §19 coverage.
eerxuan
left a comment
There was a problem hiding this comment.
About 40-45% of the cases duplicate centralized sites, concentrated in two files:
- test_insert_bson_type_preservation.py — should be deleted, ~10 net-new cases moved to bson_types/types/
- test_insert_numeric_and_datetime_boundaries.py — should be deleted, ~6 net-new cases moved to bson_types/types/
Plus a smaller cleanup pass:
- 15-case namespace BSON matrix in argument_handling.py → 1-2 wiring cases (consistent with §19, inconsistent with the rest of the repo — see my earlier point about this being a repo-wide problem)
- ARRAY_TESTS, STRUCTURE_TESTS in document_structure.py → drop
- empty_documents_array duplicated between two files → drop one
- test_insert_capped_collection_wraps_old_documents → drop (capped/ owns it)
…ry-and-write Signed-off-by: Ian Forster <forstaia@amazon.com>
main relocated command_test_case.py from: core/collections/commands/utils/ to: core/utils/ Update all four insert test files to use the new path. Signed-off-by: Ian Forster <forstaia@amazon.com>
…_succeeds The old name implied boundary coverage of maxWriteBatchSize but the implementation inserted only 1,000 docs (three orders of magnitude under the 100,000 limit). Rename to accurately reflect intent and add a docstring explaining this is a wiring test, not a limit test. Signed-off-by: Ian Forster <forstaia@amazon.com>
DBRef is a client-side convention enforced by the driver, not a server-side feature. The server treats $ref/$id as plain fields. The test was not exercising any server behavior. Signed-off-by: Ian Forster <forstaia@amazon.com>
test_insert_null_field_exists, test_insert_missing_field_not_null, and test_insert_timestamp_zero_gets_autofilled are genuinely insert-command behaviors, not BSON round-trip tests. Move them to test_insert_core_behavior where they belong, ahead of deleting the bson_type_preservation file. Signed-off-by: Ian Forster <forstaia@amazon.com>
The BSON round-trip matrix (BSON_TYPE_TESTS, INT64_BOUNDARY_TESTS, DOUBLE_SPECIAL_TESTS, DATE_BOUNDARY_TESTS, TIMESTAMP_BOUNDARY_TESTS, OID_BOUNDARY_TESTS, DECIMAL128_PRECISION_TESTS, BINARY_SUBTYPE_TESTS, BSON_DISTINCTION_TESTS) materially duplicates the coverage already owned by data-types/bson_types/types/ via RoundTripTestCase. The three insert-command-specific tests (null field semantics, missing field, Timestamp(0,0) auto-fill) were moved to test_insert_core_behavior.py in the previous commit. Signed-off-by: Ian Forster <forstaia@amazon.com>
#39
Feature: query-and-write insert command
Test cases: 253
Docs: https://www.mongodb.com/docs/v8.2/reference/command/insert/
Adds compatibility test coverage for the query-and-write insert command, validating its full surface area - from basic document insertion and implicit collection creation to BSON type preservation, ordered/unordered error handling, _id field semantics, schema validation, and argument type rejection - across document types and edge cases.
253 test cases across 13 files following project test guidelines.
Also renames the query-and-write directory tree to use underscores (query_and_write) for valid Python package naming, and renames read-concern and write-concern subdirectories accordingly.