Add TypingCache to maintain a list of users typing #559
Conversation
| } | ||
| } | ||
|
|
||
| func testGetTypingUsersAfterTimeout(t *testing.T, tCache *TypingCache) { |
There was a problem hiding this comment.
This test fails since the removeUserAfterDuration function will remove user1 after the first timeout occurs regardless of the fact that user1 is again added with a longer expiry.
| } | ||
|
|
||
| func testGetTypingUsersAfterTimeout(t *testing.T, tCache *TypingCache) { | ||
| time.Sleep(defaultInterval) |
There was a problem hiding this comment.
Please don't use Sleep in tests, they should complete quickly. Instead have the cache accept a "clock", this can then be mocked in the unit tests
| } | ||
|
|
||
| t.data[roomID][userID] = userExists | ||
| t.Unlock() |
There was a problem hiding this comment.
I would heavily recommend using defer where possible.
t.Lock()
defer t.Unlock()
...There was a problem hiding this comment.
I got swept away by the argument that defer introduce an overhead. :P
But good programming practice.
|
|
||
| for _, tt := range tests { | ||
| tCache.removeUserIfExpired(tt.userID, tt.roomID) | ||
| if gotUsers := tCache.GetTypingUsers(tt.roomID); !reflect.DeepEqual(gotUsers, tt.wantUsers) { |
There was a problem hiding this comment.
DeepEqual fails when the order of the entries does not match.
This makes this test flaky.
There was a problem hiding this comment.
BTW there is this. You might reuse that somehow:
There was a problem hiding this comment.
Thanks for the suggestion. :)
Signed-Off-By: Matthias Kesler <krombel@krombel.de>
|
New test from @krombel. Yay! |
|
For some reason travis build is not visible. One can it here https://travis-ci.org/matrix-org/dendrite/builds/409862091 |
| } | ||
| i := rand.Intn(len(tt.userIDs)) | ||
| tCache.removeUser(tt.userIDs[i], tt.roomID) | ||
| expLeftUsers := append(tt.userIDs[:i], tt.userIDs[i+1:]...) |
There was a problem hiding this comment.
Please don't use random in tests, it can make them flakey and annoying to debug. I'd just pick one to remove, e.g. the last, as there's nothing that should depend on the ordering here.
ready for review.
Signed-off-by: Anant Prakash <anantprakashjsr@gmail.com>