Skip to content

Conversation

@matthewleemle
Copy link
Contributor

No description provided.

}).lean();
let availbilityStatus = await AvailbilityService.checkAvailabilityStatus(r.id);
if (availbilityStatus == true) count++;
try{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i believe this is not needed (try catch block)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was something I had during debugging, I'll take it away

Copy link
Contributor

@mankitkong mankitkong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Responder endpoints checking should be in another file, they are no longer in the authentication flow.

For authentication tests. You should also check for invalid cases, like password incorrect etc. Try to hit all the different 4** http codes (e.g. 400). It is actually more important to test the error cases than it is the correct cases. Reason being correct cases can usually be checked by navigating the app itself.

@mankitkong
Copy link
Contributor

Have you also been getting timeout errors? You can increase this timeout duration.
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (C:\Users\Nicholas\Documents\Code the Change\UHN-Backend-Server\tests\authentication.test.js)

@matthewleemle
Copy link
Contributor Author

Have you also been getting timeout errors? You can increase this timeout duration.
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (C:\Users\Nicholas\Documents\Code the Change\UHN-Backend-Server\tests\authentication.test.js)

I haven't been getting time out issues, which test case is this?

@matthewleemle
Copy link
Contributor Author

Responder endpoints checking should be in another file, they are no longer in the authentication flow.

For authentication tests. You should also check for invalid cases, like password incorrect etc. Try to hit all the different 4** http codes (e.g. 400). It is actually more important to test the error cases than it is the correct cases. Reason being correct cases can usually be checked by navigating the app itself.

It's more like I'm just retrieving the login tokens in those tests to be used in other test cases. Should I delete the test cases and retrieve login tokens separately?

@mankitkong
Copy link
Contributor

Have you also been getting timeout errors? You can increase this timeout duration.
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (C:\Users\Nicholas\Documents\Code the Change\UHN-Backend-Server\tests\authentication.test.js)

I haven't been getting time out issues, which test case is this?

The authenticaiton ones

@mankitkong
Copy link
Contributor

Responder endpoints checking should be in another file, they are no longer in the authentication flow.
For authentication tests. You should also check for invalid cases, like password incorrect etc. Try to hit all the different 4** http codes (e.g. 400). It is actually more important to test the error cases than it is the correct cases. Reason being correct cases can usually be checked by navigating the app itself.

It's more like I'm just retrieving the login tokens in those tests to be used in other test cases. Should I delete the test cases and retrieve login tokens separately?

It should instead be assumed to be working in the responder tests, done in the before block. If there were any issues with the authentication part, the authentication test block will catch it. Of course, all the other tests will fail too if that's the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants