From 7bbc6369b3acbd3f40125e14895472cbd0ca20a0 Mon Sep 17 00:00:00 2001 From: Makyen Date: Tue, 24 May 2022 23:05:23 -0700 Subject: [PATCH 1/3] Add threading locks for global values in chatcommunicate.py --- chatcommunicate.py | 361 ++++++++++++++++------------ findspam.py | 3 +- test/test_chatcommunicate.py | 440 ++++++++++++++++++----------------- ws.py | 10 +- 4 files changed, 440 insertions(+), 374 deletions(-) diff --git a/chatcommunicate.py b/chatcommunicate.py index 169d5cbfd0..1b585b2053 100644 --- a/chatcommunicate.py +++ b/chatcommunicate.py @@ -15,6 +15,7 @@ import time import yaml import shlex +import copy import datahandling import metasmoke @@ -41,22 +42,36 @@ class CmdException(Exception): _prefix_commands = {} +_prefix_commands_lock = threading.RLock() + _reply_commands = {} +_reply_commands_lock = threading.RLock() _clients = { "stackexchange.com": None, "stackoverflow.com": None, "meta.stackexchange.com": None } +_clients_lock = threading.RLock() -_command_rooms = set() -_watcher_rooms = set() _room_roles = {} +_room_roles_lock = threading.RLock() + _privileges = {} +_privileges_lock = threading.RLock() _global_block = -1 +_global_block_lock = threading.RLock() + _rooms = {} +_command_rooms = set() +_watcher_rooms = set() +_rooms_lock = threading.RLock() + _last_messages = LastMessages({}, collections.OrderedDict()) +_last_messages_lock = threading.RLock() + +# queue.Queue() is already thread safe, so doesn't need manual locks. _msg_queue = queue.Queue() _pickle_run = threading.Event() @@ -68,7 +83,10 @@ def init(username, password, try_cookies=True): global _room_data global _last_messages - for site in _clients.keys(): + with _clients_lock: + client_keys = [key for key in _clients.keys()] + + for site in client_keys: client = Client(site) logged_in = False @@ -128,7 +146,8 @@ def init(username, password, try_cookies=True): else: raise Exception("Failed to log into " + site + ", max retries exceeded") - _clients[site] = client + with _clients_lock: + _clients[site] = client if os.path.exists("rooms_custom.yml"): parse_room_config("rooms_custom.yml") @@ -140,7 +159,8 @@ def init(username, password, try_cookies=True): if datahandling.has_pickle("messageData.p"): try: - _last_messages = datahandling.load_pickle("messageData.p") + with _last_messages_lock: + _last_messages = datahandling.load_pickle("messageData.p") except EOFError: pass @@ -152,13 +172,15 @@ def init(username, password, try_cookies=True): def join_command_rooms(): - for site, roomid in _command_rooms: - room = _clients[site].get_room(roomid) - deletion_watcher = (site, roomid) in _watcher_rooms + with _rooms_lock: + for site, roomid in _command_rooms: + with _clients_lock: + room = _clients[site].get_room(roomid) + deletion_watcher = (site, roomid) in _watcher_rooms - room.join() - room.watch_socket(on_msg) - _rooms[(site, roomid)] = RoomData(room, -1, deletion_watcher) + room.join() + room.watch_socket(on_msg) + _rooms[(site, roomid)] = RoomData(room, -1, deletion_watcher) def parse_room_config(path): @@ -172,55 +194,57 @@ def parse_room_config(path): rooms = {} host_fields = {'stackexchange.com': 1, 'meta.stackexchange.com': 2, 'stackoverflow.com': 3} - for site, site_rooms in room_dict.items(): - for roomid, room in site_rooms.items(): - room_identifier = (site, roomid) - # print("Process {}".format(room_identifier)) - rooms[room_identifier] = room - if "privileges" in room and "inherit" in room["privileges"]: - inherits.append({'from': (room["privileges"]["inherit"]["site"], - room["privileges"]["inherit"]["room"]), 'to': room_identifier}) - if "additional" in room["privileges"]: - _privileges[room_identifier] =\ - set([user_data[x][host_fields[site]] for x in room["privileges"]["additional"]]) - elif "privileges" in room: - _privileges[room_identifier] = set([user_data[x][host_fields[site]] for x in room["privileges"]]) - else: - _privileges[room_identifier] = set() - - if "commands" in room and room["commands"]: - _command_rooms.add(room_identifier) - - if "watcher" in room and room["watcher"]: - _watcher_rooms.add(room_identifier) - - if "msg_types" in room: - add_room(room_identifier, room["msg_types"]) - - for inherit in inherits: - if inherit["from"] in rooms: - from_privs = _privileges[inherit["from"]] - from_accounts = [k for k, v in user_data.items() if v[host_fields[inherit["from"][0]]] in from_privs] - inherit_from = set([user_data[x][host_fields[inherit["to"][0]]] for x in from_accounts]) - - if inherit["to"] in _privileges: - before = _privileges[inherit["to"]] - _privileges[inherit["to"]] = _privileges[inherit["to"]] | inherit_from - # log('debug', '{} inheriting privs from {} with additional: before {}, after {}'.format( - # inherit["to"], inherit["from"], before, _privileges[inherit["to"]])) + with _privileges_lock, _rooms_lock: + for site, site_rooms in room_dict.items(): + for roomid, room in site_rooms.items(): + room_identifier = (site, roomid) + # print("Process {}".format(room_identifier)) + rooms[room_identifier] = room + if "privileges" in room and "inherit" in room["privileges"]: + inherits.append({'from': (room["privileges"]["inherit"]["site"], + room["privileges"]["inherit"]["room"]), 'to': room_identifier}) + if "additional" in room["privileges"]: + _privileges[room_identifier] =\ + set([user_data[x][host_fields[site]] for x in room["privileges"]["additional"]]) + elif "privileges" in room: + _privileges[room_identifier] = set([user_data[x][host_fields[site]] for x in room["privileges"]]) + else: + _privileges[room_identifier] = set() + + if "commands" in room and room["commands"]: + _command_rooms.add(room_identifier) + + if "watcher" in room and room["watcher"]: + _watcher_rooms.add(room_identifier) + + if "msg_types" in room: + add_room(room_identifier, room["msg_types"]) + + for inherit in inherits: + if inherit["from"] in rooms: + from_privs = _privileges[inherit["from"]] + from_accounts = [k for k, v in user_data.items() if v[host_fields[inherit["from"][0]]] in from_privs] + inherit_from = set([user_data[x][host_fields[inherit["to"][0]]] for x in from_accounts]) + + if inherit["to"] in _privileges: + before = _privileges[inherit["to"]] + _privileges[inherit["to"]] = _privileges[inherit["to"]] | inherit_from + # log('debug', '{} inheriting privs from {} with additional: before {}, after {}'.format( + # inherit["to"], inherit["from"], before, _privileges[inherit["to"]])) + else: + _privileges[inherit["to"]] = inherit_from else: - _privileges[inherit["to"]] = inherit_from - else: - log('warn', 'Room {} on {} specified privilege inheritance from {}, but no such room exists'.format( - inherit["to"][1], inherit["to"][1], inherit["from"][1])) + log('warn', 'Room {} on {} specified privilege inheritance from {}, but no such room exists'.format( + inherit["to"][1], inherit["to"][1], inherit["from"][1])) def add_room(room, roles): - for role in roles: - if role not in _room_roles: - _room_roles[role] = set() + with _room_roles_lock: + for role in roles: + if role not in _room_roles: + _room_roles[role] = set() - _room_roles[role].add(room) + _room_roles[role].add(room) def pickle_last_messages(): @@ -228,7 +252,9 @@ def pickle_last_messages(): _pickle_run.wait() _pickle_run.clear() - datahandling.dump_pickle("messageData.p", _last_messages) + with _last_messages_lock: + last_messages_copy = copy.deepcopy(_last_messages) + datahandling.dump_pickle("messageData.p", last_messages_copy) def send_messages(): @@ -249,21 +275,23 @@ def send_messages(): identifier = (room.room._client.host, room.room.id) message_id = response["id"] - if identifier not in _last_messages.messages: - _last_messages.messages[identifier] = collections.deque((message_id,)) - else: - last = _last_messages.messages[identifier] + with _last_messages_lock: + if identifier not in _last_messages.messages: + _last_messages.messages[identifier] = collections.deque((message_id,)) + else: + last = _last_messages.messages[identifier] - if len(last) > 100: - last.popleft() + if len(last) > 100: + last.popleft() - last.append(message_id) + last.append(message_id) if report_data: - _last_messages.reports[(room.room._client.host, message_id)] = report_data + with _last_messages_lock: + _last_messages.reports[(room.room._client.host, message_id)] = report_data - if len(_last_messages.reports) > 50: - _last_messages.reports.popitem(last=False) + if len(_last_messages.reports) > 50: + _last_messages.reports.popitem(last=False) if room.deletion_watcher: callback = room.room._client.get_message(message_id).delete @@ -279,6 +307,17 @@ def send_messages(): _msg_queue.task_done() +def send_reply_if_not_blank(room_ident, reply_to_id, message): + if message: + send_reply(room_ident, reply_to_id, message) + + +def send_reply(room_ident, reply_to_id, message): + s = ":{}\n{}" if "\n" not in message and len(message) >= 488 else ":{} {}" + with _rooms_lock: + _msg_queue.put((_rooms[room_ident], s.format(reply_to_id, message), None)) + + def on_msg(msg, client): global _room_roles @@ -287,13 +326,13 @@ def on_msg(msg, client): message = msg.message room_ident = (client.host, message.room.id) - room_data = _rooms[room_ident] - if message.owner.id == client._br.user_id: - if 'direct' in _room_roles and room_ident in _room_roles['direct']: - SocketScience.receive(message.content_source.replace("\u200B", "").replace("\u200C", "")) + with _room_roles_lock: + if message.owner.id == client._br.user_id: + if 'direct' in _room_roles and room_ident in _room_roles['direct']: + SocketScience.receive(message.content_source.replace("\u200B", "").replace("\u200C", "")) - return + return if message.content.startswith("
"): message.content = message.content[21:] @@ -307,32 +346,25 @@ def on_msg(msg, client): cmd = GlobalVars.parser.unescape(strip_mention) result = dispatch_reply_command(message.parent, message, cmd) - - if result: - s = ":{}\n{}" if "\n" not in result and len(result) >= 488 else ":{} {}" - _msg_queue.put((room_data, s.format(message.id, result), None)) + send_reply_if_not_blank(room_ident, message.id, result) except ValueError: pass elif message.content.lower().startswith("sd "): result = dispatch_shorthand_command(message) - - if result: - s = ":{}\n{}" if "\n" not in result and len(result) >= 488 else ":{} {}" - _msg_queue.put((room_data, s.format(message.id, result), None)) + send_reply_if_not_blank(room_ident, message.id, result) elif message.content.startswith("!!/") or message.content.lower().startswith("sdc "): result = dispatch_command(message) - - if result: - s = ":{}\n{}" if "\n" not in result and len(result) >= 488 else ":{} {}" - _msg_queue.put((room_data, s.format(message.id, result), None)) + send_reply_if_not_blank(room_ident, message.id, result) elif classes.feedback.FEEDBACK_REGEX.search(message.content) \ and is_privileged(message.owner, message.room) and datahandling.last_feedbacked: ids, expires_in = datahandling.last_feedbacked if time.time() < expires_in: Tasks.do(metasmoke.Metasmoke.post_auto_comment, message.content_source, message.owner, ids=ids) - elif 'direct' in _room_roles and room_ident in _room_roles['direct']: - SocketScience.receive(message.content_source.replace("\u200B", "").replace("\u200C", "")) + else: + with _room_roles_lock: + if 'direct' in _room_roles and room_ident in _room_roles['direct']: + SocketScience.receive(message.content_source.replace("\u200B", "").replace("\u200C", "")) def tell_rooms_with(prop, msg, notify_site="", report_data=None): @@ -350,96 +382,107 @@ def tell_rooms(msg, has, hasnt, notify_site="", report_data=None): msg = redact_passwords(msg) target_rooms = set() - # Go through the list of properties in "has" and add all rooms which have any of those properties - # to the target_rooms set. _room_roles contains a list of rooms for each property. - for prop_has in has: - if isinstance(prop_has, tuple): - # If the current prop_has is a tuple, then it's assumed to be a descriptor of a specific room. - # The format is: (_client.host, room.id) - target_rooms.add(prop_has) + with _room_roles_lock, _rooms_lock: + # Go through the list of properties in "has" and add all rooms which have any of those properties + # to the target_rooms set. _room_roles contains a list of rooms for each property. + for prop_has in has: + if isinstance(prop_has, tuple): + # If the current prop_has is a tuple, then it's assumed to be a descriptor of a specific room. + # The format is: (_client.host, room.id) + target_rooms.add(prop_has) - if prop_has not in _room_roles: - # No rooms have this property. - continue + if prop_has not in _room_roles: + # No rooms have this property. + continue - for room in _room_roles[prop_has]: - if all(map(lambda prop: prop not in _room_roles or room not in _room_roles[prop], hasnt)): - if room not in _rooms: - # If SD is not already in the room, then join the room. - site, roomid = room - deletion_watcher = room in _watcher_rooms + for room in _room_roles[prop_has]: + if all(map(lambda prop: prop not in _room_roles or room not in _room_roles[prop], hasnt)): + if room not in _rooms: + # If SD is not already in the room, then join the room. + site, roomid = room + deletion_watcher = room in _watcher_rooms - new_room = _clients[site].get_room(roomid) - new_room.join() + with _clients_lock: + new_room = _clients[site].get_room(roomid) + new_room.join() - _rooms[room] = RoomData(new_room, -1, deletion_watcher) + _rooms[room] = RoomData(new_room, -1, deletion_watcher) - target_rooms.add(room) + target_rooms.add(room) - for room_id in target_rooms: - room = _rooms[room_id] + for room_id in target_rooms: + room = _rooms[room_id] - if notify_site: - pings = datahandling.get_user_names_on_notification_list(room.room._client.host, - room.room.id, - notify_site, - room.room._client) + if notify_site: + pings = datahandling.get_user_names_on_notification_list(room.room._client.host, + room.room.id, + notify_site, + room.room._client) - msg_pings = datahandling.append_pings(msg, pings) - else: - msg_pings = msg + msg_pings = datahandling.append_pings(msg, pings) + else: + msg_pings = msg - timestamp = time.time() + timestamp = time.time() - if room.block_time < timestamp and _global_block < timestamp: - if report_data and "delay" in _room_roles and room_id in _room_roles["delay"]: - def callback(room=room, msg=msg_pings): - post = fetch_post_id_and_site_from_url(report_data[0])[0:2] + with _global_block_lock: + is_global_block_before_timestamp = _global_block < timestamp + if room.block_time < timestamp and is_global_block_before_timestamp: + if report_data and "delay" in _room_roles and room_id in _room_roles["delay"]: + def callback(room=room, msg=msg_pings): + post = fetch_post_id_and_site_from_url(report_data[0])[0:2] - if not datahandling.is_false_positive(post) and not datahandling.is_ignored_post(post): - _msg_queue.put((room, msg, report_data)) + if not datahandling.is_false_positive(post) and not datahandling.is_ignored_post(post): + _msg_queue.put((room, msg, report_data)) - task = Tasks.later(callback, after=300) + task = Tasks.later(callback, after=300) - GlobalVars.deletion_watcher.subscribe(report_data[0], callback=task.cancel) - else: - _msg_queue.put((room, msg_pings, report_data)) + GlobalVars.deletion_watcher.subscribe(report_data[0], callback=task.cancel) + else: + _msg_queue.put((room, msg_pings, report_data)) def get_last_messages(room, count): identifier = (room._client.host, room.id) - if identifier not in _last_messages.messages: - return + with _last_messages_lock: + if identifier not in _last_messages.messages: + return - for msg_id in itertools.islice(reversed(_last_messages.messages[identifier]), count): + last_messages_identifier_copy = copy.deepcopy(_last_messages.messages[identifier]) + + for msg_id in itertools.islice(reversed(last_messages_identifier_copy), count): yield room._client.get_message(msg_id) def get_report_data(message): identifier = (message._client.host, message.id) - if identifier in _last_messages.reports: - return _last_messages.reports[identifier] - else: - post_url = fetch_post_url_from_msg_content(message.content_source) + with _last_messages_lock: + if identifier in _last_messages.reports: + return _last_messages.reports[identifier] + else: + post_url = fetch_post_url_from_msg_content(message.content_source) - if post_url: - return (post_url, fetch_owner_url_from_msg_content(message.content_source)) + if post_url: + return (post_url, fetch_owner_url_from_msg_content(message.content_source)) def is_privileged(user, room): - # print(_privileges) - return user.id in _privileges[(room._client.host, room.id)] or user.is_moderator + with _privileges_lock: + # print(_privileges) + return user.id in _privileges[(room._client.host, room.id)] or user.is_moderator def block_room(room_id, site, time): global _global_block if room_id is None: - _global_block = time + with _global_block_lock: + _global_block = time else: - _rooms[(site, room_id)].block_time = time + with _rooms_lock: + _rooms[(site, room_id)].block_time = time class ChatCommand: @@ -458,8 +501,9 @@ def __call__(self, *args, original_msg=None, alias_used=None, quiet_action=False disable_key = "no-" + self.__func__.__name__ try: room_identifier = (original_msg.room._client.host, original_msg.room.id) - if disable_key in _room_roles and room_identifier in _room_roles[disable_key]: - return "This command is disabled in this room" + with _room_roles_lock: + if disable_key in _room_roles and room_identifier in _room_roles[disable_key]: + return "This command is disabled in this room" except AttributeError: # Test cases in CI don't contain enough data pass @@ -510,15 +554,17 @@ def decorator(func): cmd = (f, arity if arity else (len(type_signature), len(type_signature))) if reply: - _reply_commands[func.__name__.replace('_', '-')] = cmd + with _reply_commands_lock: + _reply_commands[func.__name__.replace('_', '-')] = cmd - for alias in aliases: - _reply_commands[alias] = cmd + for alias in aliases: + _reply_commands[alias] = cmd else: - _prefix_commands[func.__name__.replace("_", "-")] = cmd + with _prefix_commands_lock: + _prefix_commands[func.__name__.replace("_", "-")] = cmd - for alias in aliases: - _prefix_commands[alias] = cmd + for alias in aliases: + _prefix_commands[alias] = cmd return f @@ -531,9 +577,10 @@ def message(msg): def get_message(id, host="stackexchange.com"): - if host not in _clients: - raise ValueError("Invalid host") - return _clients[host].get_message(int(id)) + with _clients_lock: + if host not in _clients: + raise ValueError("Invalid host") + return _clients[host].get_message(int(id)) def dispatch_command(msg): @@ -561,11 +608,15 @@ def dispatch_command(msg): quiet_action = command_name[-1] == "-" command_name = regex.sub(r"[[:punct:]]*$", "", command_name).replace("_", "-") - if command_name not in _prefix_commands: + with _prefix_commands_lock: + is_command_name_in_prefix_commands = command_name in _prefix_commands + + if not is_command_name_in_prefix_commands: return "No such command '{}'.".format(command_name) else: log('debug', 'Command received: ' + msg.content) - func, (min_arity, max_arity) = _prefix_commands[command_name] + with _prefix_commands_lock: + func, (min_arity, max_arity) = _prefix_commands[command_name] if max_arity == 0: return func(original_msg=msg, alias_used=command_name, quiet_action=quiet_action) @@ -600,9 +651,11 @@ def dispatch_reply_command(msg, reply, full_cmd, comment=True): quiet_action = cmd[-1] == "-" cmd = regex.sub(r"\W*$", "", cmd) - if cmd in _reply_commands: - func, (min_arity, max_arity) = _reply_commands[cmd] + with _reply_commands_lock: + if cmd in _reply_commands: + func, (min_arity, max_arity) = _reply_commands[cmd] + if func: assert min_arity == 1 if max_arity == 1: diff --git a/findspam.py b/findspam.py index c420c022b9..71503c7bb2 100644 --- a/findspam.py +++ b/findspam.py @@ -1841,7 +1841,8 @@ def luncheon_meat(s, site): # Random "signature" like asdfghjkl @create_rule("himalayan pink salt detected", whole_post=True, disabled=True) def turkey2(post): if regex.search("([01]{8}|zoe)", post.body): - pingable = chatcommunicate._clients["stackexchange.com"].get_room(11540).get_pingable_user_names() + with chatcommunicate._clients_lock: + pingable = chatcommunicate._clients["stackexchange.com"].get_room(11540).get_pingable_user_names() if not pingable or not isinstance(pingable, list): return False, False, False, "" diff --git a/test/test_chatcommunicate.py b/test/test_chatcommunicate.py index 4862e920db..524b85f104 100644 --- a/test/test_chatcommunicate.py +++ b/test/test_chatcommunicate.py @@ -46,39 +46,40 @@ def test_validate_yaml(): def test_parse_room_config(): - chatcommunicate.parse_room_config("test/test_rooms.yml") - - assert ("stackexchange.com", 11540) in chatcommunicate._command_rooms - assert ("stackexchange.com", 30332) in chatcommunicate._command_rooms - assert ("stackoverflow.com", 111347) in chatcommunicate._command_rooms - - assert ("stackexchange.com", 3) not in chatcommunicate._command_rooms - assert ("stackexchange.com", 54445) not in chatcommunicate._command_rooms - assert ("meta.stackexchange.com", 89) not in chatcommunicate._command_rooms - - assert ("stackexchange.com", 11540) in chatcommunicate._watcher_rooms - assert ("stackexchange.com", 3) in chatcommunicate._watcher_rooms - assert ("meta.stackexchange.com", 89) in chatcommunicate._watcher_rooms - - assert ("stackexchange.com", 30332) not in chatcommunicate._watcher_rooms - assert ("stackexchange.com", 54445) not in chatcommunicate._watcher_rooms - assert ("stackoverflow.com", 111347) not in chatcommunicate._watcher_rooms - - assert chatcommunicate._privileges[("stackexchange.com", 11540)] == {121520, 10145} - assert chatcommunicate._privileges[("stackexchange.com", 30332)] == {121520, 10145} - assert chatcommunicate._privileges[("stackexchange.com", 3)] == set() - assert chatcommunicate._privileges[("stackexchange.com", 54445)] == set() - assert chatcommunicate._privileges[("meta.stackexchange.com", 89)] == {262823} - assert chatcommunicate._privileges[("stackoverflow.com", 111347)] == {3160466, 603346} - - assert len(chatcommunicate._room_roles) == 5 - assert chatcommunicate._room_roles["debug"] == {("stackexchange.com", 11540)} - assert chatcommunicate._room_roles["all"] == {("stackexchange.com", 11540), - ("stackexchange.com", 54445), - ("stackoverflow.com", 111347)} - assert chatcommunicate._room_roles["metatavern"] == {("meta.stackexchange.com", 89)} - assert chatcommunicate._room_roles["delay"] == {("meta.stackexchange.com", 89)} - assert chatcommunicate._room_roles["no-all-caps title"] == {("meta.stackexchange.com", 89)} + with chatcommunicate._rooms_lock, chatcommunicate._privileges_lock, chatcommunicate._room_roles_lock: + chatcommunicate.parse_room_config("test/test_rooms.yml") + + assert ("stackexchange.com", 11540) in chatcommunicate._command_rooms + assert ("stackexchange.com", 30332) in chatcommunicate._command_rooms + assert ("stackoverflow.com", 111347) in chatcommunicate._command_rooms + + assert ("stackexchange.com", 3) not in chatcommunicate._command_rooms + assert ("stackexchange.com", 54445) not in chatcommunicate._command_rooms + assert ("meta.stackexchange.com", 89) not in chatcommunicate._command_rooms + + assert ("stackexchange.com", 11540) in chatcommunicate._watcher_rooms + assert ("stackexchange.com", 3) in chatcommunicate._watcher_rooms + assert ("meta.stackexchange.com", 89) in chatcommunicate._watcher_rooms + + assert ("stackexchange.com", 30332) not in chatcommunicate._watcher_rooms + assert ("stackexchange.com", 54445) not in chatcommunicate._watcher_rooms + assert ("stackoverflow.com", 111347) not in chatcommunicate._watcher_rooms + + assert chatcommunicate._privileges[("stackexchange.com", 11540)] == {121520, 10145} + assert chatcommunicate._privileges[("stackexchange.com", 30332)] == {121520, 10145} + assert chatcommunicate._privileges[("stackexchange.com", 3)] == set() + assert chatcommunicate._privileges[("stackexchange.com", 54445)] == set() + assert chatcommunicate._privileges[("meta.stackexchange.com", 89)] == {262823} + assert chatcommunicate._privileges[("stackoverflow.com", 111347)] == {3160466, 603346} + + assert len(chatcommunicate._room_roles) == 5 + assert chatcommunicate._room_roles["debug"] == {("stackexchange.com", 11540)} + assert chatcommunicate._room_roles["all"] == {("stackexchange.com", 11540), + ("stackexchange.com", 54445), + ("stackoverflow.com", 111347)} + assert chatcommunicate._room_roles["metatavern"] == {("meta.stackexchange.com", 89)} + assert chatcommunicate._room_roles["delay"] == {("meta.stackexchange.com", 89)} + assert chatcommunicate._room_roles["no-all-caps title"] == {("meta.stackexchange.com", 89)} @patch("chatcommunicate.threading.Thread") @@ -93,7 +94,8 @@ def test_init(room_config, client_constructor, thread): # https://stackoverflow.com/questions/23337471/ with pytest.raises(Exception) as e: chatcommunicate.init("shoutouts", "to simpleflips", try_cookies=False) - assert str(e.value).endswith("Failed to log into {}, max retries exceeded".format(next(iter(chatcommunicate._clients)))) + with chatcommunicate._clients_lock: + assert str(e.value).endswith("Failed to log into {}, max retries exceeded".format(next(iter(chatcommunicate._clients)))) client.login.side_effect = None client.login.reset_mock() @@ -107,7 +109,8 @@ def test_init(room_config, client_constructor, thread): except Exception: return # This interferes with the following tests - assert len(chatcommunicate._rooms) == 0 + with chatcommunicate._rooms_lock: + assert len(chatcommunicate._rooms) == 0 assert client_constructor.call_count == 3 client_constructor.assert_any_call("stackexchange.com") @@ -149,10 +152,11 @@ def throw_every_other(*_): thread.assert_any_call(name="pickle ---rick--- runner", target=chatcommunicate.pickle_last_messages, daemon=True) thread.assert_any_call(name="message sender", target=chatcommunicate.send_messages, daemon=True) - assert len(chatcommunicate._rooms) == 3 - assert chatcommunicate._rooms[("stackexchange.com", 11540)].deletion_watcher is True - assert chatcommunicate._rooms[("stackexchange.com", 30332)].deletion_watcher is False - assert chatcommunicate._rooms[("stackoverflow.com", 111347)].deletion_watcher is False + with chatcommunicate._rooms_lock: + assert len(chatcommunicate._rooms) == 3 + assert chatcommunicate._rooms[("stackexchange.com", 11540)].deletion_watcher is True + assert chatcommunicate._rooms[("stackexchange.com", 30332)].deletion_watcher is False + assert chatcommunicate._rooms[("stackoverflow.com", 111347)].deletion_watcher is False @pytest.mark.skipif(has_pickle("messageData.p"), reason="shouldn't overwrite file") @@ -205,252 +209,256 @@ def test_message_sender(pickle_rick): time.sleep(0) room.room._client._do_action_despite_throttling.assert_called_once_with(("send", 30332, "test")) - assert chatcommunicate._last_messages.messages[("stackexchange.com", 11540)] == collections.deque((1,)) - assert chatcommunicate._last_messages.reports == collections.OrderedDict({("stackexchange.com", 2): "did you hear about what happened to pluto"}) + with chatcommunicate._last_messages_lock: + assert chatcommunicate._last_messages.messages[("stackexchange.com", 11540)] == collections.deque((1,)) + assert chatcommunicate._last_messages.reports == collections.OrderedDict({("stackexchange.com", 2): "did you hear about what happened to pluto"}) @patch("chatcommunicate._msg_queue.put") @patch("chatcommunicate.get_last_messages") def test_on_msg(get_last_messages, post_msg): - client = Fake({ - "_br": { - "user_id": 1337 - }, + # This is loading fake room data into chatcommunicate._rooms, so we need to keep chatcommunicate._rooms_lock for the entire test. + # It's also mocking chatcommunicate._prefix_commands and chatcommunicate._reply_commands. + with chatcommunicate._rooms_lock, chatcommunicate._prefix_commands_lock, chatcommunicate._reply_commands_lock: + client = Fake({ + "_br": { + "user_id": 1337 + }, - "host": "stackexchange.com" - }) + "host": "stackexchange.com" + }) - room_data = chatcommunicate.RoomData(Mock(), -1, False) - chatcommunicate._rooms[("stackexchange.com", 11540)] = room_data + room_data = chatcommunicate.RoomData(Mock(), -1, False) + chatcommunicate._rooms[("stackexchange.com", 11540)] = room_data - chatcommunicate.on_msg(Fake({}, spec=chatcommunicate.events.MessageStarred), None) # don't reply to events we don't care about + chatcommunicate.on_msg(Fake({}, spec=chatcommunicate.events.MessageStarred), None) # don't reply to events we don't care about - msg1 = Fake({ - "message": { - "room": { - "id": 11540, - }, + msg1 = Fake({ + "message": { + "room": { + "id": 11540, + }, - "owner": { - "id": 1, - }, + "owner": { + "id": 1, + }, - "parent": None, - "content": "shoutouts to simpleflips" - } - }, spec=chatcommunicate.events.MessagePosted) + "parent": None, + "content": "shoutouts to simpleflips" + } + }, spec=chatcommunicate.events.MessagePosted) - chatcommunicate.on_msg(msg1, client) + chatcommunicate.on_msg(msg1, client) - msg2 = Fake({ - "message": { - "room": { - "id": 11540 - }, + msg2 = Fake({ + "message": { + "room": { + "id": 11540 + }, - "owner": { - "id": 1337 - }, - - "id": 999, - "parent": None, - "content": "!!/not_actually_a_command" - } - }, spec=chatcommunicate.events.MessagePosted) + "owner": { + "id": 1337 + }, - chatcommunicate.on_msg(msg2, client) + "id": 999, + "parent": None, + "content": "!!/not_actually_a_command" + } + }, spec=chatcommunicate.events.MessagePosted) - msg3 = Fake({ - "message": { - "room": { - "id": 11540, - }, + chatcommunicate.on_msg(msg2, client) - "owner": { - "id": 1 - }, + msg3 = Fake({ + "message": { + "room": { + "id": 11540, + }, - "id": 999, - "parent": None, - "content": "!!/a_command" - } - }, spec=chatcommunicate.events.MessagePosted) + "owner": { + "id": 1 + }, - mock_command = Mock(side_effect=lambda *_, **kwargs: "hi" if not kwargs["quiet_action"] else "") - chatcommunicate._prefix_commands["a-command"] = (mock_command, (0, 0)) + "id": 999, + "parent": None, + "content": "!!/a_command" + } + }, spec=chatcommunicate.events.MessagePosted) - chatcommunicate.on_msg(msg3, client) + mock_command = Mock(side_effect=lambda *_, **kwargs: "hi" if not kwargs["quiet_action"] else "") + chatcommunicate._prefix_commands["a-command"] = (mock_command, (0, 0)) - assert post_msg.call_count == 1 - assert post_msg.call_args_list[0][0][0][1] == ":999 hi" - mock_command.assert_called_once_with(original_msg=msg3.message, alias_used="a-command", quiet_action=False) + chatcommunicate.on_msg(msg3, client) - post_msg.reset_mock() - mock_command.reset_mock() + assert post_msg.call_count == 1 + assert post_msg.call_args_list[0][0][0][1] == ":999 hi" + mock_command.assert_called_once_with(original_msg=msg3.message, alias_used="a-command", quiet_action=False) - msg3.message.content = "!!/a_command-" - chatcommunicate.on_msg(msg3, client) + post_msg.reset_mock() + mock_command.reset_mock() - post_msg.assert_not_called() - mock_command.assert_called_once_with(original_msg=msg3.message, alias_used="a-command", quiet_action=True) + msg3.message.content = "!!/a_command-" + chatcommunicate.on_msg(msg3, client) - post_msg.reset_mock() - mock_command.reset_mock() + post_msg.assert_not_called() + mock_command.assert_called_once_with(original_msg=msg3.message, alias_used="a-command", quiet_action=True) - chatcommunicate._prefix_commands["a-command"] = (mock_command, (0, 1)) - chatcommunicate.on_msg(msg3, client) + post_msg.reset_mock() + mock_command.reset_mock() - post_msg.assert_not_called() - mock_command.assert_called_once_with(None, original_msg=msg3.message, alias_used="a-command", quiet_action=True) + chatcommunicate._prefix_commands["a-command"] = (mock_command, (0, 1)) + chatcommunicate.on_msg(msg3, client) - post_msg.reset_mock() - mock_command.reset_mock() + post_msg.assert_not_called() + mock_command.assert_called_once_with(None, original_msg=msg3.message, alias_used="a-command", quiet_action=True) - msg3.message.content = "!!/a-command 1 2 3" - chatcommunicate.on_msg(msg3, client) + post_msg.reset_mock() + mock_command.reset_mock() - assert post_msg.call_count == 1 - assert post_msg.call_args_list[0][0][0][1] == ":999 hi" - mock_command.assert_called_once_with("1 2 3", original_msg=msg3.message, alias_used="a-command", quiet_action=False) + msg3.message.content = "!!/a-command 1 2 3" + chatcommunicate.on_msg(msg3, client) - post_msg.reset_mock() - mock_command.reset_mock() + assert post_msg.call_count == 1 + assert post_msg.call_args_list[0][0][0][1] == ":999 hi" + mock_command.assert_called_once_with("1 2 3", original_msg=msg3.message, alias_used="a-command", quiet_action=False) - chatcommunicate._prefix_commands["a-command"] = (mock_command, (1, 2)) + post_msg.reset_mock() + mock_command.reset_mock() - msg3.message.content = "!!/a-command" - chatcommunicate.on_msg(msg3, client) + chatcommunicate._prefix_commands["a-command"] = (mock_command, (1, 2)) - assert post_msg.call_count == 1 - assert post_msg.call_args_list[0][0][0][1] == ":999 Too few arguments." - mock_command.assert_not_called() + msg3.message.content = "!!/a-command" + chatcommunicate.on_msg(msg3, client) - post_msg.reset_mock() - mock_command.reset_mock() + assert post_msg.call_count == 1 + assert post_msg.call_args_list[0][0][0][1] == ":999 Too few arguments." + mock_command.assert_not_called() - msg3.message.content = "!!/a-command 1 2 oatmeal" - chatcommunicate.on_msg(msg3, client) + post_msg.reset_mock() + mock_command.reset_mock() - assert post_msg.call_count == 1 - assert post_msg.call_args_list[0][0][0][1] == ":999 Too many arguments." - mock_command.assert_not_called() + msg3.message.content = "!!/a-command 1 2 oatmeal" + chatcommunicate.on_msg(msg3, client) - post_msg.reset_mock() - mock_command.reset_mock() + assert post_msg.call_count == 1 + assert post_msg.call_args_list[0][0][0][1] == ":999 Too many arguments." + mock_command.assert_not_called() - msg3.message.content = "!!/a-command- 1 2" - chatcommunicate.on_msg(msg3, client) + post_msg.reset_mock() + mock_command.reset_mock() - post_msg.assert_not_called() - mock_command.assert_called_once_with("1", "2", original_msg=msg3.message, alias_used="a-command", quiet_action=True) + msg3.message.content = "!!/a-command- 1 2" + chatcommunicate.on_msg(msg3, client) - post_msg.reset_mock() - mock_command.reset_mock() + post_msg.assert_not_called() + mock_command.assert_called_once_with("1", "2", original_msg=msg3.message, alias_used="a-command", quiet_action=True) - msg3.message.content = "!!/a-command 3" - chatcommunicate.on_msg(msg3, client) + post_msg.reset_mock() + mock_command.reset_mock() - assert post_msg.call_count == 1 - assert post_msg.call_args_list[0][0][0][1] == ":999 hi" - mock_command.assert_called_once_with("3", None, original_msg=msg3.message, alias_used="a-command", quiet_action=False) + msg3.message.content = "!!/a-command 3" + chatcommunicate.on_msg(msg3, client) - post_msg.reset_mock() - mock_command.reset_mock() + assert post_msg.call_count == 1 + assert post_msg.call_args_list[0][0][0][1] == ":999 hi" + mock_command.assert_called_once_with("3", None, original_msg=msg3.message, alias_used="a-command", quiet_action=False) - msg4 = Fake({ - "message": { - "room": { - "id": 11540, - }, + post_msg.reset_mock() + mock_command.reset_mock() - "owner": { - "id": 1 - }, + msg4 = Fake({ + "message": { + "room": { + "id": 11540, + }, - "parent": { "owner": { - "id": 2 - } - }, + "id": 1 + }, - "id": 1000, - "content": "asdf" - } - }, spec=chatcommunicate.events.MessageEdited) + "parent": { + "owner": { + "id": 2 + } + }, - chatcommunicate.on_msg(msg4, client) + "id": 1000, + "content": "asdf" + } + }, spec=chatcommunicate.events.MessageEdited) - msg5 = Fake({ - "message": { - "room": { - "id": 11540, - }, + chatcommunicate.on_msg(msg4, client) - "owner": { - "id": 1 - }, + msg5 = Fake({ + "message": { + "room": { + "id": 11540, + }, - "parent": { "owner": { - "id": 1337 - } - }, + "id": 1 + }, - "id": 1000, - "content": "@SmokeDetector why " - } - }, spec=chatcommunicate.events.MessageEdited) + "parent": { + "owner": { + "id": 1337 + } + }, - chatcommunicate._reply_commands["why"] = (mock_command, (0, 0)) + "id": 1000, + "content": "@SmokeDetector why " + } + }, spec=chatcommunicate.events.MessageEdited) - threw_exception = False + chatcommunicate._reply_commands["why"] = (mock_command, (0, 0)) - try: - chatcommunicate.on_msg(msg5, client) - except AssertionError: - threw_exception = True + threw_exception = False - assert threw_exception - mock_command.assert_not_called() - post_msg.assert_not_called() + try: + chatcommunicate.on_msg(msg5, client) + except AssertionError: + threw_exception = True - chatcommunicate._reply_commands["why"] = (mock_command, (1, 1)) - chatcommunicate.on_msg(msg5, client) + assert threw_exception + mock_command.assert_not_called() + post_msg.assert_not_called() - assert post_msg.call_count == 1 - assert post_msg.call_args_list[0][0][0][1] == ":1000 hi" - mock_command.assert_called_once_with(msg5.message.parent, original_msg=msg5.message, alias_used="why", quiet_action=False) + chatcommunicate._reply_commands["why"] = (mock_command, (1, 1)) + chatcommunicate.on_msg(msg5, client) - post_msg.reset_mock() - mock_command.reset_mock() + assert post_msg.call_count == 1 + assert post_msg.call_args_list[0][0][0][1] == ":1000 hi" + mock_command.assert_called_once_with(msg5.message.parent, original_msg=msg5.message, alias_used="why", quiet_action=False) - msg5.message.content = "@SmokeDetector why@!@#-" - chatcommunicate.on_msg(msg5, client) + post_msg.reset_mock() + mock_command.reset_mock() - post_msg.assert_not_called() - mock_command.assert_called_once_with(msg5.message.parent, original_msg=msg5.message, alias_used="why", quiet_action=True) + msg5.message.content = "@SmokeDetector why@!@#-" + chatcommunicate.on_msg(msg5, client) - msg6 = Fake({ - "message": { - "room": { - "id": 11540, - }, + post_msg.assert_not_called() + mock_command.assert_called_once_with(msg5.message.parent, original_msg=msg5.message, alias_used="why", quiet_action=True) - "owner": { - "id": 1 - }, + msg6 = Fake({ + "message": { + "room": { + "id": 11540, + }, + + "owner": { + "id": 1 + }, - "id": 1000, - "parent": None, - "content": "sd why - 2why 2why- 2- why- " - } - }, spec=chatcommunicate.events.MessageEdited) + "id": 1000, + "parent": None, + "content": "sd why - 2why 2why- 2- why- " + } + }, spec=chatcommunicate.events.MessageEdited) - get_last_messages.side_effect = lambda _, num: (Fake({"id": i}) for i in range(num)) - chatcommunicate.on_msg(msg6, client) + get_last_messages.side_effect = lambda _, num: (Fake({"id": i}) for i in range(num)) + chatcommunicate.on_msg(msg6, client) - assert post_msg.call_count == 1 - assert post_msg.call_args_list[0][0][0][1] == ":1000 [:0] hi\n[:1] \n[:2] hi\n[:3] hi\n[:4] \n[:5] \n[:6] \n[:7] \n[:8] " + assert post_msg.call_count == 1 + assert post_msg.call_args_list[0][0][0][1] == ":1000 [:0] hi\n[:1] \n[:2] hi\n[:3] hi\n[:4] \n[:5] \n[:6] \n[:7] \n[:8] " def test_message_type(): diff --git a/ws.py b/ws.py index b2dd97789e..4955e9685a 100755 --- a/ws.py +++ b/ws.py @@ -217,9 +217,13 @@ def load_ms_cache_data(): # noinspection PyProtectedMember def check_socket_connections(): - for client in chatcommunicate._clients.values(): - if client.last_activity and (datetime.utcnow() - client.last_activity).total_seconds() >= 60: - exit_mode("socket_failure") + socket_failure = False + with chatcommunicate._clients_lock: + for client in chatcommunicate._clients.values(): + if client.last_activity and (datetime.utcnow() - client.last_activity).total_seconds() >= 60: + socket_failure = True + if socket_failure: + exit_mode("socket_failure") Tasks.periodic(check_socket_connections, interval=90) From 33b18519e5f636cfcd7f0cd7cd2e20b20f11c526 Mon Sep 17 00:00:00 2001 From: Makyen Date: Fri, 27 May 2022 06:50:26 -0700 Subject: [PATCH 2/3] Add wrappers to test_chatcommands.py to lock and restore values --- test/test_chatcommands.py | 55 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/test/test_chatcommands.py b/test/test_chatcommands.py index 3021e19c24..03cf38c5c8 100644 --- a/test/test_chatcommands.py +++ b/test/test_chatcommands.py @@ -24,6 +24,47 @@ from unittest.mock import patch +def rewrap_for_paramiterized_test_bisect(): + def decorator(func): + def wrap(command, test_text, expected_result): + return func(command, test_text, expected_result) + return wrap + return decorator + + +def lock_and_restore_chatcommunicate_rooms(): + def decorator(func): + def wrap(*args, **kwargs): + chatcommunicate._privileges_lock.acquire() + try: + chatcommunicate._rooms_lock.acquire() + privileges = chatcommunicate._privileges + rooms = chatcommunicate._rooms + command_rooms = chatcommunicate._command_rooms + watcher_rooms = chatcommunicate._watcher_rooms + try: + chatcommunicate._privileges = {} + chatcommunicate._rooms = {} + chatcommunicate._command_rooms = set() + chatcommunicate._watcher_rooms = set() + return func(*args, **kwargs) + except Exception: + pass + finally: + # Reset the values which were controlled under the locks + chatcommunicate._privileges = privileges + chatcommunicate._rooms = rooms + chatcommunicate._command_rooms = command_rooms + chatcommunicate._watcher_rooms = watcher_rooms + chatcommunicate._rooms_lock.release() + except Exception: + pass + finally: + chatcommunicate._privileges_lock.release() + return wrap + return decorator + + def dummy_tell_rooms_with(dummy1, dummy2): pass @@ -117,6 +158,8 @@ def test_version(): # normalized with 1 ("bisect_number", "13 bar 18167873816 bar", """Matched by `1-816-787-3816` on [line 2 of blacklisted_numbers.txt](https://github.com/{bot_repo_slug}/blob/{commit_id}/blacklisted_numbers.txt#L2): 18167873816 found normalized"""), ]) +@rewrap_for_paramiterized_test_bisect() +@lock_and_restore_chatcommunicate_rooms() def test_bisect(command, test_text, expected_result): chatcommunicate.parse_room_config("test/test_rooms.yml") msg = Fake({ @@ -193,6 +236,7 @@ def test_blame(): assert chatcommands.blame2("\u200B\u200C\u2060\u200D\u180E\uFEFF\u2063", original_msg=msg2) == "It's [J F](https://chat.stackexchange.com/users/161943)'s fault." +@lock_and_restore_chatcommunicate_rooms() def test_privileged(): chatcommunicate.parse_room_config("test/test_rooms.yml") @@ -219,12 +263,14 @@ def test_privileged(): assert chatcommands.amiprivileged(original_msg=msg) == "\u2713 You are a privileged user." +@lock_and_restore_chatcommunicate_rooms() def test_deprecated_blacklist(): chatcommunicate.parse_room_config("test/test_rooms.yml") assert chatcommands.blacklist("").startswith("The `!!/blacklist` command has been deprecated.") @pytest.mark.skipif(GlobalVars.on_branch != "master", reason="avoid branch checkout") +@lock_and_restore_chatcommunicate_rooms() def test_watch(monkeypatch): chatcommunicate.parse_room_config("test/test_rooms.yml") # XXX TODO: expand @@ -278,6 +324,7 @@ def wrap_watch(pattern, force=False): git.checkout("master") +@lock_and_restore_chatcommunicate_rooms() def test_approve(monkeypatch): chatcommunicate.parse_room_config("test/test_rooms.yml") msg = Fake({ @@ -305,6 +352,7 @@ def test_approve(monkeypatch): assert chatcommands.approve(2518, original_msg=msg)[:8] in {"PR #2518", "Cannot c"} +@lock_and_restore_chatcommunicate_rooms() def test_reject(monkeypatch): chatcommunicate.parse_room_config("test/test_rooms.yml") msg = Fake({ @@ -333,6 +381,7 @@ def test_reject(monkeypatch): @patch("chatcommands.handle_spam") +@lock_and_restore_chatcommunicate_rooms() def test_report(handle_spam): chatcommunicate.parse_room_config("test/test_rooms.yml") # Documentation: The process before scanning the post is identical regardless of alias_used. @@ -417,6 +466,7 @@ def test_report(handle_spam): @patch("chatcommands.handle_spam") +@lock_and_restore_chatcommunicate_rooms() def test_allspam(handle_spam): chatcommunicate.parse_room_config("test/test_rooms.yml") try: @@ -498,6 +548,7 @@ def test_allspam(handle_spam): @pytest.mark.skipif(os.path.isfile("blacklistedUsers.p"), reason="shouldn't overwrite file") +@lock_and_restore_chatcommunicate_rooms() def test_blacklisted_users(): chatcommunicate.parse_room_config("test/test_rooms.yml") try: @@ -570,6 +621,7 @@ def test_blacklisted_users(): @pytest.mark.skipif(os.path.isfile("whitelistedUsers.p"), reason="shouldn't overwrite file") +@lock_and_restore_chatcommunicate_rooms() def test_whitelisted_users(): chatcommunicate.parse_room_config("test/test_rooms.yml") try: @@ -638,6 +690,7 @@ def test_whitelisted_users(): remove_pickle("whitelistedUsers.p") +@lock_and_restore_chatcommunicate_rooms() def test_metasmoke(): chatcommunicate.parse_room_config("test/test_rooms.yml") orig_tell_rooms_with = chatcommunicate.tell_rooms_with @@ -670,6 +723,7 @@ def test_metasmoke(): @pytest.mark.skipif(os.path.isfile("notifications.p"), reason="shouldn't overwrite file") +@lock_and_restore_chatcommunicate_rooms() def test_notifications(): chatcommunicate.parse_room_config("test/test_rooms.yml") try: @@ -776,6 +830,7 @@ def test_notifications(): remove_pickle("notifications.p") +@lock_and_restore_chatcommunicate_rooms() def test_inqueue(): chatcommunicate.parse_room_config("test/test_rooms.yml") site = Fake({"keys": (lambda: ['1'])}) From 11c4f6bb272f55ac4457704470f799daa0f9e854 Mon Sep 17 00:00:00 2001 From: Makyen Date: Tue, 24 May 2022 18:08:23 -0700 Subject: [PATCH 3/3] Show more than one problem in test_validate_yaml() --- test/test_chatcommunicate.py | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/test/test_chatcommunicate.py b/test/test_chatcommunicate.py index 524b85f104..83e3b2fccf 100644 --- a/test/test_chatcommunicate.py +++ b/test/test_chatcommunicate.py @@ -24,7 +24,6 @@ def test_validate_yaml(): with open("users.yml", "r") as f: user_data = yaml.safe_load(f.read()) - flatten = lambda l: [item for sublist in l for item in sublist] privileged_users = [] for site, site_rooms in room_data.items(): @@ -33,16 +32,31 @@ def test_validate_yaml(): continue if "additional" in room["privileges"]: - privileged_users.append(room["privileges"]["additional"]) + privileged_users.extend(room["privileges"]["additional"]) if "inherit" not in room["privileges"]: - privileged_users.append(room["privileges"]) + privileged_users.extend(room["privileges"]) - privileged_users = set(flatten(privileged_users)) + privileged_users = set(privileged_users) + + issues = [] + + for user_key in user_data.keys(): + if type(user_key) != int: + print('type(user_key):', type(user_key)) + issues.append('user.yml user ID "{}" is not an int'.format(user_key)) + + for user_id in privileged_users: + if type(user_id) != int: + print('type(user_id):', type(user_id)) + issues.append('user.yml user ID "{}" is not an int'.format(user_key)) for uid in privileged_users: if uid not in user_data: - pytest.fail("Privileged user {} does not have a corresponding entry in users.yml".format(uid)) + issues.append("Privileged user {} does not have a corresponding entry in users.yml".format(uid)) + + if issues: + pytest.fail('\n'.join(issues)) def test_parse_room_config():