Adding tests for $addToSet, $min, $currentDate, $inc, for increased coverage

[SarthakDalmia1]

This is part of the following issue #618

[documentdb-triage-tool]

🤖 Auto-triaged by documentdb-triage-tool.

Applied: compatibility test, enhancement
Project fields suggested: Component test-coverage · Priority P2 · Effort L · Status In Progress
Confidence: 0.80 (mixed)

Reasoning

component from path globs (test-coverage); effort from diff stats (1522+0 LOC, 3 files); LLM: Adds new compatibility tests for multiple update operators ($addToSet, $min, $currentDate, $inc) to increase test coverage.

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.

[eerxuan]

Thanks for the coverage push. The behaviors themselves are reasonable, but as written these six *_behaviors.py files mostly duplicate coverage that already exists in each folder's aspect-split files (added earlier), a few tests are in the wrong folder, and the format diverges from the established convention. Rather than land new monolithic files, please fold the genuinely-new cases into the existing files and drop the rest. Details below.

1. Move — window-frame tests are $setWindowFields stage coverage, not $addToSet coverage

All 7 tests in window/addToSet/test_window_addToSet_behaviors.py (bounded_documents_window, range_based_window, partitionBy_resets_per_partition, self_only_window_yields_single_element, unbounded_window_collects_full_partition, errors_when_documents_and_unit_both_set, errors_when_no_sortBy_for_documents_window) exercise $setWindowFields frame semantics, not anything specific to $addToSet. This is a compatibility framework — frame semantics are assumed spec-consistent across all window operators, so we don't re-test them per operator.

Please move them to core/operator/stages/setWindowFields/ (e.g. test_setWindowFields_window_bounds.py), keeping $addToSet as a representative operator, with a module docstring making the intent explicit — e.g. "Validates $setWindowFields frame semantics using $addToSet as a representative window operator; not re-tested per operator, spec assumed consistent." window/addToSet/ then keeps only its existing test_smoke_window_addToSet.py (one smoke case per operator, per TEST_COVERAGE.md §18).

2. Move — findAndModify tests belong in the findAndModify folder

Per REVIEW.md "Feature ownership", test_inc_via_findAndModify_returns_updated_doc, test_min_via_findAndModify_returns_updated_doc, and test_currentDate_via_findAndModify_returns_updated_doc verify findAndModify's new:true / lastErrorObject behavior, not the operator's arithmetic. Move to core/query_and_write/commands/findAndModify/ as one representative context case, not one per operator.

3. Keep — genuinely new; insert into the existing aspect files

New test(s)	Insert into
test_inc_errors_when_operand_is_{null,array,string,bool,integer} (top-level {$inc: <non-doc>} → code 9)	inc/test_inc_error_cases.py
test_inc_dotted_past_array_end_pads_with_nulls	inc/test_inc_field_paths.py
test_min_errors_when_operand_is_{null,array,string,bool,integer}	min/test_min_error_cases.py
test_min_dotted_past_array_end_pads_with_nulls	min/test_min_field_paths.py
test_currentDate_errors_when_operand_is_{null,array,string,bool,integer}	currentDate/test_currentDate_errors.py
test_currentDate_dotted_past_array_end_pads_with_nulls, test_currentDate_dotted_into_array_element_by_index_sets_field	currentDate/test_currentDate_field_paths.py
test_addToSet_binary_dedups_by_value (only if not already covered)	update/array/addToSet/test_addToSet_data_types.py

Add each as a new case in the target file's existing parametrize/dataclass list, using framework.error_codes constants (FAILED_TO_PARSE_ERROR, TYPE_MISMATCH_ERROR, IMMUTABLE_FIELD_ERROR, …) rather than raw 9/14/66 literals (TEST_FORMAT.md §4).

4. Drop — duplicates of existing coverage

Everything else re-asserts a property an existing sibling already owns. Examples: test_inc_int_plus_double_promotes_to_double = test_inc_type_conversion.py::int32_plus_double; test_inc_int64_max_overflow_errors = test_inc_error_cases.py::int64_max_plus_1_int64; test_accumulator_addToSet_dedups_null_values/_numerically_equal/_decimal128_trailing_zeros/_reordered_subdocument_keys = test_accumulator_addToSet_dedup.py (doc_with_null_value, Numeric Equivalence, doc_different_field_order); objectid/date = test_accumulator_addToSet_bson_types.py. The same pattern holds for min, update/array/addToSet, and currentDate. REVIEW.md flags "duplicate coverage with another folder" (Major). After extracting the §3 keepers, these files should be deleted.

