Skip to content

test(NODE-4251): sync csfle create spec tests #3271

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

Merged
merged 10 commits into from
Jun 10, 2022
Merged

test(NODE-4251): sync csfle create spec tests #3271

merged 10 commits into from
Jun 10, 2022

Conversation

durran
Copy link
Member

@durran durran commented May 25, 2022

Description

Adds list collections assertions in the create spec tests plus tests for create indexes and collmod.

What is changing?

Spec test sync. mongodb/specifications@f0d1330 and mongodb/specifications@2fd6b69

Is there new documentation needed for these changes?

None

What is the motivation for this change?

NODE-4251/DRIVERS-2309

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

@durran durran marked this pull request as ready for review May 25, 2022 17:25
@durran durran added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label May 27, 2022
@durran
Copy link
Member Author

durran commented May 27, 2022

This fixes the failing fle2 tests in evergreen for the events count mismatch - adding the listCollections expectations in the command started events was required with the latest fle changes. The 2 remaining failures are expected and not covered by this PR.

@dariakp dariakp self-requested a review May 31, 2022 20:52
addaleax
addaleax previously approved these changes Jun 1, 2022
Copy link
Contributor

@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.

There are a lot of failing tests now - which ones, exactly, are expected to fail and why?

@durran
Copy link
Member Author

durran commented Jun 1, 2022

There are a lot of failing tests now - which ones, exactly, are expected to fail and why?

All of them are expected to fail and be resolved in another PR or if we wait on https://jira.mongodb.org/browse/SERVER-66901

@dariakp
Copy link
Contributor

dariakp commented Jun 1, 2022

There are a lot of failing tests now - which ones, exactly, are expected to fail and why?

All of them are expected to fail and be resolved in another PR or if we wait on jira.mongodb.org/browse/SERVER-66901

Is that the reason for all 11 failures? The error messages look different, some of them expect an error but are currently passing, and your original comment only mentioned expected failures for two tests.

@durran
Copy link
Member Author

durran commented Jun 2, 2022

There are a lot of failing tests now - which ones, exactly, are expected to fail and why?

All of them are expected to fail and be resolved in another PR or if we wait on jira.mongodb.org/browse/SERVER-66901

Is that the reason for all 11 failures? The error messages look different, some of them expect an error but are currently passing, and your original comment only mentioned expected failures for two tests.

Actually the one error expecting an error is a legitimate bug - I'm looking into where we missed that validation.

@nbbeeken
Copy link
Contributor

nbbeeken commented Jun 3, 2022

I skipped the related we've been seeing failures in this PR: #3281, once it's in you can rebase and unskip them here, its isolated so easy to do git wise but at least other PRs can go green on 6.0

@durran
Copy link
Member Author

durran commented Jun 6, 2022

mongodb/libmongocrypt#364 merges and the int/long contention errors are fixed.

@durran durran force-pushed the fix-csfle-tests branch from 2937e0d to 0c9a58d Compare June 9, 2022 15:35
@durran durran requested review from dariakp and addaleax June 9, 2022 15:49
addaleax
addaleax previously approved these changes Jun 9, 2022
Copy link
Contributor

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

lgtm!

@dariakp dariakp added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Jun 10, 2022
@dariakp dariakp self-requested a review June 10, 2022 15:45
Copy link
Contributor

@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.

Waiting to update the fle dependency to merge so that all tests are passing

@dariakp dariakp merged commit 4121ec6 into main Jun 10, 2022
@dariakp dariakp deleted the fix-csfle-tests branch June 10, 2022 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team Review Needs review from team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants