test(verification): integration tests for otp verification #1928
Conversation
…or otp verification
seaerchin
requested review from
karrui,
mantariksh,
yong-jie,
tshuli and
chowyiyin
src/app/routes/api/v3/forms/__tests__/public-forms.verification.routes.spec.ts
Outdated
Show resolved
Hide resolved
src/app/routes/api/v3/forms/__tests__/public-forms.verification.routes.spec.ts
Outdated
Show resolved
Hide resolved
| // Arrange | ||
| jest | ||
| .spyOn(VerificationModel, 'findById') | ||
| .mockReturnValue({ exec: jest.fn().mockRejectedValue('no') }) |
There was a problem hiding this comment.
Not sure, but is it okay that the mockRejectedValue is 'no' instead of an error?
There was a problem hiding this comment.
yeap! the reason this is ok (or not, because of neverthrow hehehe) is due to neverthrow's ResultAsync.fromPromise.
our calls to the database essentially returns Promise<ModelSchema | null>, where the null is used to indicate an expected error when something is off with the query and the requested information could not be found (eg: no model matching the specified options).
however, promises and throwable functions actually have the possibility of an exception, which is an unexpected error that the called function isn't qualified to handle, and returns to teh caller. this is not encoded in the type signature and hence, is a mental overhead that engineers have to keep in the when calling functions, lest it crashes their system. In light of this, promises are actually typed as such: Promise<your_return_type | exception>
The ResultAsync.fromPromise function in neverthrow essentially allows us to sidestep this problem for promises (the corresponding function for throwables is Result.fromThrowable. This allows us to catch a rejecting promise (or an error) and map it back using a function. Because the type of exception is not encoded in ts' type system, this has to transform an unknown error back into a proper error type, which is done by transformMongoError
tl; dr: rejecting the promise causes it to go into error branch, and then a default error is returned from transformMongoError hence it's fine. thx for coming to my ted talk.
There was a problem hiding this comment.
Ohh okay thanks for sharing!
There was a problem hiding this comment.
@chowyiyin fyi the team decided a while ago that integration tests only need to cover Joi validation and success cases, since the controller unit tests should already test for the various error cases. @seaerchin has put in a lot of effort to mock lower layers even in integration tests so that error cases can be tested, which is great, but requires quite a bit of additional work.
|
|
||
| describe('POST /forms/:formId/fieldverifications/:transactionId/fields/:fieldId/otp/verify', () => { | ||
| beforeAll(() => { | ||
| jest.spyOn(bcrypt, 'compare').mockResolvedValue(true) |
There was a problem hiding this comment.
does this need to be mocked? we should try to only mock things we cannot simulate in integration tests; we should instead call the generate end point for real and see if verify returns success.
see our auth integration tests
There was a problem hiding this comment.
good point! will do! will wait for otp generation to be merged first because this change is dependent on that
… otp instead of mocking hash
) * refactor(fieldverificationservice): updated verifyOtp to new parameters and updated tests * docs(verification): adds deprecation notice for verifyOtp * feat(verification): adds new controller method and route for otp verification * refactor(verifiable-field): updated call site for verify otp * test(verification/controller/spec): adds unit tests for handleOtpVerification * docs(public-forms/verification/routes): adds docs for otp verification * docs(verification/controller): add docs for otp verification * test(verification): integration tests for otp verification (#1928) * test(public-forms/verification/routes/spec): adds integration tests for otp verification * docs(public-forms/verification/routes/test): fixed typo * refactor(public-forms/verifaction/routes/spec): changed to generating otp instead of mocking hash
Problem
#1926 has no integration tests to prevent review fatigue. This PR is a follow up on that
Solution
This adds integration tests for #1926