Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(NODE-3719,NODE-3883): identify causal consistency spec tests #3112

Merged
merged 4 commits into from
Jan 21, 2022

Conversation

dariakp
Copy link
Contributor

@dariakp dariakp commented Jan 20, 2022

Description

NODE-3719, NODE-3883

What is changing?

Identifying implemented and missing causal consistency spec tests; spec also says we should run on sharded topologies, not just replica set, so I expanded the required topology list.

Is there new documentation needed for these changes?

N/A

What is the motivation for this change?

Spec compliance audit

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: <type>(NODE-xxxx)<!>: <description>
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

Copy link
Contributor Author

@dariakp dariakp left a comment

Choose a reason for hiding this comment

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

Open question

@dariakp dariakp marked this pull request as ready for review January 21, 2022 14:16
@dariakp dariakp requested review from kmahar and removed request for kmahar January 21, 2022 15:46
@dariakp dariakp force-pushed the NODE-3719/spec-compliance-causal-consistency-tests branch from 888fda8 to 36bfe7f Compare January 21, 2022 15:53
@dariakp dariakp changed the title chore(NODE-3719): identify causal consistency spec tests chore(NODE-3719,NODE-3883): identify causal consistency spec tests Jan 21, 2022
@nbbeeken nbbeeken added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Jan 21, 2022
@nbbeeken nbbeeken self-requested a review January 21, 2022 16:25
Copy link
Contributor

@nbbeeken nbbeeken 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 we captured everything. One Q that we could add to the follow up ticket

{
metadata: {
requires: { topology: ['replicaset', 'sharded'] },
// Skipping session leak tests b/c these are explicit sessions
Copy link
Contributor

Choose a reason for hiding this comment

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

This is interesting, I would think this test still clean up the sessions it creates

@nbbeeken nbbeeken requested review from durran and baileympearson and removed request for durran and baileympearson January 21, 2022 16:38
@nbbeeken nbbeeken merged commit 2229d1a into main Jan 21, 2022
@nbbeeken nbbeeken deleted the NODE-3719/spec-compliance-causal-consistency-tests branch January 21, 2022 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Primary Review In Review with primary reviewer, not yet ready for team's eyes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants