Use the new instrumentation assertions in other test suites

- When possible, assert the exact number of events (usually one)
- When suitable, test negative conditions
- Replace custom waiting code with wait_and_assert(_new)

big_tests/tests/accounts_SUITE.erl

@@ -189,20 +189,22 @@ unregister(Config) ->
     assert_event(auth_unregister_user, escalus_users:get_jid(Config, UserSpec)).
 
 already_registered(Config) ->
-    escalus_fresh:story(Config, [{alice, 1}], fun(Alice) ->
-        escalus:send(Alice, escalus_stanza:get_registration_fields()),
-        Stanza = escalus:wait_for_stanza(Alice),
-        escalus:assert(is_iq_result, Stanza),
-        true = has_registered_element(Stanza)
+    escalus_fresh:story(Config, [{alice, 1}], fun already_registered_story/1).
+
+already_registered_story(Alice) ->
+    AliceJid = escalus_utils:get_short_jid(Alice),
+    assert_event(auth_register_user, AliceJid), % one event expected
+    escalus:send(Alice, escalus_stanza:get_registration_fields()),
+    Stanza = escalus:wait_for_stanza(Alice),
+    escalus:assert(is_iq_result, Stanza),
+    true = has_registered_element(Stanza),
+    assert_event(auth_register_user, AliceJid). % still one event - nothing new
 
-                                              end).
 registration_conflict(Config) ->
     [Alice] = escalus_users:get_users([alice]),
     {ok, result, _Stanza} = escalus_users:create_user(Config, Alice),
     {ok, conflict, _Raw} = escalus_users:create_user(Config, Alice).
 
-
-
 admin_notify(Config) ->
     [{Name1, UserSpec1}, {Name2, UserSpec2}] = escalus_users:get_users([alice, bob]),
     [{_, AdminSpec}] = escalus_users:get_users([admin]),

big_tests/tests/amp_big_SUITE.erl

@@ -133,8 +133,9 @@ amp_test_helper_code() ->
     "  end.\n".
 
 declared_events() ->
