Merge pull request #4437 from esl/carboncopy-flaky-test

Unflake the`dropped_client_doesnt_create_duplicate_carbons` test

The description of #4423 was not entirely correct, and this PR should unlflake the tests for good. The reason why the tests are changed, and not the implementation is because the underlying race condition is a bit tricky to fix elegantly.

mod_carboncopy:remove_connection is a handler for the unset_presence hook, and it removes CC state from the ejabberd_sm info field in the session record. This is however done in an async manner by the C2S process, and when the user terminates, the request is never handled. This doesn't matter much, as the user process exits shortly, and is removed from ejabberd_sm altogether. However, if they receives a message in this short time, CC might run, and see the process still with the CC state in the ejabberd_sm info field. This results in a CC copy sent on behalf of this exiting process. Since a fix would be overly complicated, and to proceed with unflaking the tests, the test will now wait for C2S process exit.

big_tests/tests/carboncopy_SUITE.erl

@@ -180,13 +180,20 @@ dropped_client_doesnt_create_duplicate_carbons(Config) ->
       fun(Alice1, Alice2, Bob) ->
               enable_carbons([Alice1, Alice2]),
               Msg = escalus_stanza:chat_to(Bob, ?BODY),
+              C2SPid = mongoose_helper:get_session_pid(Alice2),
               escalus_client:stop(Config, Alice2),
               escalus:assert(is_presence_with_type, [<<"unavailable">>],
                              escalus_client:wait_for_stanza(Alice1)),
+              %% Ensure there is no session so that the test doesn't flake and fail by chance.
+              %% The issue comes from the fact that mod_presence sends the "unavailable" stanza
+              %% just before removing the is removed from ejabberd_sm. If the msg is received by
+              %% mod_carboncopy in this short window, it still considers the resource enabled.
+              %% It's a race condition, but from the user's perspective it shouldn't be a big issue.
+              mongoose_helper:wait_for_pid_to_die(C2SPid),
               escalus_client:send(Alice1, Msg),
               escalus:assert(is_chat_message, [?BODY],
                              escalus_client:wait_for_stanza(Bob)),
-              [] = escalus_client:peek_stanzas(Alice1)
+              escalus_assert:has_no_stanzas(Alice1)
       end).
 
 prop_forward_received_chat_messages(Config) ->