-
Notifications
You must be signed in to change notification settings - Fork 52
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 test to ensure issuanceDate is added to VC 1.1 VCs. #166
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.
Just some minor nits.
test/10-verify.spec.js
Outdated
const jsonDate = now.toJSON(); | ||
const expectedIssuanceDate = `${jsonDate.slice(0, jsonDate.length - 5)}Z`; |
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.
Just another way of doing it, but one less line.
const jsonDate = now.toJSON(); | |
const expectedIssuanceDate = `${jsonDate.slice(0, jsonDate.length - 5)}Z`; | |
const expectedIssuanceDate = now.toISOString().slice(0, -5) + 'Z'; |
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 like the now.toISOString()
approach better, but we should keep the string interpolation in place.
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.
@JSAssassin @BigBlueHat I like the toISOString
method more too, I was copying the way the issuanceDate is set inside of index.js
should I update that to? test updated here: b34181d
p.s. I could update the one in index.js
to use toISOString.slice(0, -5)
in the validUntil
PR as that already contains breaking changes to the DateRegexp.
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.
Agree with @JSAssassin. Those small tweaks would be nice.
…vcs in test titles. Co-authored-by: Tashi D. Gyeltshen <[email protected]>
No description provided.