Add $facet tests#669
Conversation
Signed-off-by: PatersonProjects <keldonhoff@gmail.com>
Signed-off-by: PatersonProjects <keldonhoff@gmail.com>
This reverts commit 014f2bc. Signed-off-by: PatersonProjects <keldonhoff@gmail.com>
This reverts commit 51a482a. Signed-off-by: PatersonProjects <keldonhoff@gmail.com>
…-tests Signed-off-by: PatersonProjects <keldonhoff@gmail.com>
…-tests Signed-off-by: PatersonProjects <keldonhoff@gmail.com>
Signed-off-by: PatersonProjects <keldonhoff@gmail.com>
Signed-off-by: PatersonProjects <keldonhoff@gmail.com>
|
🤖 Auto-triaged by documentdb-triage-tool. Applied: Reasoningcomponent from path globs (test-coverage, test-framework); effort from diff stats (1755+0 LOC, 12 files); LLM: Adds new compatibility tests for the $facet aggregation stage operator, expanding test coverage for an existing feature. If a label is wrong, remove it manually and ping |
|
|
||
|
|
||
| @pytest.mark.aggregate | ||
| def test_facet_subpipeline_lookup_basic(collection): |
There was a problem hiding this comment.
The $lookup test only covers the equi-join form (localField/foreignField). Missing: $lookup with pipeline form inside $facet — this tests nested sub-pipelines ($facet → $lookup → pipeline → [stages]). Consider adding:
- Success tests with pipeline-form
$lookupcontaining multiple stages (e.g.,$match,$project,$sort,$group,$unwind,$addFields, etc.) - Error tests with all
$facet-forbidden stages ($out,$merge,$collStats,$indexStats,$planCacheStats,$geoNear,$facet) inside the$lookuppipeline to verify they're still rejected when nested inside$facet
There was a problem hiding this comment.
Many success/behavior tests (across the whole PR) use a single sub-pipeline. $facet is designed for multiple sub-pipelines on the same input — single-sub-pipeline is a special/minimal case. Consider adding a second sub-pipeline (e.g., "first": [{"$limit": 1}]) to the success cases in test_facet_subpipeline_stages.py, test_stages_position_facet.py, and test_facet_collation.py to make them more realistic.
|
|
||
|
|
||
| @pytest.mark.aggregate | ||
| @pytest.mark.parametrize("test_case", pytest_params(FACET_SUBPIPELINE_STAGE_TESTS)) |
There was a problem hiding this comment.
Per the container-feature rule (one test per sub-feature), these stages are valid inside $facet but not tested: $redact, $sample, $unionWith, $setWindowFields. Each should have one smoke case confirming it works inside a $facet sub-pipeline.
|
|
||
| # Property [Valid Edge Cases]: empty sub-pipelines, many sub-pipelines, and | ||
| # unusual-but-valid output field names are accepted. | ||
| FACET_ARGUMENT_SUCCESS_TESTS: list[StageTestCase] = [ |
There was a problem hiding this comment.
Missing field name edge cases that should be verified (as either accepted or rejected): spaces ("my field"), hyphens ("my-field"), underscores ("my_field"), numeric-only ("123"), at-sign ("@field"), null byte ("a\x00b"). These should be tested to document the actual behavior — whether they succeed or error.
| msg="$facet should yield an empty array when $skip exceeds the document count", | ||
| ), | ||
| StageTestCase( | ||
| id="allowDiskUse_true_operates_normally", |
There was a problem hiding this comment.
Missing the allowDiskUse: false case. Both true and false should be tested to confirm $facet operates normally regardless of the flag value.
| # Property [Sub-Pipeline Independence]: a $limit, $match, or $addFields in one | ||
| # sub-pipeline does not affect the input seen by another; a runtime error in any | ||
| # sub-pipeline fails the whole stage. | ||
| FACET_INDEPENDENCE_TESTS: list[StageTestCase] = [ |
There was a problem hiding this comment.
Please add two tests:
$samplebefore$facet— two sub-pipelines that each return all documents and verify they see exactly the same data$addFieldswith$randbefore$facet— two sub-pipelines that each return all documents and verify they see exactly the same$randvalues
This pr adds tests for the $facet stage operator
Ref: Issue #47, #485