Repository navigation
Add dropIndexes change stream compatibility tests - #726
Conversation
eerxuan
left a comment
There was a problem hiding this comment.
This is the first full-suite change stream test. Please add a coverage rule to docs/testing/TEST_COVERAGE.md. Here is a draft. Please review and adjust.
Draft rule
### 23. Change Stream Event Coverage
**Rule**: Test each change stream event type on seven axes. For each event, list the cases for each axis. Use the event reference page and the reference page of the triggering command. Each case in a list is one cell. Coverage is complete when each cell is tested or marked not applicable. Write the reason for each not-applicable cell in the docstring of the test file that covers that axis. Put the tests in `tests/changeStreams/$eventName/`. The test inputs are the triggering command, the collection type, and the `$changeStream` options. §1 (BSON data types) applies only to events that carry user documents. It does not apply to events that carry only metadata. Use the behavior of the reference server as the expected value.
**1. Emission.** List each form of the triggering command that the reference page of the command shows. Test each form. Assert the exact number of events. Do not assert the order of events from one command.
**2. Non-emission.** Test each case in which the event must not appear.
- The command fails.
- The command succeeds but changes nothing.
- The server makes the change as a side effect of another operation. Another event type can report it.
- A stream on a different namespace must not receive the event.
**3. Gating options.** Test each option that controls whether the event appears or which fields it has. Examples are `showExpandedEvents` and `showSystemEvents`. Test the values `true`, `false`, and omitted. The command must have the same effect on the data in all three cases. If the event has no gating option, mark the cell not applicable.
**4. Event shape.** Check that each field on the event reference page is present and has the correct type. Check that each field the event must not have is absent. Check the value of a field when the page defines a rule for it. Fields for transactions (`lsid`, `txnNumber`) are present only inside a transaction. Read dates with a timezone-aware codec.
**5. Payload fidelity.** Some data in an event comes from another feature. Compare that data to the output of the other feature. Test one case for each distinct representation. Take the list of representations from the test folder of the other feature. Do not test each option of the other feature (§19). If the command changes the data, read the output of the other feature before the command runs.
**6. Scope and resumption.**
- Test each scope that can receive the event: collection, database, and cluster. The shape of the event must be the same in all scopes.
- Resume with `resumeAfter` and the token of the event. The stream must return only later events. Do the same for `startAfter` and `startAtOperationTime`.
- If one command produces many events, the tokens must be different. Resume from the token of a middle event. The stream must replay only the events after it.
**7. Collection variants.** The collection type is an input to the event. Test one case for each supported type. This follows the rule "Collection type as input" in `FOLDER_STRUCTURE.md`. The types are regular, capped, view, and time-series. Do not test the semantics of the type itself. Examples are view pipelines, capped-collection eviction, and time-series bucketing. For each type, record whether the event is produced and what the event contains. Mark the cell not applicable if the event cannot occur on that type or the suite does not cover it.
**Tested once, elsewhere (§19)**: test these items once, in the shared change stream tests.
- The format of the resume token.
- The lifecycle of the cursor and of `getMore`.
- The behavior of `invalidate`.
- The `fullDocument*` options.
- The validation of `$changeStream` options.
- The rejection of a stream opened on a view.
- The stages that follow `$changeStream`.
Test each event in its own folder. Do not test one event in the folder of another event. Do not repeat the shared items in an event folder.
**Out of scope**:
- Oplog rewrite and `$match` pushdown. These are not observable.
- Engine-specific details, such as storage engine options.
- Axes that need a topology that the suite does not run. Mark these cells not applicable.
- Transaction fields for an event that cannot occur in a transaction. Mark these cells not applicable.
**Helpers**: put the helpers in `tests/changeStreams/utils/`. All events share them. The helpers open and close a stream, read events up to a marker document, and close the cursor.
**Applies to**: all values of `operationType` on the MongoDB Change Events page.Please also add this line to the Test Category Checklist at the end of the file:
- [ ] **Change stream event**: emission, non-emission, gating options, event shape, payload fidelity, scope and resumption, collection variants (if change stream event)Missing cases in this PR, based on the rule
- Emission
indexgiven as a key pattern, for example{a: 1}.indexgiven as an array of names, with one event per name.
- Non-emission
"*"on a collection with only_idemits nothing. Record what the reference server does.- Dropping the
_idindex fails and emits nothing. Record what the reference server does. - A collection stream on a different collection receives nothing.
- Gating
showExpandedEventsomitted emits nothing, and the index is still dropped.
- Shape
collectionUUIDequals the UUID fromlistCollections.wallTimeis a date._idis a document andclusterTimeis a Timestamp. The PR only checks that they exist.documentKey,fullDocument,lsidandtxnNumberare absent.
- Payload fidelity. Take the index families from the index tests under
tests/core/indexes/. The PR does not yet cover these:- A plain single-field index.
- A hashed index.
- A 2d index.
hidden: true.- A wildcard index with
wildcardProjection.
- Scope and resumption
startAfter.
- Collection variants. Record what the reference server does for each type.
- Capped collection.
- View.
- Time-series collection.
- Rule conformance
- Move the helpers from
dropIndexes/utils/totests/changeStreams/utils/. - Write the reason for each not-applicable cell in the docstrings, for example transaction fields and sharded topology.
- Move the helpers from
121c063 to
4aec782
Compare
eerxuan
left a comment
There was a problem hiding this comment.
Thanks for the thorough rework — this addresses all eight missing-case points from the prior round and lands the Rule 23 coverage entry. No blocking issues. A few minor/nitpick items inline; none need to hold the PR.
Coverage vs. the prior round — all delivered
- Emission: key pattern + array-of-names ✅
- Non-emission:
*with only_id,_id_failure, other-collection stream ✅ (plus missing-index, collection-drop, database-drop) - Gating:
showExpandedEventsomitted and false ✅ - Shape:
collectionUUIDvslistCollections,wallTimedate,_id/clusterTimetypes, absentdocumentKey/fullDocument/lsid/txnNumber✅ - Payload fidelity: single-field, hashed, 2d, hidden, wildcardProjection ✅
- Resumption:
startAfteradded ✅ - Collection variants: capped, view, time-series ✅
- Helpers moved to
changeStreams/utils/, NA reasons in docstrings ✅
Positive
collect_until_marker(insert a sentinel, draingetMoreuntil it appears) is a clean, deterministic bound on event collection with no sleeps, and it keeps order-insensitive assertions so it honors the rule's own "don't assert event order from one command."- The shape test is exactly right:
collectionUUIDcompared against the livelistCollectionsUUID, andwallTimechecked for both BSONdatetype and timezone-awareness. - Error codes come from
framework.error_codes(INVALID_OPTIONS_ERROR,COMMAND_NOT_SUPPORTED_ON_VIEW_ERROR), not raw integers.
Signed-off-by: sathvik vallivedu <tvsathvi@amazon.com>
4aec782 to
68eb42c
Compare
eerxuan
left a comment
There was a problem hiding this comment.
Verified 68eb42ca against the three items from the last round — all addressed:
- Resume tests now use
assertProperties(summary, {…: Eq(…)}, raw_res=True)instead ofassertSuccesson a synthetic dict. ✅ - Collection variants now use the
CappedCollection/ViewCollection/TimeseriesCollectionTargetCollectionsubclasses via.resolve(...)instead of hand-rolledcreate_collection/command("create", ...). kwargs (size,options,timeseries_options) match the framework definitions. ✅ # Property [Index specification]: ...comment added above the parametrized case list. ✅
Coverage (all eight prior points) and the §23 rule entry are unchanged and complete. No further comments — LGTM.
Adds compatibility tests for
dropIndexeschange-stream event behavior.Coverage includes:
dropIndexes("*").resumeAfterandstartAtOperationTime.