Skip to content

Add comprehensive tests for the query-and-write insert command#597

Open
imforster wants to merge 11 commits into
documentdb:mainfrom
imforster:forstaia/insert/query-and-write
Open

Add comprehensive tests for the query-and-write insert command#597
imforster wants to merge 11 commits into
documentdb:mainfrom
imforster:forstaia/insert/query-and-write

Conversation

@imforster

Copy link
Copy Markdown
Collaborator

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

@imforster imforster requested a review from a team as a code owner June 11, 2026 21:28
@imforster imforster force-pushed the forstaia/insert/query-and-write branch from 7fbe2bc to b3d793b Compare June 11, 2026 22:21
@documentdb-triage-tool

Copy link
Copy Markdown

🤖 Auto-triaged by documentdb-triage-tool.

Applied: compatibility test, enhancement
Project fields suggested: Component test-coverage · Priority P2 · Effort XL · Status Needs Review
Confidence: 0.90 (mixed)

Reasoning

component 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 @patty-chow so the rules can be tuned. The bot will not re-label items that already have component labels.

@documentdb-triage-tool documentdb-triage-tool Bot added compatibility test Compatibility test related enhancement New feature or request labels Jun 11, 2026
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>
@imforster imforster force-pushed the forstaia/insert/query-and-write branch from b3d793b to d0a1030 Compare June 11, 2026 22:41
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>
Comment thread documentdb_tests/framework/test_constants.py Outdated
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] = [

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

▎ [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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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 eerxuan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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>
@imforster imforster requested a review from eerxuan June 12, 2026 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compatibility test Compatibility test related enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants