Skip to content

Conversation

abr-egn
Copy link
Contributor

@abr-egn abr-egn commented Jan 25, 2022

RUST-523

This pulls in the unified spec test for change streams and adds support to the Rust unified runner for creating and iterating change streams. I also added more detailed mismatch reporting to the runner as part of debugging initial failures.

Copy link
Contributor

@patrickfreed patrickfreed left a comment

Choose a reason for hiding this comment

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

Overall looks good! I like the added detail to the matching. Only have a few minor questions/suggestions

@abr-egn abr-egn requested a review from patrickfreed January 27, 2022 14:22
Copy link
Contributor

@isabelatkinson isabelatkinson left a comment

Choose a reason for hiding this comment

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

looks good! nice to see some upgrades to the matcher

observe_sensitive_commands: bool,
}

impl ClientEntity {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this method is necessary -- ClientEntity implements Deref with a target type of Client so you should be able to call watch directly on a ClientEntity (where this method is used).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I missed that, thank you! Removed.

Copy link
Contributor

@patrickfreed patrickfreed left a comment

Choose a reason for hiding this comment

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

LGTM mod Isabel's comment

Copy link
Contributor

@isabelatkinson isabelatkinson left a comment

Choose a reason for hiding this comment

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

lgtm! looks like there's something going on with evergreen but i don't think we need another full CI run here

@abr-egn abr-egn merged commit 724a227 into mongodb:master Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants