Skip to content

Conversation

@nbbeeken
Copy link
Contributor

@nbbeeken nbbeeken commented Feb 17, 2023

Description

What is changing?

  • Moves prose test 14 to new file in Typescript
  • Changes try/catch to our .catch(error=>error) pattern
  • Add WC majority to ensure writes can be read

What is the motivation for this change?

Easier to manager modularized testing

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
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@nbbeeken nbbeeken added the wip label Feb 17, 2023
@nbbeeken nbbeeken force-pushed the NODE-4646-csfle-prose14 branch from 8f2b3df to 91f1074 Compare February 21, 2023 14:31
@nbbeeken nbbeeken changed the title test(NODE-4646): fix flakey csfle prose 14 test test(NODE-5069): move FLE prose test 14 to TS file Feb 21, 2023
@nbbeeken nbbeeken marked this pull request as ready for review February 21, 2023 14:33
@nbbeeken nbbeeken removed the wip label Feb 21, 2023
@nbbeeken nbbeeken force-pushed the NODE-4646-csfle-prose14 branch from 91f1074 to 823a726 Compare February 22, 2023 18:40
@nbbeeken nbbeeken changed the title test(NODE-5069): move FLE prose test 14 to TS file test(NODE-5069): move and improve FLE prose 14 test Feb 22, 2023
@@ -0,0 +1,270 @@
import { expect } from 'chai';
Copy link
Contributor

Choose a reason for hiding this comment

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

file name "decrytion"

Comment on lines 157 to 158
expect(error).property('code', 123);
expect(aggregateFailed).nested.property('failure.code', 123);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
expect(error).property('code', 123);
expect(aggregateFailed).nested.property('failure.code', 123);
expect(error).to.have.property('code', 123);
expect(aggregateFailed).to.have.nested.property('failure.code', 123);

minor but we generally try for natural phrasing in our assertions

Copy link
Contributor

Choose a reason for hiding this comment

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

(there are a number of assertions like this in this file that are new)

@baileympearson baileympearson merged commit a45e0e1 into main Feb 27, 2023
@baileympearson baileympearson deleted the NODE-4646-csfle-prose14 branch February 27, 2023 18:38
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