Skip to content

Add validate tests#639

Open
alinaliBQ wants to merge 15 commits into
documentdb:mainfrom
alinaliBQ:validate
Open

Add validate tests#639
alinaliBQ wants to merge 15 commits into
documentdb:mainfrom
alinaliBQ:validate

Conversation

@alinaliBQ

Copy link
Copy Markdown
Contributor

Add command operator tests for validate. Tests database validate behavior, output collection, syntax, and errors.
Adds setup field in DiagnosticTestCase.

@alinaliBQ alinaliBQ marked this pull request as ready for review June 23, 2026 18:13
@alinaliBQ alinaliBQ requested a review from a team as a code owner June 23, 2026 18:13
@documentdb-triage-tool documentdb-triage-tool Bot added compatibility test Compatibility test related enhancement New feature or request labels Jun 23, 2026
@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.82 (mixed)

Reasoning

component from path globs (test-coverage, test-framework); effort from diff stats (2087+1 LOC, 12 files); LLM: Adds new validate command tests and extends DiagnosticTestCase with a setup field, expanding test coverage for a specific MongoDB command.

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.

from documentdb_tests.framework.property_checks import Eq

# Property [Type Coercion]: validate accepts all BSON types for the full parameter via coercion.
ACCEPTED_TYPE_TESTS: list[DiagnosticTestCase] = [

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.

🟠 [Major] The per-parameter coercion files are enormous and partly redundant against §20's canonical matrix. Five files (full, metadata, checkBSONConformance, repair, fixMultikey) each test the same ~21-type coercion matrix (~105 near-identical cases). Per TEST_COVERAGE.md §20, boolean-param coercion should mirror the canonical matrix (bypassDocumentValidation), but it
doesn't require re-testing all 21 BSON types five times for one command — the coercion logic is shared across these boolean params. Consider collapsing to one representative coercion matrix plus per-param confirmation that each is wired to it (a few cases each). As written this is a lot of duplicated surface for one behavior; TEST_FORMAT.md "don't ask for exhaustive
BSON matrix on simple shared params" cuts toward trimming.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed the exhaustive tests

def test_validate_response_details(collection, test):
"""Test validate response values, sub-structure, and option-specific shapes."""
for cmd in test.setup:
execute_command(collection, {**cmd, next(iter(cmd)): collection.name})

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.

[Question] setup runner uses next(iter(cmd)) to rewrite the collection name. The pattern {**cmd, next(iter(cmd)): collection.name} assumes the command name is the first key in each setup dict. That holds for the literal dicts here ({"insert": "", ...}), but it's a slightly fragile idiom — if a dict were ever written with another key first, it'd rewrite the wrong
field silently. A small explicit helper or a command_name field would be more robust. Minor.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Switched to use helper bind_collection

alinaliBQ added 12 commits June 25, 2026 11:31
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>

@alinaliBQ alinaliBQ left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed comments. also added validate_repair capability to skip on validate tests that only work on standalone instances

from documentdb_tests.framework.property_checks import Eq

# Property [Type Coercion]: validate accepts all BSON types for the full parameter via coercion.
ACCEPTED_TYPE_TESTS: list[DiagnosticTestCase] = [

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed the exhaustive tests

def test_validate_response_details(collection, test):
"""Test validate response values, sub-structure, and option-specific shapes."""
for cmd in test.setup:
execute_command(collection, {**cmd, next(iter(cmd)): collection.name})

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Switched to use helper bind_collection

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.

2 participants