5. Remove — engine_xfail(engine="pgmongo") markers

pgmongo is not a configured engine (only mongodb in dev/compose.yaml), so conftest.py never activates these — they're inert no-ops that pass against MongoDB and hard-fail against DocumentDB. REVIEW.md: "Don't recommend engine_xfail("documentdb") … engine xfails aren't handled in this repo." DocumentDB divergences are tracked in the DocumentDB repo. More broadly: if a test's goal is to catch bugs in the operator's implementation (vs. verifying spec-documented behavior), this compat suite isn't the right venue.

6. Format — parametrize + dataclass, not monolithic plain defs

The target files already use @dataclass(frozen=True) + pytest_params() (and bson_type_validator where applicable) with # Property [Name]: comments (TEST_FORMAT.md, REVIEW.md §4). Please add the kept cases as rows in those parametrized lists — the BSON-type dedup cases (objectid/date/binary/numeric-equivalence/decimal-trailing-zeros) as rows in the existing dedup/bson-type list, and the operand-type matrix (null/array/string/bool/int) as a single parametrized test — with a # Property [Name]: group comment.

[SarthakDalmia1]

Thanks for the coverage push. The behaviors themselves are reasonable, but as written these six *_behaviors.py files mostly duplicate coverage that already exists in each folder's aspect-split files (added earlier), a few tests are in the wrong folder, and the format diverges from the established convention. Rather than land new monolithic files, please fold the genuinely-new cases into the existing files and drop the rest. Details below.

1. Move — window-frame tests are $setWindowFields stage coverage, not $addToSet coverage

All 7 tests in window/addToSet/test_window_addToSet_behaviors.py (bounded_documents_window, range_based_window, partitionBy_resets_per_partition, self_only_window_yields_single_element, unbounded_window_collects_full_partition, errors_when_documents_and_unit_both_set, errors_when_no_sortBy_for_documents_window) exercise $setWindowFields frame semantics, not anything specific to $addToSet. This is a compatibility framework — frame semantics are assumed spec-consistent across all window operators, so we don't re-test them per operator.

Please move them to core/operator/stages/setWindowFields/ (e.g. test_setWindowFields_window_bounds.py), keeping $addToSet as a representative operator, with a module docstring making the intent explicit — e.g. "Validates $setWindowFields frame semantics using $addToSet as a representative window operator; not re-tested per operator, spec assumed consistent." window/addToSet/ then keeps only its existing test_smoke_window_addToSet.py (one smoke case per operator, per TEST_COVERAGE.md §18).

2. Move — findAndModify tests belong in the findAndModify folder

Per REVIEW.md "Feature ownership", test_inc_via_findAndModify_returns_updated_doc, test_min_via_findAndModify_returns_updated_doc, and test_currentDate_via_findAndModify_returns_updated_doc verify findAndModify's new:true / lastErrorObject behavior, not the operator's arithmetic. Move to core/query_and_write/commands/findAndModify/ as one representative context case, not one per operator.

3. Keep — genuinely new; insert into the existing aspect files

New test(s) Insert into
test_inc_errors_when_operand_is_{null,array,string,bool,integer} (top-level {$inc: <non-doc>} → code 9) inc/test_inc_error_cases.py
test_inc_dotted_past_array_end_pads_with_nulls inc/test_inc_field_paths.py
test_min_errors_when_operand_is_{null,array,string,bool,integer} min/test_min_error_cases.py
test_min_dotted_past_array_end_pads_with_nulls min/test_min_field_paths.py
test_currentDate_errors_when_operand_is_{null,array,string,bool,integer} currentDate/test_currentDate_errors.py
test_currentDate_dotted_past_array_end_pads_with_nulls, test_currentDate_dotted_into_array_element_by_index_sets_field currentDate/test_currentDate_field_paths.py
test_addToSet_binary_dedups_by_value (only if not already covered) update/array/addToSet/test_addToSet_data_types.py
Add each as a new case in the target file's existing parametrize/dataclass list, using framework.error_codes constants (FAILED_TO_PARSE_ERROR, TYPE_MISMATCH_ERROR, IMMUTABLE_FIELD_ERROR, …) rather than raw 9/14/66 literals (TEST_FORMAT.md §4).

