From cb34af02c628fdf0b6bbd45df2b14dde9319e010 Mon Sep 17 00:00:00 2001 From: David Teller Date: Tue, 5 Jul 2022 15:29:01 +0200 Subject: [PATCH] Revert "Fix: roomMemberTest off-by-one error (#319)" (#323) This reverts commit d8aac434f1279b387cef7b59ed5d2b20f3b25f12. --- package.json | 1 - test/integration/roomMembersTest.ts | 97 +++++++++-------------------- yarn.lock | 12 ---- 3 files changed, 29 insertions(+), 81 deletions(-) diff --git a/package.json b/package.json index f85ceb5..0abada2 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,6 @@ "js-yaml": "^4.1.0", "jsdom": "^16.6.0", "matrix-bot-sdk": "^0.5.19", - "node": "^18.4.0", "parse-duration": "^1.0.2", "shell-quote": "^1.7.3" }, diff --git a/test/integration/roomMembersTest.ts b/test/integration/roomMembersTest.ts index a4017f5..792c911 100644 --- a/test/integration/roomMembersTest.ts +++ b/test/integration/roomMembersTest.ts @@ -401,37 +401,25 @@ describe("Test: Testing RoomMemberManager", function() { } // Create and protect rooms. - // - // We reserve two control rooms: - // - room 0, also known as the "control unprotected room" is unprotected - // (we're not calling `!mjolnir rooms add` for this room), so none - // of the operations of `!mjolnir since` shoud affect it. We are - // using it to control, at the end of each experiment, that none of - // the `!mjolnir since` operations affect it. - // - room 1, also known as the "control protected room" is protected - // (we are calling `!mjolnir rooms add` for this room), but we are - // never directly requesting any `!mjolnir since` action against - // this room. We are using it to control, at the end of each experiment, - // that none of the `!mjolnir since` operations that should target - // one single other room also affect that room. It is, however, affected - // by general operations that are designed to affect all protected rooms. + // - room 0 remains unprotected, as witness; + // - room 1 is protected but won't be targeted directly, also as witness. const NUMBER_OF_ROOMS = 18; - const allRoomIds: string[] = []; - const allRoomAliases: string[] = []; + const roomIds: string[] = []; + const roomAliases: string[] = []; const mjolnirUserId = await this.mjolnir.client.getUserId(); for (let i = 0; i < NUMBER_OF_ROOMS; ++i) { const roomId = await this.moderator.createRoom({ invite: [mjolnirUserId, ...goodUserIds, ...badUserIds], }); - allRoomIds.push(roomId); + roomIds.push(roomId); const alias = `#since-test-${randomUUID()}:localhost:9999`; await this.moderator.createRoomAlias(alias, roomId); - allRoomAliases.push(alias); + roomAliases.push(alias); } - for (let i = 1; i < allRoomIds.length; ++i) { - // Protect all rooms except allRoomIds[0], as control. - const roomId = allRoomIds[i]; + for (let i = 1; i < roomIds.length; ++i) { + // Protect all rooms except roomIds[0], as witness. + const roomId = roomIds[i]; await this.mjolnir.client.joinRoom(roomId); await this.moderator.setUserPowerLevel(mjolnirUserId, roomId, 100); await this.moderator.sendMessage(this.mjolnir.managementRoomId, { msgtype: 'm.text', body: `!mjolnir rooms add ${roomId}` }); @@ -441,8 +429,8 @@ describe("Test: Testing RoomMemberManager", function() { do { let protectedRooms = this.mjolnir.protectedRooms; protectedRoomsUpdated = true; - for (let i = 1; i < allRoomIds.length; ++i) { - const roomId = allRoomIds[i]; + for (let i = 1; i < roomIds.length; ++i) { + const roomId = roomIds[i]; if (!(roomId in protectedRooms)) { protectedRoomsUpdated = false; await new Promise(resolve => setTimeout(resolve, 1_000)); @@ -452,7 +440,7 @@ describe("Test: Testing RoomMemberManager", function() { // Good users join before cut date. for (let user of this.goodUsers) { - for (let roomId of allRoomIds) { + for (let roomId of roomIds) { await user.joinRoom(roomId); } } @@ -465,30 +453,25 @@ describe("Test: Testing RoomMemberManager", function() { // Bad users join after cut date. for (let user of this.badUsers) { - for (let roomId of allRoomIds) { + for (let roomId of roomIds) { await user.joinRoom(roomId); } } - // Finally, prepare our control rooms and separate them - // from the regular rooms. - const CONTROL_UNPROTECTED_ROOM_ID = allRoomIds[0]; - const CONTROL_PROTECTED_ID = allRoomIds[1]; - const roomIds = allRoomIds.slice(2); - const roomAliases = allRoomAliases.slice(2); - enum Method { kick, ban, mute, unmute, } + const WITNESS_UNPROTECTED_ROOM_ID = roomIds[0]; + const WITNESS_ROOM_ID = roomIds[1]; class Experiment { // A human-readable name for the command. readonly name: string; - // If `true`, this command should affect room `CONTROL_PROTECTED_ID`. + // If `true`, this command should affect room `WITNESS_ROOM_ID`. // Defaults to `false`. - readonly shouldAffectControlProtected: boolean; + readonly shouldAffectWitnessRoom: boolean; // The actual command-line. readonly command: (roomId: string, roomAlias: string) => string; // The number of responses we expect to this command. @@ -501,23 +484,17 @@ describe("Test: Testing RoomMemberManager", function() { // Defaults to `false`. readonly isSameRoomAsPrevious: boolean; - // The index of the room on which we're acting. - // - // Initialized by `addTo`. roomIndex: number | undefined; - constructor({name, shouldAffectControlProtected, command, n, method, sameRoom}: {name: string, command: (roomId: string, roomAlias: string) => string, shouldAffectControlProtected?: boolean, n?: number, method: Method, sameRoom?: boolean}) { + constructor({name, shouldAffectWitnessRoom, command, n, method, sameRoom}: {name: string, command: (roomId: string, roomAlias: string) => string, shouldAffectWitnessRoom?: boolean, n?: number, method: Method, sameRoom?: boolean}) { this.name = name; - this.shouldAffectControlProtected = typeof shouldAffectControlProtected === "undefined" ? false : shouldAffectControlProtected; + this.shouldAffectWitnessRoom = typeof shouldAffectWitnessRoom === "undefined" ? false : shouldAffectWitnessRoom; this.command = command; this.n = typeof n === "undefined" ? 1 : n; this.method = method; this.isSameRoomAsPrevious = typeof sameRoom === "undefined" ? false : sameRoom; } - // Add an experiment to the list of experiments. - // - // This is how `roomIndex` gets initialized. addTo(experiments: Experiment[]) { if (this.isSameRoomAsPrevious) { this.roomIndex = experiments[experiments.length - 1].roomIndex; @@ -609,7 +586,7 @@ describe("Test: Testing RoomMemberManager", function() { new Experiment({ name: "kick with date and reason", command: (roomId: string) => `!mjolnir since "${cutDate}" kick 100 ${roomId} bad, bad user`, - shouldAffectControlProtected: false, + shouldAffectWitnessRoom: false, n: 1, method: Method.kick, }), @@ -649,35 +626,19 @@ describe("Test: Testing RoomMemberManager", function() { new Experiment({ name: "kick with date everywhere", command: () => `!mjolnir since "${cutDate}" kick 100 * bad, bad user`, - shouldAffectControlProtected: true, + shouldAffectWitnessRoom: true, n: NUMBER_OF_ROOMS - 1, method: Method.kick, }), ]) { experiment.addTo(EXPERIMENTS); } - - // Just-in-case health check, before starting. - { - const usersInUnprotectedControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_UNPROTECTED_ROOM_ID); - const usersInControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_PROTECTED_ID); - for (let userId of goodUserIds) { - assert.ok(usersInUnprotectedControlProtected.includes(userId), `Initially, good user ${userId} should be in the unprotected control room`); - assert.ok(usersInControlProtected.includes(userId), `Initially, good user ${userId} should be in the control room`); - } - for (let userId of badUserIds) { - assert.ok(usersInUnprotectedControlProtected.includes(userId), `Initially, bad user ${userId} should be in the unprotected control room`); - assert.ok(usersInControlProtected.includes(userId), `Initially, bad user ${userId} should be in the control room`); - } - } - for (let i = 0; i < EXPERIMENTS.length; ++i) { const experiment = EXPERIMENTS[i]; - const index = experiment.roomIndex!; - const roomId = roomIds[index]; + const index = experiment.roomIndex! + 1; + const roomId = roomIds[index]; const roomAlias = roomAliases[index]; const joined = this.mjolnir.roomJoins.getUsersInRoom(roomId, start, 100); - console.debug(`Running experiment ${i} "${experiment.name}" in room index ${index} (${roomId} / ${roomAlias}): \`${experiment.command(roomId, roomAlias)}\``); assert.ok(joined.length >= 2 * SAMPLE_SIZE, `In experiment ${experiment.name}, we should have seen ${2 * SAMPLE_SIZE} users, saw ${joined.length}`); // Run experiment. @@ -689,12 +650,12 @@ describe("Test: Testing RoomMemberManager", function() { // Check post-conditions. const usersInRoom = await this.mjolnir.client.getJoinedRoomMembers(roomId); - const usersInUnprotectedControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_UNPROTECTED_ROOM_ID); - const usersInControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_PROTECTED_ID); + const usersInUnprotectedWitnessRoom = await this.mjolnir.client.getJoinedRoomMembers(WITNESS_UNPROTECTED_ROOM_ID); + const usersInWitnessRoom = await this.mjolnir.client.getJoinedRoomMembers(WITNESS_ROOM_ID); for (let userId of goodUserIds) { assert.ok(usersInRoom.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in affected room`); - assert.ok(usersInControlProtected.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in control room (${CONTROL_PROTECTED_ID})`); - assert.ok(usersInUnprotectedControlProtected.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in unprotected control room (${CONTROL_UNPROTECTED_ROOM_ID})`); + assert.ok(usersInWitnessRoom.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in witness room`); + assert.ok(usersInUnprotectedWitnessRoom.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in unprotected witness room`); } if (experiment.method === Method.mute) { for (let userId of goodUserIds) { @@ -717,8 +678,8 @@ describe("Test: Testing RoomMemberManager", function() { } else { for (let userId of badUserIds) { assert.ok(!usersInRoom.includes(userId), `After a ${experiment.name}, bad user ${userId} should NOT be in affected room`); - assert.equal(usersInControlProtected.includes(userId), !experiment.shouldAffectControlProtected, `After a ${experiment.name}, bad user ${userId} should ${experiment.shouldAffectControlProtected ? "NOT" : "still"} be in control room`); - assert.ok(usersInUnprotectedControlProtected.includes(userId), `After a ${experiment.name}, bad user ${userId} should still be in unprotected control room`); + assert.equal(usersInWitnessRoom.includes(userId), !experiment.shouldAffectWitnessRoom, `After a ${experiment.name}, bad user ${userId} should ${experiment.shouldAffectWitnessRoom ? "NOT" : "still"} be in witness room`); + assert.ok(usersInUnprotectedWitnessRoom.includes(userId), `After a ${experiment.name}, bad user ${userId} should still be in unprotected witness room`); const leaveEvent = await this.mjolnir.client.getRoomStateEvent(roomId, "m.room.member", userId); switch (experiment.method) { case Method.kick: diff --git a/yarn.lock b/yarn.lock index 25469bf..21df715 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2061,18 +2061,6 @@ negotiator@0.6.3: resolved "https://registry.yarnpkg.com/negotiator/-/negotiator-0.6.3.tgz#58e323a72fedc0d6f9cd4d31fe49f51479590ccd" integrity sha512-+EUsqGPLsM+j/zdChZjsnX51g4XrHFOIXwfnCVPGlQk/k5giakcKsuxCObBRu6DSm9opw/O6slWbJdghQM4bBg== -node-bin-setup@^1.0.0: - version "1.1.0" - resolved "https://registry.yarnpkg.com/node-bin-setup/-/node-bin-setup-1.1.0.tgz#9df94c41335a8f41958a639b2736f860582a209c" - integrity sha512-pTeU6NgUrexiLNtd+AKwvg6cngHMvj5FZ5e2bbv2ogBSIc9yhkXSSaTScfSRZnwHIh5YFmYSYlemLWkiKD7rog== - -node@^18.4.0: - version "18.4.0" - resolved "https://registry.yarnpkg.com/node/-/node-18.4.0.tgz#94e9a09f7303ad5cc64bc60950dba78e9a5664c5" - integrity sha512-uOeU340PxuLdddq9IWLBNrPU1x/ndr+p7UE3C9D+hHeXqjWC7zfe+QY67pfis/1Z5e0WXIImU5eBj7tfOpHIVg== - dependencies: - node-bin-setup "^1.0.0" - normalize-path@^3.0.0, normalize-path@~3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/normalize-path/-/normalize-path-3.0.0.tgz#0dcd69ff23a1c9b11fd0978316644a0388216a65"