-
Notifications
You must be signed in to change notification settings - Fork 56
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
Improve mock #723
base: dev
Are you sure you want to change the base?
Improve mock #723
Conversation
@@ -760,6 +760,9 @@ public Q_SLOTS: | |||
static Connection* makeMockConnection(const QString& mxId, | |||
bool enableEncryption = E2EE_Enabled); | |||
|
|||
static Connection* makeMockConnection(Connection *connection, const QString& mxId, |
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.
this API looks weird, why is it a static function that's passed an object?
Looking at the neochat MR for this, it seems that we could easily use the existing makeMockConnection function
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 issue with the existing function is that it return a Connection *
and not a NeoChatConnection *
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.
AccountRegistry
looks like a good place for it? Then you could derive from it and override a virtual function. Or simply making makeMockConnection()
a template would also work I guess (would need pulling completeSetup()
from Connection::Private
to Connection
but that's a smaller evil)?
@@ -947,6 +950,9 @@ public Q_SLOTS: | |||
Room* provideRoom(const QString& roomId, | |||
Omittable<JoinState> joinState = none); | |||
|
|||
//! Add room to the connection |
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.
Please also mention that this is only for tests and not useful in normal operation
No description provided.