Refactor the widget tests to use MockMatrixServer
#4236
Conversation
toger5
force-pushed
the
toger5/mock-matrix-server-widget-tests
branch
from
b0790c0
to
a628d83
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4236 +/- ##
==========================================
- Coverage 85.10% 85.08% -0.02%
==========================================
Files 274 274
Lines 30203 30240 +37
==========================================
+ Hits 25703 25729 +26
- Misses 4500 4511 +11 ☔ View full report in Codecov by Sentry. |
poljar
changed the title
MockMatrixServer
toger5
force-pushed
the
toger5/mock-matrix-server-widget-tests
branch
from
a628d83
to
b345d64
Compare
|
How do i test |
toger5
force-pushed
the
toger5/mock-matrix-server-widget-tests
branch
from
b345d64
to
c573c2d
Compare
We usually don't test the test utilities; they should be straightforward enough that using them in tests would reveal their issues. |
toger5
force-pushed
the
toger5/mock-matrix-server-widget-tests
branch
from
c573c2d
to
e8d57ca
Compare
Examples in the docstrings will be run as the doc tests which can run and test the mocks. At least that's how I wrote all the examples for the |
| impl<'a> MockEndpoint<'a, RoomMessagesEndpoint> { | ||
| /// Returns a messages endpoint that emulates success, i.e. the messages | ||
| /// provided as `response` could be retrieved. | ||
| pub fn ok(self, response: impl Into<Value>) -> MatrixMock<'a> { |
There was a problem hiding this comment.
No, we should not use impl Into<Value>, otherwise we lose all the usefulness of these mocking APIs by reverting to "we don't know what the return value looks like, it can be an arbitrary JSON". Instead we should use high-level types. We have another place where we do mock messages, grep for fn mock_messages (…it's even better, we do have two implementations :D).
toger5
force-pushed
the
toger5/mock-matrix-server-widget-tests
branch
from
2df7183
to
0af992b
Compare
toger5
force-pushed
the
toger5/mock-matrix-server-widget-tests
branch
4 times, most recently
from
40276b7
to
a7d0264
Compare
toger5
force-pushed
the
toger5/mock-matrix-server-widget-tests
branch
from
a7d0264
to
5d5bec4
Compare
| let endpoint = RoomSendStateEndpoint { event_type: Some(event_type), ..self.endpoint }; | ||
| let matcher = path_regex(Self::generate_path_regex(&endpoint)); | ||
| Self { endpoint, mock: self.mock.and(matcher), ..self } |
There was a problem hiding this comment.
I think here it would be better to generate the path regex at the end, just before building the final MatrixMock.
There was a problem hiding this comment.
Where would that be?
There are numerous places we build the final MatrixMock. Do you want to implement all the methods from MockEndpoint<'a, T> in
MockEndpoint<'a, RoomSendStateEndpoint>. What if someone adds a new method to the generic version that can build a matrix mock but forgets to update this version to build the regex?
| let endpoint = RoomSendStateEndpoint { state_key: Some(state_key), ..self.endpoint }; | ||
| let matcher = path_regex(Self::generate_path_regex(&endpoint)); | ||
| Self { endpoint, mock: self.mock.and(matcher), ..self } |
There was a problem hiding this comment.
Yeah, same here: we should fill the state_key field in RoomSendStateEndpoint, but not make the matcher more precise there.
| let matcher = path_regex(Self::generate_path_regex(&endpoint)); | ||
| Self { endpoint, mock: self.mock.and(matcher), ..self } | ||
| } | ||
|
|
There was a problem hiding this comment.
note: doc comment for the ok_with_event_id is incorrect, as it duplicates the doc comment for mock_room_send without any change in the example code.
There was a problem hiding this comment.
You mean the doc comment for ok?
I suppose this is still helpful to have for ok. What other test du you have in mind?
toger5
force-pushed
the
toger5/mock-matrix-server-widget-tests
branch
from
966bf1c
to
62a1fcf
Compare
This is a follow up PR on: #3987
And tries to use the
MockMatrixServerwherever reasonable in the widget integration tests.Signed-off-by: