From 5e2c8cabc1462aa1606307b47813e23ee74949a2 Mon Sep 17 00:00:00 2001 From: jfreegman Date: Tue, 16 Jan 2024 18:08:11 -0500 Subject: [PATCH] cleanup: make some improvements to group moderation test - We no longer assert peer roles in the mod event callback because this causes an issue with the new events implementation, which triggers the events after all the packets from the current tox_iterate() are processed, rather than as the packets are received. These checks were superfluous and shouldn't reduce code coverage. - A moderator now sets the topic before the founder kicks him in order to increase internal code coverage. --- auto_tests/group_moderation_test.c | 26 ++++++++++++------- .../docker/tox-bootstrapd.sha256 | 2 +- toxcore/group_chats.c | 1 + 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/auto_tests/group_moderation_test.c b/auto_tests/group_moderation_test.c index 6f10ef76..d87491ee 100644 --- a/auto_tests/group_moderation_test.c +++ b/auto_tests/group_moderation_test.c @@ -193,7 +193,7 @@ static void group_peer_join_handler(Tox *tox, uint32_t group_number, uint32_t pe ck_assert(state->num_peers < NUM_GROUP_TOXES); } -static void handle_mod(State *state, const char *peer_name, size_t peer_name_len, Tox_Group_Role role) +static void handle_mod(State *state, const char *peer_name, size_t peer_name_len) { if (state->mod_event_count == 0) { ck_assert(memcmp(peer_name, state->mod_name1, peer_name_len) == 0); @@ -205,10 +205,9 @@ static void handle_mod(State *state, const char *peer_name, size_t peer_name_len ++state->mod_event_count; state->mod_check = true; - ck_assert(role == TOX_GROUP_ROLE_MODERATOR); } -static void handle_observer(State *state, const char *peer_name, size_t peer_name_len, Tox_Group_Role role) +static void handle_observer(State *state, const char *peer_name, size_t peer_name_len) { if (state->observer_event_count == 0) { ck_assert(memcmp(peer_name, state->observer_name1, peer_name_len) == 0); @@ -220,10 +219,9 @@ static void handle_observer(State *state, const char *peer_name, size_t peer_nam ++state->observer_event_count; state->observer_check = true; - ck_assert(role == TOX_GROUP_ROLE_OBSERVER); } -static void handle_user(State *state, const char *peer_name, size_t peer_name_len, Tox_Group_Role role) +static void handle_user(State *state, const char *peer_name, size_t peer_name_len) { // event 1: observer1 gets promoted back to user // event 2: observer2 gets promoted to moderator @@ -243,7 +241,6 @@ static void handle_user(State *state, const char *peer_name, size_t peer_name_le ++state->user_event_count; state->user_check = true; - ck_assert(role == TOX_GROUP_ROLE_USER); } static void group_mod_event_handler(Tox *tox, uint32_t group_number, uint32_t source_peer_id, uint32_t target_peer_id, @@ -274,6 +271,7 @@ static void group_mod_event_handler(Tox *tox, uint32_t group_number, uint32_t so Tox_Group_Role role = tox_group_peer_get_role(tox, group_number, target_peer_id, &q_err); ck_assert(q_err == TOX_ERR_GROUP_PEER_QUERY_OK); + ck_assert(role <= TOX_GROUP_ROLE_OBSERVER); fprintf(stderr, "tox%u: got moderator event %d (%s), role = %s\n", autotox->index, mod_type, tox_group_mod_event_to_string(mod_type), @@ -281,17 +279,17 @@ static void group_mod_event_handler(Tox *tox, uint32_t group_number, uint32_t so switch (mod_type) { case TOX_GROUP_MOD_EVENT_MODERATOR: { - handle_mod(state, peer_name, peer_name_len, role); + handle_mod(state, peer_name, peer_name_len); break; } case TOX_GROUP_MOD_EVENT_OBSERVER: { - handle_observer(state, peer_name, peer_name_len, role); + handle_observer(state, peer_name, peer_name_len); break; } case TOX_GROUP_MOD_EVENT_USER: { - handle_user(state, peer_name, peer_name_len, role); + handle_user(state, peer_name, peer_name_len); break; } @@ -607,6 +605,15 @@ static void group_moderation_test(AutoTox *autotoxes) tox_group_mod_kick_peer(tox1, state1->group_number, founder_peer_id, &k_err); ck_assert_msg(k_err != TOX_ERR_GROUP_MOD_KICK_PEER_OK, "Mod kicked founder"); + /* the moderator about to be kicked changes the topic to trigger the founder to + * re-sign and redistribute it after the kick. + */ + State *state_x = (State *)autotoxes[idx].state; + Tox *tox_x = autotoxes[idx].tox; + Tox_Err_Group_Topic_Set topic_err; + tox_group_set_topic(tox_x, state_x->group_number, nullptr, 0, &topic_err); + ck_assert(topic_err == TOX_ERR_GROUP_TOPIC_SET_OK); + /* founder kicks moderator (this triggers two events: user and kick) */ fprintf(stderr, "Founder is kicking %s\n", state0->peers[0].name); @@ -619,7 +626,6 @@ static void group_moderation_test(AutoTox *autotoxes) fprintf(stderr, "All peers successfully received kick event\n"); fprintf(stderr, "Founder is demoting moderator to user\n"); - tox_group_mod_set_role(tox0, state0->group_number, state0->peers[2].peer_id, TOX_GROUP_ROLE_USER, &role_err); ck_assert_msg(role_err == TOX_ERR_GROUP_MOD_SET_ROLE_OK, "Failed to demote peer 3 to User. error: %d", role_err); diff --git a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 index 68b6611e..79db638e 100644 --- a/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 +++ b/other/bootstrap_daemon/docker/tox-bootstrapd.sha256 @@ -1 +1 @@ -189633f3accb67886c402bf242616d9b3e8258f8050dbd00a10b7c6147ed28aa /usr/local/bin/tox-bootstrapd +030f7ea99c34523091b268df0ea8fb02e81ee340d608af85d502bace4817d6b0 /usr/local/bin/tox-bootstrapd diff --git a/toxcore/group_chats.c b/toxcore/group_chats.c index 0a80af08..021fa6c8 100644 --- a/toxcore/group_chats.c +++ b/toxcore/group_chats.c @@ -3862,6 +3862,7 @@ static bool update_gc_topic(GC_Chat *chat, const uint8_t *public_sig_key) return true; } + LOGGER_TRACE(chat->log, "founder is re-signing topic"); return gc_set_topic(chat, chat->topic_info.topic, chat->topic_info.length) == 0; }