-    [
-     {mod_privacy_set, #{host_type => host_type()}} % tested by privacy helpers
+    [ % tested by privacy helpers
+     {mod_privacy_set, #{host_type => host_type()}},
+     {mod_privacy_get, #{host_type => host_type()}}
     ].
 
 end_per_suite(C) ->

big_tests/tests/anonymous_SUITE.erl

@@ -89,7 +89,7 @@ connection_is_registered_with_login(Config) ->
         true = F(),
         escalus_connection:kill(Anna),
         mongoose_helper:wait_until(F, false),
-        assert_event(auth_anonymous_register_user, JID)
+        assert_event(auth_anonymous_unregister_user, JID)
     end).
 
 messages_story(Config) ->
@@ -110,5 +110,6 @@ host_type() ->
     domain_helper:anonymous_host_type().
 
 assert_event(EventName, #jid{luser = LUser, lserver = LServer}) ->
-    instrument_helper:assert(EventName, #{host_type => host_type()},
-                             fun(M) -> M =:= #{count => 1, user => LUser, server => LServer} end).
+    instrument_helper:assert_one(
+      EventName, #{host_type => host_type()},
+      fun(M) -> M =:= #{count => 1, user => LUser, server => LServer} end).

big_tests/tests/auth_helper.erl

@@ -10,17 +10,18 @@ assert_event(EventName, BinJid)
     F = fun(M) ->
             M =:= #{count => 1, user => LUser, server => LServer}
         end,
-    instrument_helper:assert(EventName, #{host_type => host_type()}, F);
+    instrument_helper:assert_one(EventName, #{host_type => host_type()}, F);
 assert_event(EventName, BinJid)
     when EventName =:= auth_authorize ->
     #jid{lserver = LServer} = jid:from_binary(BinJid),
     F = fun(#{time := Time, count := 1, server := Server}) ->
            (Time > 0) and (Server =:= LServer)
         end,
+    %% Note: this could match events from other tests because there is no user name
     instrument_helper:assert(EventName, #{host_type => host_type()}, F);
 assert_event(EventName, BinJid) ->
     #jid{luser = LUser, lserver = LServer} = jid:from_binary(BinJid),
     F = fun(#{time := Time, count := 1, user := User, server := Server}) ->
            (Time > 0) and (User =:= LUser) and (Server =:= LServer)
         end,
-    instrument_helper:assert(EventName, #{host_type => host_type()}, F).
+    instrument_helper:assert_one(EventName, #{host_type => host_type()}, F).

big_tests/tests/disco_and_caps_SUITE.erl

@@ -235,5 +235,5 @@ urls(sales) -> [<<"[email protected]">>].
 
 assert_roster_get_event(Client) ->
     ClientJid = jid:from_binary(escalus_client:full_jid(Client)),
-    instrument_helper:assert(mod_disco_roster_get, #{host_type => host_type()},
-                             fun(#{count := 1, jid := Jid}) -> ClientJid =:= Jid end).
+    instrument_helper:assert_one(mod_disco_roster_get, #{host_type => host_type()},
+                                 fun(#{count := 1, jid := Jid}) -> ClientJid =:= Jid end).

big_tests/tests/domain_isolation_SUITE.erl

@@ -157,7 +157,7 @@ verify_alice_has_no_pending_messages(Alice, Bob) ->
 assert_stanza_dropped(Sender, Recipient, Stanza) ->
     SenderJid = jid:from_binary(escalus_utils:get_jid(Sender)),
     RecipientJid = jid:from_binary(escalus_utils:get_jid(Recipient)),
-    instrument_helper:assert(
+    instrument_helper:assert_one(
       router_stanza_dropped, #{host_type => host_type()},
       fun(#{count := 1, from_jid := From, to_jid := To, stanza := DroppedStanza}) ->
               From =:= SenderJid andalso To =:= RecipientJid andalso DroppedStanza =:= Stanza

big_tests/tests/graphql_roster_SUITE.erl

@@ -398,7 +398,7 @@ admin_list_contacts_story(Config, Alice, Bob) ->
     [#{<<"subscription">> := <<"NONE">>, <<"ask">> := <<"NONE">>, <<"jid">> := BobBin,
        <<"name">> := BobName, <<"groups">> := ?DEFAULT_GROUPS}] =
         get_ok_value([data, roster, listContacts], Res),
-    roster_helper:assert_roster_event(Alice, mod_roster_get).
+    roster_helper:assert_roster_event(escalus_client:short_jid(Alice), mod_roster_get).
 
 admin_list_contacts_wrong_user(Config) ->
     % User with a non-existent domain
@@ -548,7 +548,7 @@ user_list_contacts_story(Config, Alice, Bob) ->
     [#{<<"subscription">> := <<"NONE">>, <<"ask">> := <<"NONE">>, <<"jid">> := BobBin,
        <<"name">> := Name, <<"groups">> := ?DEFAULT_GROUPS}] =
         get_ok_value(?LIST_CONTACTS_PATH, Res),
-    roster_helper:assert_roster_event(Alice, mod_roster_get).
+    roster_helper:assert_roster_event(escalus_client:short_jid(Alice), mod_roster_get).
 
 user_get_contact(Config) ->
     escalus:fresh_story_with_config(Config, [{alice, 1}, {bob, 1}],

big_tests/tests/instrument_cets_SUITE.erl

@@ -42,15 +42,13 @@ end_per_group(_, _Config) ->
 init_per_testcase(_, Config) ->
     Config.
 
-check_instrumentation(Config) ->
-    instrument_helper:wait_for_new(cets_info, #{}),
-    instrument_helper:assert(cets_info, #{}, fun(Res) ->
-            %% Values are integers
-            lists:all(fun(Name) -> is_integer(maps:get(Name, Res)) end, instrumentation_metrics_names())
-            andalso
-            %% Check that there are no unknown fields
-            [] =:= maps:keys(maps:without(instrumentation_metrics_names(), Res))
-        end).
+check_instrumentation(_Config) ->
+    instrument_helper:wait_and_assert_new(cets_info, #{}, fun check_info/1).
+
+%% Check that values are integers and there are no unknown fields
+check_info(Res) ->
+    lists:all(fun(Name) -> is_integer(maps:get(Name, Res)) end, instrumentation_metrics_names())
+        andalso #{} =:= maps:without(instrumentation_metrics_names(), Res).
 
 instrumentation_metrics_names() ->
     [available_nodes,

big_tests/tests/instrument_helper.erl

@@ -8,7 +8,7 @@
 
 %% API for assertions in test cases
 -export([assert/3, assert_one/3, assert_not_emitted/3, assert_not_emitted/2, assert_not_emitted/1,
-         wait_and_assert/3, wait_and_assert_new/3, assert/4]).
+         wait_and_assert/3, wait_and_assert_new/3, assert/4, timestamp/0]).
 
 -import(distributed_helper, [rpc/4, mim/0]).
 
@@ -22,7 +22,7 @@
 -type measurements() :: #{atom() => term()}.
 -type check_fun() :: fun((measurements()) -> boolean()).
 -type event_count() :: non_neg_integer() | positive.
--type opts() :: #{timestamp => integer(),
+-type opts() :: #{min_timestamp => integer(),
                   retries := non_neg_integer(),
                   delay := non_neg_integer(),
                   expected_count := event_count()}.
@@ -103,7 +103,7 @@ wait_and_assert(EventName, Labels, CheckF) ->
 %% @doc Waits for a matching event, ignoring past events.
 -spec wait_and_assert_new(event_name(), labels(), check_fun()) -> ok.
 wait_and_assert_new(EventName, Labels, CheckF) ->
-    assert(EventName, Labels, CheckF, #{timestamp => timestamp(), retries => 50, delay => 100}).
+    assert(EventName, Labels, CheckF, #{min_timestamp => timestamp(), retries => 50, delay => 100}).
 
 %% @doc Assert that an expected number of events with `EventName' and `Labels' are present.
 %% Events are filtered by applying `CheckF' to the map of measurements.
@@ -114,33 +114,21 @@ assert(EventName, Labels, CheckF, Opts) ->
     FullOpts = maps:merge(default_opts(), Opts),
     assert_loop(EventName, Labels, CheckF, FullOpts).
 
-%% Low-level API
-
--spec filter(fun((measurements()) -> boolean()), [measurements()]) -> [measurements()].
-filter(CheckF, MeasurementsList) ->
-    lists:filter(fun(Measurements) ->
-                         try CheckF(Measurements) catch error:function_clause -> false end
-                 end, MeasurementsList).
-
--spec select(event_name(), labels()) -> [measurements()].
-select(EventName, Labels) ->
-    rpc(mim(), ?HANDLER_MODULE, select, [EventName, Labels]).
-
--spec select_new(event_name(), labels(), integer()) -> [measurements()].
-select_new(EventName, Labels, Timestamp) ->
-    rpc(mim(), ?HANDLER_MODULE, select_new, [EventName, Labels, Timestamp]).
-
 -spec timestamp() -> integer().
 timestamp() ->
     rpc(mim(), ?HANDLER_MODULE, timestamp, []).
 
 %% Internal functions
 
+-spec default_opts() -> opts().
+default_opts() ->
+    #{retries => 0, delay => timer:seconds(1), expected_count => positive}.
+
 -spec assert_loop(event_name(), labels(), check_fun(), opts()) -> ok.
 assert_loop(EventName, Labels, CheckF, Opts) ->
     #{retries := Retries, expected_count := ExpectedCount, delay := Delay} = Opts,
     All = case Opts of
-              #{timestamp := Timestamp} ->
+              #{min_timestamp := Timestamp} ->
                   select_new(EventName, Labels, Timestamp);
               #{} ->
                   select(EventName, Labels)
@@ -154,9 +142,19 @@ assert_loop(EventName, Labels, CheckF, Opts) ->
             assert_check_result(EventName, Labels, All, Filtered, CheckResult, ExpectedCount)
     end.
 
--spec default_opts() -> opts().
-default_opts() ->
-    #{retries => 0, delay => timer:seconds(1), expected_count => positive}.
+-spec filter(fun((measurements()) -> boolean()), [measurements()]) -> [measurements()].
+filter(CheckF, MeasurementsList) ->
+    lists:filter(fun(Measurements) ->
+                         try CheckF(Measurements) catch error:function_clause -> false end
+                 end, MeasurementsList).
+
+-spec select(event_name(), labels()) -> [measurements()].
+select(EventName, Labels) ->
+    rpc(mim(), ?HANDLER_MODULE, select, [EventName, Labels]).
+
+-spec select_new(event_name(), labels(), integer()) -> [measurements()].
+select_new(EventName, Labels, Timestamp) ->
+    rpc(mim(), ?HANDLER_MODULE, select_new, [EventName, Labels, Timestamp]).
 
 -spec check([measurements()], event_count()) -> boolean().
 check(Filtered, positive) ->
@@ -168,12 +166,13 @@ check(Filtered, ExpectedCount) ->
                           Filtered :: [measurements()], CheckResult :: boolean(),
                           event_count()) -> ok.
 assert_check_result(_EventName, _Labels, _All, [], true, _ExpectedCount) ->
-    ok; % don't mark events as tested
+    ok; % don't mark non-emitted events as tested
 assert_check_result(EventName, Labels, _All, Filtered, true, _ExpectedCount) ->
     ct:log("Matching measurements for event ~p with labels ~p:~n~p", [EventName, Labels, Filtered]),
     event_tested(EventName, Labels);
 assert_check_result(EventName, Labels, All, Filtered, false, ExpectedCount) ->
-    ct:log("All measurements for event ~p with labels ~p:~n~p", [EventName, Labels, All]),
+    ct:log("Assertion failed for event ~p with labels ~p.~nMatching measurements:~n~p~n~n"
+           "Other measurements:~n~p", [EventName, Labels, Filtered, All -- Filtered]),
     ct:fail("Incorrect number of instrumentation events - matched: ~p, expected: ~p",
             [length(Filtered), ExpectedCount]).
 

big_tests/tests/mam_SUITE.erl

@@ -113,7 +113,8 @@
          assert_lookup_event/2,
          assert_flushed_event_if_async/2,
          assert_dropped_iq_event/2,
-         assert_event_with_jid/2
+         assert_event_with_jid/2,
+         assert_no_event_with_jid/2
         ]).
 
 -import(muc_light_helper,
@@ -3285,7 +3286,7 @@ prefs_set_request(Config) ->
         ?assert_equal(ResultIQ1, ResultIQ2),
         ok
         end,
-    escalus:story(Config, [{alice, 1}], F).
+    escalus:fresh_story(Config, [{alice, 1}], F).
 
 query_get_request(Config) ->
     F = fun(Alice) ->
@@ -3375,11 +3376,13 @@ muc_prefs_set_request(ConfigIn) ->
 muc_prefs_set_request_not_an_owner(ConfigIn) ->
     F = fun(Config, _Alice, Bob) ->
         Room = ?config(room, Config),
+        RoomAddr = room_address(Room),
         escalus:send(Bob, stanza_to_room(stanza_prefs_set_request(<<"roster">>,
                                                                     [<<"[email protected]">>],
                                                                     [<<"[email protected]">>],
                                                                     mam_ns_binary()), Room)),
-        escalus:assert(is_error, [<<"cancel">>, <<"not-allowed">>], escalus:wait_for_stanza(Bob))
+        escalus:assert(is_error, [<<"cancel">>, <<"not-allowed">>], escalus:wait_for_stanza(Bob)),
+        assert_no_event_with_jid(mod_mam_muc_get_prefs, RoomAddr)
     end,
     RoomOpts = [{persistent, true}],
     UserSpecs = [{alice, 1}, {bob, 1}],

big_tests/tests/mam_helper.erl

@@ -1414,27 +1414,30 @@ assert_list_size(N, List) when N =:= length(List) -> List.
 %% Assertions for instrumentation events
 
 assert_archive_message_event(EventName, BinJid) ->
-    assert_event(EventName,
-                 fun(#{count := 1, time := T, params := #{local_jid := LocalJid}}) ->
-                         eq_bjid(LocalJid, BinJid) andalso pos_int(T)
-                 end).
+    instrument_helper:assert_one(
+      EventName, labels(),
+      fun(#{count := 1, time := T, params := #{local_jid := LocalJid}}) ->
+              eq_bjid(LocalJid, BinJid) andalso pos_int(T)
+      end).
 
 assert_lookup_event(EventName, BinJid) ->
-    assert_event(EventName,
-                 fun(#{count := 1, size := 1, time := T, params := #{caller_jid := CallerJid}}) ->
-                         eq_bjid(CallerJid, BinJid) andalso pos_int(T)
-                 end).
+    instrument_helper:assert_one(
+      EventName, labels(),
+      fun(#{count := 1, size := 1, time := T, params := #{caller_jid := CallerJid}}) ->
+              eq_bjid(CallerJid, BinJid) andalso pos_int(T)
+      end).
 
 %% The event might originate from a different test case running in parallel,
 %% but there is no easy way around it other than adding all flushed messages to measurements.
 assert_flushed_event_if_async(EventName, Config) ->
     case ?config(configuration, Config) of
         C when C =:= rdbms_async_pool;
                C =:= rdbms_async_cache ->
-            assert_event(EventName,
-                         fun(#{count := Count, time := T, time_per_message := T1}) ->
-                                 pos_int(Count) andalso pos_int(T) andalso pos_int(T1) andalso T >= T1
-                         end);
+            instrument_helper:assert(
+              EventName, labels(),
+              fun(#{count := Count, time := T, time_per_message := T1}) ->
+                      pos_int(Count) andalso pos_int(T) andalso pos_int(T1) andalso T >= T1
+              end);
         _ ->
             ok
     end.
@@ -1444,18 +1447,19 @@ assert_dropped_iq_event(Config, BinJid) ->
                     undefined -> mod_mam_pm_dropped_iq;
                     _ -> mod_mam_muc_dropped_iq
                 end,
-    assert_event(EventName, fun(#{acc := #{stanza := #{from_jid := FromJid}}}) ->
-                                    eq_bjid(FromJid, BinJid)
-                            end).
+    instrument_helper:assert_one(
+      EventName, labels(),
+      fun(#{acc := #{stanza := #{from_jid := FromJid}}}) -> eq_bjid(FromJid, BinJid) end).
 
 assert_event_with_jid(EventName, BinJid) ->
-    assert_event(EventName, fun(#{count := 1, jid := Jid}) -> eq_bjid(Jid, BinJid) end).
+    instrument_helper:assert_one(
+      EventName, labels(), fun(#{count := 1, jid := Jid}) -> eq_bjid(Jid, BinJid) end).
 
-assert_dropped_msg_event(EventName) ->
-    assert_event(EventName, fun(#{count := 1}) -> true end).
+assert_no_event_with_jid(EventName, BinJid) ->
+    instrument_helper:assert_not_emitted(
+      EventName, labels(), fun(#{count := 1, jid := Jid}) -> eq_bjid(Jid, BinJid) end).
 
-assert_event(EventName, F) ->
-    instrument_helper:assert(EventName, #{host_type => host_type()}, F).
+labels() -> #{host_type => host_type()}.
 
 pos_int(T) -> is_integer(T) andalso T > 0.
 

big_tests/tests/metrics_c2s_SUITE.erl

@@ -80,8 +80,8 @@ login_story(Alice) ->
 
     %% Note: The first two events might originate from other tests because of unknown JID.
     %% It is acceptable, because the goal is to check that they are emitted when users log in.
-    assert_event(out, fun(#xmlel{name = Name}) -> Name =:= <<"stream:features">> end),
-    assert_event(in, fun(#xmlel{name = Name}) -> Name =:= <<"auth">> end),
+    assert_events(out, fun(#xmlel{name = Name}) -> Name =:= <<"stream:features">> end),
+    assert_events(in, fun(#xmlel{name = Name}) -> Name =:= <<"auth">> end),
 
     assert_event(out, AliceBareJid, #{},
                  fun(#xmlel{name = Name}) -> Name =:= <<"success">> end),
@@ -189,11 +189,11 @@ has_child(SubElName, El) ->
 
 assert_event(Dir, ClientOrJid, Measurements) ->
     Jid = jid:from_binary(escalus_utils:get_jid(ClientOrJid)),
-    instrument_helper:assert(
+    instrument_helper:assert_one(
       event_name(Dir), #{host_type => host_type()},
       fun(M) -> M =:= Measurements#{jid => Jid, count => 1} end).
 
-assert_event(Dir, CheckElFun) ->
+assert_events(Dir, CheckElFun) ->
     instrument_helper:assert(
       event_name(Dir), #{host_type => host_type()},
       fun(M = #{element := El}) ->
@@ -202,7 +202,7 @@ assert_event(Dir, CheckElFun) ->
 
 assert_event(Dir, ClientOrJid, Measurements, CheckElFun) ->
     Jid = jid:from_binary(escalus_utils:get_jid(ClientOrJid)),
-    instrument_helper:assert(
+    instrument_helper:assert_one(
       event_name(Dir), #{host_type => host_type()},
       fun(M = #{element := El}) ->
                maps:remove(element, M) =:= Measurements#{jid => Jid, count => 1}
@@ -217,7 +217,7 @@ event_name(in) -> c2s_element_in.
 assert_message_bounced_event(Sender, Recipient) ->
     FromJid = jid:from_binary(escalus_utils:get_jid(Sender)),
     ToJid = jid:from_binary(escalus_utils:get_jid(Recipient)),
-    instrument_helper:assert(
+    instrument_helper:assert_one(
       sm_message_bounced, #{host_type => host_type()},
       fun(#{count := 1, from_jid := From, to_jid := To}) ->
                From =:= FromJid andalso To =:= ToJid