4. Drop — duplicates of existing coverage

Everything else re-asserts a property an existing sibling already owns. Examples: test_inc_int_plus_double_promotes_to_double = test_inc_type_conversion.py::int32_plus_double; test_inc_int64_max_overflow_errors = test_inc_error_cases.py::int64_max_plus_1_int64; test_accumulator_addToSet_dedups_null_values/_numerically_equal/_decimal128_trailing_zeros/_reordered_subdocument_keys = test_accumulator_addToSet_dedup.py (doc_with_null_value, Numeric Equivalence, doc_different_field_order); objectid/date = test_accumulator_addToSet_bson_types.py. The same pattern holds for min, update/array/addToSet, and currentDate. REVIEW.md flags "duplicate coverage with another folder" (Major). After extracting the §3 keepers, these files should be deleted.

5. Remove — engine_xfail(engine="pgmongo") markers

pgmongo is not a configured engine (only mongodb in dev/compose.yaml), so conftest.py never activates these — they're inert no-ops that pass against MongoDB and hard-fail against DocumentDB. REVIEW.md: "Don't recommend engine_xfail("documentdb") … engine xfails aren't handled in this repo." DocumentDB divergences are tracked in the DocumentDB repo. More broadly: if a test's goal is to catch bugs in the operator's implementation (vs. verifying spec-documented behavior), this compat suite isn't the right venue.

6. Format — parametrize + dataclass, not monolithic plain defs

The target files already use @dataclass(frozen=True) + pytest_params() (and bson_type_validator where applicable) with # Property [Name]: comments (TEST_FORMAT.md, REVIEW.md §4). Please add the kept cases as rows in those parametrized lists — the BSON-type dedup cases (objectid/date/binary/numeric-equivalence/decimal-trailing-zeros) as rows in the existing dedup/bson-type list, and the operand-type matrix (null/array/string/bool/int) as a single parametrized test — with a # Property [Name]: group comment.

Thanks for the review @eerxuan . I have addressed the comments. Please take a look.

[Copilot]

Pull request overview

Adds additional compatibility test coverage for several update operators ($min, $inc, $currentDate) and expands coverage around $addToSet (including a new $setWindowFields window-bounds suite that uses $addToSet as the representative window operator). Also enables manual triggering of the Docker build workflow.

Changes:

• Add new field-path cases for $min, $inc, and $currentDate, including dotted paths that address array indices past the current array length.
• Add operand-type parse-failure coverage for $min, $inc, and $currentDate when the operator operand is not a document.
• Extend $addToSet type behavior coverage (Binary subtype 0) and add a new $setWindowFields window-bounds test suite.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
documentdb_tests/compatibility/tests/core/operator/update/fields/min/test_min_field_paths.py	Adds $min field-path coverage for dotted array indices beyond current array length.
documentdb_tests/compatibility/tests/core/operator/update/fields/min/test_min_error_cases.py	Adds $min operand-type parse-failure cases (non-document operands).
documentdb_tests/compatibility/tests/core/operator/update/fields/inc/test_inc_field_paths.py	Adds $inc field-path coverage for dotted array indices beyond current array length.
documentdb_tests/compatibility/tests/core/operator/update/fields/inc/test_inc_error_cases.py	Adds $inc operand-type parse-failure cases (non-document operands).
documentdb_tests/compatibility/tests/core/operator/update/fields/currentDate/test_currentDate_field_paths.py	Adds $currentDate dotted path coverage into array elements and past array end (structure-based assertions).
documentdb_tests/compatibility/tests/core/operator/update/fields/currentDate/test_currentDate_errors.py	Adds $currentDate operand-type parse-failure cases (non-document operands).
documentdb_tests/compatibility/tests/core/operator/update/array/addToSet/test_addToSet_data_types.py	Adds $addToSet dedup behavior coverage for Binary subtype 0.
documentdb_tests/compatibility/tests/core/operator/stages/setWindowFields/test_setWindowFields_window_bounds.py	Introduces a new $setWindowFields window-bounds test suite using $addToSet output.
.github/workflows/docker-build.yml	Adds workflow_dispatch so Docker builds can be triggered manually.