-
Notifications
You must be signed in to change notification settings - Fork 20
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
test: add support for legacy test vectors #695
Conversation
@@ -151,6 +151,21 @@ module {:options "-functionSyntax:4"} EsdkTestVectors { | |||
description: string, | |||
decryptionMethod: DecryptionMethod | |||
) | |||
| PositiveV1DecryptTestVector( |
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.
Where does "V1" come from? What is "V1" referring to?
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.
V1 is the manifest version
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.
It might be helpful to call it PositiveV1ManifestDecryptTestVector
but I won't block on that since you can find it from other context
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.
PositiveV1OrV2ManifestDecryptTestVector
var keyDescriptions :- GetKeyDescriptions(masterKeys, keys); | ||
var keyDescription :- ToMultiKeyDescription(keyDescriptions); | ||
|
||
Success(PositiveV1DecryptTestVector( |
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 test is V2 but it uses V1 vectors? It's probably reasonable but an explanation would be very helpful
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.
I was planning on having a PositiveV2DecryptTestVector but figured it was not needed since the positive case is the same for both versions
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.
I could name it PositiveLegacyDecryptTestVector?
thoughts?
Issue #, if available:
Description of changes:
Squash/merge commit message, if applicable:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.