Tests should not be fragile glue.

Last week we had a systemic CI failure because of https://github.com/matrix-org/mjolnir/issues/281 and several tests (abuseReportTest, makeAdminCommandTest, roomMemberManagerTest (for the since command)) failed in very unclear ways.

This was because not only did the rely on the implicit behaviour of Synapse making new rooms public, but the event listeners further along in the tests that are used to detect certain events doing a round trip (e.g. power level changes after invoking a command) also relied on an "implicit temporal link". I don't know how to describe this formally, but essentially we weren't _waiting_ for the event listeners to detect the round trip before carrying out seemingly unrelated assertions. The general quality of the tests involved was also just poor for straight forward reasons (massive lexical scope, masses of imperative glue code)

the since command was by far the worst for this and I was close to reverting it entirely.

I still don't understand how the roomMemberTest is supposed to work, at all. https://github.com/matrix-org/mjolnir/blob/main/test/integration/roomMembersTest.ts#L286-L295

https://github.com/matrix-org/mjolnir/pull/219/files#diff-4e8bbc9dde21b7b895e0c081d2e3375c8958c51a10442497dfffb677f0d59a1aR1-R107
https://github.com/matrix-org/mjolnir/pull/238
https://github.com/matrix-org/mjolnir/pull/238/files#diff-865e52ce48a7c93d2dd6886810e49e5b01d3d723f06e5c7abae54913579f3014R114
https://github.com/matrix-org/mjolnir/blob/main/test/integration/abuseReportTest.ts#L28-L197

One way of solving some of these problems (especially in regards to less trivial things such as "what happens when a test times out", "how should i wait for Mjolnir to send some event?") would be to create a testing guide that could be referred to vigorously by both the author of tests and the reviewer.

Start by reading issue 281 and the cited integration tests, especially test/integration/roomMembersTest.ts lines 286-295 and test/integration/abuseReportTest.ts lines 28-197. Compare the related pull requests to understand the implicit room and event-order assumptions. Done should mean the affected tests and their waiting behavior are documented or made reliable, but the issue does not define a bounded final scope.

Written by the indexing model from the issue text.