Revert "Fix: roomMemberTest off-by-one error (#319)"

This reverts commit d8aac434f1.
This commit is contained in:
David Teller 2022-07-05 15:25:17 +02:00 committed by GitHub
parent d8aac434f1
commit 5c4ad7337c
3 changed files with 29 additions and 81 deletions

View File

@ -44,7 +44,6 @@
"js-yaml": "^4.1.0", "js-yaml": "^4.1.0",
"jsdom": "^16.6.0", "jsdom": "^16.6.0",
"matrix-bot-sdk": "^0.5.19", "matrix-bot-sdk": "^0.5.19",
"node": "^18.4.0",
"parse-duration": "^1.0.2", "parse-duration": "^1.0.2",
"shell-quote": "^1.7.3" "shell-quote": "^1.7.3"
}, },

View File

@ -401,37 +401,25 @@ describe("Test: Testing RoomMemberManager", function() {
} }
// Create and protect rooms. // Create and protect rooms.
// // - room 0 remains unprotected, as witness;
// We reserve two control rooms: // - room 1 is protected but won't be targeted directly, also as witness.
// - 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.
const NUMBER_OF_ROOMS = 18; const NUMBER_OF_ROOMS = 18;
const allRoomIds: string[] = []; const roomIds: string[] = [];
const allRoomAliases: string[] = []; const roomAliases: string[] = [];
const mjolnirUserId = await this.mjolnir.client.getUserId(); const mjolnirUserId = await this.mjolnir.client.getUserId();
for (let i = 0; i < NUMBER_OF_ROOMS; ++i) { for (let i = 0; i < NUMBER_OF_ROOMS; ++i) {
const roomId = await this.moderator.createRoom({ const roomId = await this.moderator.createRoom({
invite: [mjolnirUserId, ...goodUserIds, ...badUserIds], invite: [mjolnirUserId, ...goodUserIds, ...badUserIds],
}); });
allRoomIds.push(roomId); roomIds.push(roomId);
const alias = `#since-test-${randomUUID()}:localhost:9999`; const alias = `#since-test-${randomUUID()}:localhost:9999`;
await this.moderator.createRoomAlias(alias, roomId); await this.moderator.createRoomAlias(alias, roomId);
allRoomAliases.push(alias); roomAliases.push(alias);
} }
for (let i = 1; i < allRoomIds.length; ++i) { for (let i = 1; i < roomIds.length; ++i) {
// Protect all rooms except allRoomIds[0], as control. // Protect all rooms except roomIds[0], as witness.
const roomId = allRoomIds[i]; const roomId = roomIds[i];
await this.mjolnir.client.joinRoom(roomId); await this.mjolnir.client.joinRoom(roomId);
await this.moderator.setUserPowerLevel(mjolnirUserId, roomId, 100); await this.moderator.setUserPowerLevel(mjolnirUserId, roomId, 100);
await this.moderator.sendMessage(this.mjolnir.managementRoomId, { msgtype: 'm.text', body: `!mjolnir rooms add ${roomId}` }); 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 { do {
let protectedRooms = this.mjolnir.protectedRooms; let protectedRooms = this.mjolnir.protectedRooms;
protectedRoomsUpdated = true; protectedRoomsUpdated = true;
for (let i = 1; i < allRoomIds.length; ++i) { for (let i = 1; i < roomIds.length; ++i) {
const roomId = allRoomIds[i]; const roomId = roomIds[i];
if (!(roomId in protectedRooms)) { if (!(roomId in protectedRooms)) {
protectedRoomsUpdated = false; protectedRoomsUpdated = false;
await new Promise(resolve => setTimeout(resolve, 1_000)); await new Promise(resolve => setTimeout(resolve, 1_000));
@ -452,7 +440,7 @@ describe("Test: Testing RoomMemberManager", function() {
// Good users join before cut date. // Good users join before cut date.
for (let user of this.goodUsers) { for (let user of this.goodUsers) {
for (let roomId of allRoomIds) { for (let roomId of roomIds) {
await user.joinRoom(roomId); await user.joinRoom(roomId);
} }
} }
@ -465,30 +453,25 @@ describe("Test: Testing RoomMemberManager", function() {
// Bad users join after cut date. // Bad users join after cut date.
for (let user of this.badUsers) { for (let user of this.badUsers) {
for (let roomId of allRoomIds) { for (let roomId of roomIds) {
await user.joinRoom(roomId); 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 { enum Method {
kick, kick,
ban, ban,
mute, mute,
unmute, unmute,
} }
const WITNESS_UNPROTECTED_ROOM_ID = roomIds[0];
const WITNESS_ROOM_ID = roomIds[1];
class Experiment { class Experiment {
// A human-readable name for the command. // A human-readable name for the command.
readonly name: string; 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`. // Defaults to `false`.
readonly shouldAffectControlProtected: boolean; readonly shouldAffectWitnessRoom: boolean;
// The actual command-line. // The actual command-line.
readonly command: (roomId: string, roomAlias: string) => string; readonly command: (roomId: string, roomAlias: string) => string;
// The number of responses we expect to this command. // The number of responses we expect to this command.
@ -501,23 +484,17 @@ describe("Test: Testing RoomMemberManager", function() {
// Defaults to `false`. // Defaults to `false`.
readonly isSameRoomAsPrevious: boolean; readonly isSameRoomAsPrevious: boolean;
// The index of the room on which we're acting.
//
// Initialized by `addTo`.
roomIndex: number | undefined; 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.name = name;
this.shouldAffectControlProtected = typeof shouldAffectControlProtected === "undefined" ? false : shouldAffectControlProtected; this.shouldAffectWitnessRoom = typeof shouldAffectWitnessRoom === "undefined" ? false : shouldAffectWitnessRoom;
this.command = command; this.command = command;
this.n = typeof n === "undefined" ? 1 : n; this.n = typeof n === "undefined" ? 1 : n;
this.method = method; this.method = method;
this.isSameRoomAsPrevious = typeof sameRoom === "undefined" ? false : sameRoom; this.isSameRoomAsPrevious = typeof sameRoom === "undefined" ? false : sameRoom;
} }
// Add an experiment to the list of experiments.
//
// This is how `roomIndex` gets initialized.
addTo(experiments: Experiment[]) { addTo(experiments: Experiment[]) {
if (this.isSameRoomAsPrevious) { if (this.isSameRoomAsPrevious) {
this.roomIndex = experiments[experiments.length - 1].roomIndex; this.roomIndex = experiments[experiments.length - 1].roomIndex;
@ -609,7 +586,7 @@ describe("Test: Testing RoomMemberManager", function() {
new Experiment({ new Experiment({
name: "kick with date and reason", name: "kick with date and reason",
command: (roomId: string) => `!mjolnir since "${cutDate}" kick 100 ${roomId} bad, bad user`, command: (roomId: string) => `!mjolnir since "${cutDate}" kick 100 ${roomId} bad, bad user`,
shouldAffectControlProtected: false, shouldAffectWitnessRoom: false,
n: 1, n: 1,
method: Method.kick, method: Method.kick,
}), }),
@ -649,35 +626,19 @@ describe("Test: Testing RoomMemberManager", function() {
new Experiment({ new Experiment({
name: "kick with date everywhere", name: "kick with date everywhere",
command: () => `!mjolnir since "${cutDate}" kick 100 * bad, bad user`, command: () => `!mjolnir since "${cutDate}" kick 100 * bad, bad user`,
shouldAffectControlProtected: true, shouldAffectWitnessRoom: true,
n: NUMBER_OF_ROOMS - 1, n: NUMBER_OF_ROOMS - 1,
method: Method.kick, method: Method.kick,
}), }),
]) { ]) {
experiment.addTo(EXPERIMENTS); 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) { for (let i = 0; i < EXPERIMENTS.length; ++i) {
const experiment = EXPERIMENTS[i]; const experiment = EXPERIMENTS[i];
const index = experiment.roomIndex!; const index = experiment.roomIndex! + 1;
const roomId = roomIds[index]; const roomId = roomIds[index];
const roomAlias = roomAliases[index]; const roomAlias = roomAliases[index];
const joined = this.mjolnir.roomJoins.getUsersInRoom(roomId, start, 100); 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}`); assert.ok(joined.length >= 2 * SAMPLE_SIZE, `In experiment ${experiment.name}, we should have seen ${2 * SAMPLE_SIZE} users, saw ${joined.length}`);
// Run experiment. // Run experiment.
@ -689,12 +650,12 @@ describe("Test: Testing RoomMemberManager", function() {
// Check post-conditions. // Check post-conditions.
const usersInRoom = await this.mjolnir.client.getJoinedRoomMembers(roomId); const usersInRoom = await this.mjolnir.client.getJoinedRoomMembers(roomId);
const usersInUnprotectedControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_UNPROTECTED_ROOM_ID); const usersInUnprotectedWitnessRoom = await this.mjolnir.client.getJoinedRoomMembers(WITNESS_UNPROTECTED_ROOM_ID);
const usersInControlProtected = await this.mjolnir.client.getJoinedRoomMembers(CONTROL_PROTECTED_ID); const usersInWitnessRoom = await this.mjolnir.client.getJoinedRoomMembers(WITNESS_ROOM_ID);
for (let userId of goodUserIds) { 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(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(usersInWitnessRoom.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in witness room`);
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(usersInUnprotectedWitnessRoom.includes(userId), `After a ${experiment.name}, good user ${userId} should still be in unprotected witness room`);
} }
if (experiment.method === Method.mute) { if (experiment.method === Method.mute) {
for (let userId of goodUserIds) { for (let userId of goodUserIds) {
@ -717,8 +678,8 @@ describe("Test: Testing RoomMemberManager", function() {
} else { } else {
for (let userId of badUserIds) { for (let userId of badUserIds) {
assert.ok(!usersInRoom.includes(userId), `After a ${experiment.name}, bad user ${userId} should NOT be in affected room`); 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.equal(usersInWitnessRoom.includes(userId), !experiment.shouldAffectWitnessRoom, `After a ${experiment.name}, bad user ${userId} should ${experiment.shouldAffectWitnessRoom ? "NOT" : "still"} be in witness room`);
assert.ok(usersInUnprotectedControlProtected.includes(userId), `After a ${experiment.name}, bad user ${userId} should still be in unprotected control 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); const leaveEvent = await this.mjolnir.client.getRoomStateEvent(roomId, "m.room.member", userId);
switch (experiment.method) { switch (experiment.method) {
case Method.kick: case Method.kick:

View File

@ -2061,18 +2061,6 @@ negotiator@0.6.3:
resolved "https://registry.yarnpkg.com/negotiator/-/negotiator-0.6.3.tgz#58e323a72fedc0d6f9cd4d31fe49f51479590ccd" resolved "https://registry.yarnpkg.com/negotiator/-/negotiator-0.6.3.tgz#58e323a72fedc0d6f9cd4d31fe49f51479590ccd"
integrity sha512-+EUsqGPLsM+j/zdChZjsnX51g4XrHFOIXwfnCVPGlQk/k5giakcKsuxCObBRu6DSm9opw/O6slWbJdghQM4bBg== 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: normalize-path@^3.0.0, normalize-path@~3.0.0:
version "3.0.0" version "3.0.0"
resolved "https://registry.yarnpkg.com/normalize-path/-/normalize-path-3.0.0.tgz#0dcd69ff23a1c9b11fd0978316644a0388216a65" resolved "https://registry.yarnpkg.com/normalize-path/-/normalize-path-3.0.0.tgz#0dcd69ff23a1c9b11fd0978316644a0388216a65"