search

awslabs / cognito-at-edge

Serverless authentication solution to protect your website or Amplify application
Apache License 2.0
168 stars 54 forks source link

Add unit test cases to achieve full coverage #2

Closed jeandek closed 3 years ago

jeandek commented 3 years ago

Description of changes: This PR is adding two additional unit test cases to achieve 100% coverage. It also updates the npm test script to calculate coverage.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

jenirain commented 3 years ago

We may also want to add some details in CONTRIBUTING.MD in the ## Testing the code section: user should do npm install of: jest pino jsonwebtoken jwk-to-pem axios

They need these installed or tests will fail. (if you feel that this is overly obvious, it's ok to leave it out)

jeandek commented 3 years ago

We may also want to add some details in CONTRIBUTING.MD in the ## Testing the code section: user should do npm install of: jest pino jsonwebtoken jwk-to-pem axios

They need these installed or tests will fail. (if you feel that this is overly obvious, it's ok to leave it out)

It's fairly obvious to any experience Node.JS developer, but I'll add some setup instructions regardless.

jenirain commented 3 years ago

IntelliJ tells me that at line 13, we should invoke the superclass constructor. I know in Java this is a best practice, but - as you know - I'm not well versed in js. Do we want to add a call to super() at line 13 - before return DATE; ?

jeandek commented 3 years ago

IntelliJ tells me that at line 13, we should invoke the superclass constructor. I know in Java this is a best practice, but - as you know - I'm not well versed in js. Do we want to add a call to super() at line 13 - before return DATE; ?

Good catch, that's a good practice in JS as well.