DRIVERS-2344: add disambiguatedPaths to change streams update document #1292
Conversation
baileympearson
requested review from
benjirewis
and removed request for
a team
|
Link to the passing unified tests in the Node POC evg run: https://evergreen.mongodb.com/task_log_raw/mongo_node_driver_next_ubuntu_18.04_erbium_test_latest_replica_set_patch_f28ba0de3bc925267a185965eb971ecf91bc2680_62fd236be3c3313be42d354a_22_08_17_17_20_44/0?type=T#L838 |
durran
left a comment
durran
left a comment
There was a problem hiding this comment.
Just noting from my conversation with @baileympearson . I thought it might be nice to add a test where the document in the db would have a key similar to "a.b" in it but we felt it was testing more server correctness than driver correctness and thus not needed.
LGTM
abr-egn
left a comment
abr-egn
left a comment
There was a problem hiding this comment.
LGTM! Passes in the Rust driver.
benjirewis
left a comment
benjirewis
left a comment
There was a problem hiding this comment.
Excellent description of the feature in the spec, thanks 🧑🔧 . I agree that testing a.b seems redundant to the point of only testing server correctness, @durran.
source/change-streams/tests/unified/change-streams-disambiguatedPaths.yml
Outdated
Show resolved
Hide resolved
|
|
||
| runOnRequirements: | ||
| - minServerVersion: "6.1.0" | ||
| topologies: [ replicaset, sharded-replicaset, load-balanced ] |
There was a problem hiding this comment.
Do these new tests pass on sharded clusters + serverless? I would hope so, but we've had issues in the past with change streams tests on those topologies.
There was a problem hiding this comment.
They pass on sharded replica sets - https://evergreen.mongodb.com/task_log_raw/mongo_node_driver_next_ubuntu_18.04_erbium_test_latest_sharded_cluster_patch_f28ba0de3bc925267a185965eb971ecf91bc2680_62fd236be3c3313be42d354a_22_08_17_17_20_44/0?type=T#L835
They pass on load balanced - https://evergreen.mongodb.com/task_log_raw/mongo_node_driver_next_ubuntu_18.04_erbium_test_latest_load_balanced_patch_f28ba0de3bc925267a185965eb971ecf91bc2680_62fd236be3c3313be42d354a_22_08_17_17_20_44/0?type=T#L751
They don't run on serverless - https://github.com/mongodb/specifications/tree/master/source/serverless-testing#other-tests
Sharded is odd because sharded clusters must be backed by replica sets as of 3.6 (https://www.mongodb.com/docs/manual/core/sharded-cluster-components/#sharded-cluster-components). So effectively they're the same now, I can add it as well to the list of topologies.
There was a problem hiding this comment.
They don't run on serverless
The spec says:
Unified spec tests from all specifications MUST be run against Atlas Serverless.
Thanks for adding sharded; it's sort of a strange topology, but might as well add.
- fix double space typos - lower schema veresion to 1.3 instead of 1.9 - add `sharded` to list of supported topoligies - remove ? in type definition in favor of the `Optional` keyword
baileympearson
left a comment
baileympearson
left a comment
There was a problem hiding this comment.
responding to comments
| - name: insertOne | ||
| object: *collection0 | ||
| arguments: | ||
| document: { _id: 1, 'a': { '1': 1 } } |
There was a problem hiding this comment.
[opt] You could use " to be consistent with the other spec tests.
https://jira.mongodb.org/browse/DRIVERS-2344
POC PR in Node: mongodb/node-mongodb-native#3365
A new field,
disambiguatedPathshas been added to change stream update documents for servers >=6.1 whereshowExpandedEventsis enabled.This feature is only supported in server 6.1 and above, so testing these changes will require running against the latest server build.