-
Notifications
You must be signed in to change notification settings - Fork 3
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
Issuer test cases for status entry and status credential #37
Conversation
Signed-off-by: Patrick <patrick.st-louis@opsecid.ca>
Signed-off-by: Patrick <patrick.st-louis@opsecid.ca>
Signed-off-by: Patrick <patrick.st-louis@opsecid.ca>
Signed-off-by: Patrick <patrick.st-louis@opsecid.ca>
Signed-off-by: Patrick <patrick.st-louis@opsecid.ca>
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.
unless I'm not getting something you got confused on what are should chai assertions and basic javascript API calls, but that's not that hard of a mistake to make.
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 a plural/singular fix
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: BigBlueHat <byoung@bigbluehat.com>
Co-authored-by: BigBlueHat <byoung@bigbluehat.com>
Co-authored-by: BigBlueHat <byoung@bigbluehat.com>
Signed-off-by: Patrick <patrick.st-louis@opsecid.ca>
Signed-off-by: Patrick <patrick.st-louis@opsecid.ca>
Whenever you have time, all comments should have been addressed |
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 am approving just so we get code into main. There are things to be done here, but I think this is a really solid start to status list.
Signed-off-by: Patrick <patrick.st-louis@opsecid.ca>
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.
More tweaks, but I fear if they're not addressed now they'll be painful to (re)find later.
Thanks!
🎩
testSlCredential({slCredential: slc}); | ||
}); | ||
it('MUST be 16kb in size when "encodedList" is decoded', | ||
should.exist(decoded, |
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.
.exist
is a property, not a method, in Chai (afaik):
https://www.chaijs.com/api/bdd/#exist
Additionally, it seems there's a good bit more to test here with more specificity.
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'm assuming that if the sl.decodeList({encodedList})
returns a response, the encoding was done properly since that decodeList
function will only work if encoded properly.
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'm not sure what action you want me to take with the exist statement as it serves the intended purpose, to make sure the value exists
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.
In the should extras, it says that by assigning should to a variable you have access to some quick helpers, including exist:
https://www.chaijs.com/guide/styles/#should
Co-authored-by: BigBlueHat <byoung@bigbluehat.com>
Co-authored-by: BigBlueHat <byoung@bigbluehat.com>
Signed-off-by: Patrick <patrick.st-louis@opsecid.ca>
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.
Small, editorial.
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
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.
Approving again to allow initial batch of tests to main.
This PR adds tests for normative statements around the status entry object and status credential object.
The
message
feature andstatusSize
feature are not implemented currently.