-
Notifications
You must be signed in to change notification settings - Fork 498
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
Add initial tests on the authentication service. #6229
Conversation
8464619
to
e889804
Compare
📱 Scan the QR code below to install the build for this PR. If you can't scan the QR code you can install the build via this link: https://i.diawi.com/2J1fg6 |
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.
LGTM 👍
Not sure what's up with the GH Action though as I fail loading logs, tests did run successfully for me.
Thanks, yeah I'm confused too. Didn't get time to look into it today so will have to have a look next week when I'm back. |
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.
The tests look good to me but a lot of them crash (EXC_BAD_ACCESS) now inside the MXRestClient (they work on develop). I believe the same thing happens on the CI but isn't handled properly.
I imagine it's linked to the sessionCreator
but I haven't really looked into it.
RiotSwiftUI/Modules/Authentication/Common/Service/MatrixSDK/AuthenticationRestClient.swift
Show resolved
Hide resolved
Bingo! I've added an empty handler in the init as this seems to be expected when the credentials are set (and not checked for nullability) in |
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.
👍 👍
Most of the authentication service was untested (and some of the tests were temporarily using live servers). This PR adds a
MockAuthenticationRestClient
and tests for login/registration. There is room for more tests (and adding an email/MSISDN identifier still needs implementation), but these tests cover the majority of the flow/state and can be added to over time.Part of #6179 depends on matrix-org/matrix-ios-sdk#1484