-
Notifications
You must be signed in to change notification settings - Fork 63
test: Condition tests for caching-materials-manager-browser #177
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: Condition tests for caching-materials-manager-browser #177
Conversation
@@ -59,4 +59,28 @@ describe('WebCryptoCachingMaterialsManager', () => { | |||
expect(test._maxBytesEncrypted).to.equal(maxBytesEncrypted) | |||
expect(test._maxMessagesEncrypted).to.equal(maxMessagesEncrypted) | |||
}) | |||
|
|||
it('Precondition: A partition value must exist for WebCryptoCachingMaterialsManager.', () => { |
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.
Does this test also need to be added to caching_materials_manager_node.test.ts ?
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.
Yes, and it is in a different PR
/* Binary data is being transformed to utf-8. | ||
* This means it is highly likely to be less than 64 characters long. | ||
*/ | ||
expect(test._partition).to.be.a('string').and.to.have.length.greaterThan(50) |
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.
What is the likelihood that 64 bytes of random data -> UTF-8 will be less than 50 characters? Enough that we will "never" see this test fail?
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.
Sigh... I "fixed" this in https://github.com/awslabs/aws-encryption-sdk-javascript/pull/178/files#diff-2e3dd129c30c2285d72b61362efd41b9R58
Once that is merged. I'll update the test... utf8 on random data is.... sub optimal.
maxAge | ||
}) | ||
/* Binary data is being transformed to utf-8. | ||
* This means it is highly likely to be less than 64 characters long. |
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 comment is no longer needed
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: