Skip to content

Add count aggregation command tests#191

Open
danielfrankcom wants to merge 2 commits into
documentdb:mainfrom
danielfrankcom:pr/count
Open

Add count aggregation command tests#191
danielfrankcom wants to merge 2 commits into
documentdb:mainfrom
danielfrankcom:pr/count

Conversation

@danielfrankcom
Copy link
Copy Markdown
Collaborator

This change adds tests for the count aggregation command.

@danielfrankcom danielfrankcom requested a review from a team as a code owner May 15, 2026 22:08
@documentdb-triage-tool documentdb-triage-tool Bot added compatibility test Compatibility test related enhancement New feature or request labels May 15, 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.85 (mixed)

Reasoning

component from path globs (test-coverage, test-framework); effort from diff stats (4446+43 LOC, 20 files); LLM: Adds new test coverage for the count aggregation command, expanding the compatibility test suite.

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.

@danielfrankcom danielfrankcom changed the title Add count aggregation command tests Add count aggregation command tests May 19, 2026
Copy link
Copy Markdown
Collaborator

@eerxuan eerxuan left a comment

Choose a reason for hiding this comment

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

I feel "namespace", "maxTimeMS", "comment" and "read_concern" are cross cutting feature outside of the individual command layer. We may just need one test for each in every command folder. The full coverage should be in there own folder. We have query-and-write/read-concern/. I feel "comment" and "maxTimeMS" can belong to /cursor/. As mongosh has helper cursor.comment() and cursor.maxTimeMs(). "namespace" can be under /collections/.
I should have notice this earlier, we can reduce duplication start from new PRs and have a backlog to remove existing duplication later.

Remove the duplication for collation and view in this PR.

Copy link
Copy Markdown
Collaborator

@eerxuan eerxuan left a comment

Choose a reason for hiding this comment

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

Can you add a rule in TEST_COVERAGE.md specify these command arguments are cross cutting features and where they belongs to.

@eerxuan
Copy link
Copy Markdown
Collaborator

eerxuan commented May 26, 2026

I feel "namespace", "maxTimeMS", "comment" and "read_concern" are cross cutting feature outside of the individual command layer. We may just need one test for each in every command folder. The full coverage should be in there own folder. We have query-and-write/read-concern/. I feel "comment" and "maxTimeMS" can belong to /cursor/. As mongosh has helper cursor.comment() and cursor.maxTimeMs(). "namespace" can be under /collections/. I should have notice this earlier, we can reduce duplication start from new PRs and have a backlog to remove existing duplication later.

Remove the duplication for collation and view in this PR.

Discussed offline.

  • We should remove view, just keep 1 or 2 smoke test here. LIMIT_NOT_POSITIVE error need to verify.
  • "collation" and "namespace" only need to keep type_.py, which test each bson type as input + a valid case.
  • "comment" and "maxTimeMS" are good as it is now, no need to remove.
  • "read_concern" is good as it is now, which covers syntax cases. The semantic cases will be covered under query-and-write/read-concern/. Same for write_concern. Don't need to test options not mentioned in specs.

Remove collation semantic/subfield tests (covered in collation/),
namespace naming rules (common spec), and thorough view tests (merged
into collection type tests). Add collection type acceptance tests
including the view limit=0 quirk.
Signed-off-by: Daniel Frankcom <[email protected]>
@danielfrankcom
Copy link
Copy Markdown
Collaborator Author

Can you add a rule in TEST_COVERAGE.md specify these command arguments are cross cutting features and where they belongs to.

Done in #246

@danielfrankcom danielfrankcom requested a review from eerxuan May 27, 2026 21:26
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