Conversation
|
Thanks for the PR! I'll review it this week, and hopefully merge it at the end! |
|
Yes the = sign instead of : was purposely done to get a server-crash 500 status code for an invalid JSON format. Tests get passed but we get to see the 400 status code in the console. |
gamemaker1
left a comment
gamemaker1
left a comment
There was a problem hiding this comment.
The changes so far are great! I have left some comments below, please take a look at them. Once you resolve those, I think this PR is good to go!
Before you start implementing the suggestions in the comments, could you please do the following:
git fetch upstream
git cherry-pick 0d5500ec423db033ad9de9a89e80c0c7bead1a44
I have separated the unit test step in GitHub actions and made a commit on a different branch, the above commands will add that commit to your PR, so that it remains up to date. Let me know if you face any problems while running the command.
| t.regex(error?.message, /An unexpected error occurred/) | ||
| // Check that only the `error` and `meta` fields were returned. | ||
| t.is(data, undefined) | ||
| }) |
There was a problem hiding this comment.
All the test cases you have added so far are great!
Just a few more that you could add:
- The ID of the DID returned when a valid request is passed is the first 16 letters of the hash of the email ID. Could you add a test case to make sure that is the case everytime?
- If you try to make a new DID with the same email address, the expected behaviour is for it to return a DID with the same ID, but with different verification and assertion methods. Could you add a test case for this as well?
2 participants
Covered 9 test cases 1 giving 500 status code and 8 giving 400 status code.