-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add RewrapManyDataKeyProseTest #1083
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of minor naming changes, to follow current conventions.
Other than that LGTM
Note: I've added JAVA-4879 to track improving the discoverability of prose tests and to define a clearer naming convention / grouping. As I'm not sure all prose tests have been implemented.
import static com.mongodb.client.Fixture.getMongoClientSettingsBuilder; | ||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
||
public abstract class AbstractRewrapManyDataKeyProseTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to AbstractClientEncryptionRewrapManyDataKeyProseTest
to aid future discovery
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many tests are named AbstractClientSideEncryption...
- is this just a minor inconsistency, or is there some difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the weirdness that I hope JAVA-4879 will clean up.
Probably a mix of inconsistent naming and grouping for the CSFLE Prose tests - its not clear which ones map to which prose tests.
import static com.mongodb.client.Fixture.getMongoClient; | ||
import static com.mongodb.client.Fixture.getMongoClientSettingsBuilder; | ||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets add a comment to the prose test in the spec
import com.mongodb.client.vault.ClientEncryptions; | ||
import com.mongodb.reactivestreams.client.syncadapter.SyncMongoClient; | ||
|
||
public class RewrapManyDataKeyProseTest extends AbstractRewrapManyDataKeyProseTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to ClientEncryptionRewrapManyDataKeyProseTest
to aid future discovery
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
JAVA-4698