From 798fcc2b2ece94f09fa2ddc185a761305e15dfbf Mon Sep 17 00:00:00 2001 From: Michal Piotrowski Date: Mon, 9 May 2016 13:05:23 +0200 Subject: [PATCH 01/11] improve interleave requests test case --- test/ejabberd_tests/src/ct_tty_hook.erl | 5 +- test/ejabberd_tests/tests/bosh_SUITE.erl | 74 ++++++++++++++++++++---- 2 files changed, 65 insertions(+), 14 deletions(-) diff --git a/test/ejabberd_tests/src/ct_tty_hook.erl b/test/ejabberd_tests/src/ct_tty_hook.erl index 096594ee301..fb5365db93f 100644 --- a/test/ejabberd_tests/src/ct_tty_hook.erl +++ b/test/ejabberd_tests/src/ct_tty_hook.erl @@ -93,7 +93,8 @@ post_end_per_testcase(TC, _Config, Return, State) -> %% @doc Called after post_init_per_suite, post_end_per_suite, post_init_per_group, %% post_end_per_group and post_end_per_testcase if the suite, group or test case failed. -on_tc_fail(_TC, _Reason, State) -> +on_tc_fail(TC, Reason, State) -> + ct:print("~p~n~p", [TC, Reason]), State. %% @doc Called when a test case is skipped by either user action @@ -148,4 +149,4 @@ print_case_enter(Group, #state{print_case = true}, Msg) -> escalus_ejabberd:rpc(error_logger, warning_msg, ["====== ~s CASE ~p", [Msg, Group]]); print_case_enter(_Group, _State, _Msg) -> - ok. \ No newline at end of file + ok. diff --git a/test/ejabberd_tests/tests/bosh_SUITE.erl b/test/ejabberd_tests/tests/bosh_SUITE.erl index 82a94468c42..53381ba392c 100644 --- a/test/ejabberd_tests/tests/bosh_SUITE.erl +++ b/test/ejabberd_tests/tests/bosh_SUITE.erl @@ -69,7 +69,8 @@ chat_test_cases() -> cant_send_invalid_rid, multiple_stanzas, namespace, - stream_error]. + stream_error + ]. time_test_cases() -> [disconnect_inactive, @@ -268,23 +269,72 @@ interleave_requests(Config) -> Rid = get_bosh_rid(Carol), Sid = get_bosh_sid(Carol), - Empty2 = escalus_bosh:empty_body(Rid + 1, Sid), - Chat2 = Empty2#xmlel{ - children = [escalus_stanza:chat_to(Geralt, <<"2nd!">>)]}, - escalus_bosh:send_raw(Carol, Chat2), + Msg1 = <<"1st!">>, + Msg2 = <<"2nd!">>, + Msg3 = <<"3rd!">>, + Msg4 = <<"4th!">>, + + send_message_with_rid(Carol, Geralt, Rid + 1, Sid, Msg2), + send_message_with_rid(Carol, Geralt, Rid, Sid, Msg1), + + send_message_with_rid(Carol, Geralt, Rid + 2, Sid, Msg3), + send_message_with_rid(Carol, Geralt, Rid + 3, Sid, Msg4), + + escalus:assert(is_chat_message, [Msg1], + escalus_client:wait_for_stanza(Geralt)), + escalus:assert(is_chat_message, [Msg2], + escalus_client:wait_for_stanza(Geralt)), + escalus:assert(is_chat_message, [Msg3], + escalus_client:wait_for_stanza(Geralt)), + escalus:assert(is_chat_message, [Msg4], + escalus_client:wait_for_stanza(Geralt)), + + true = escalus_bosh:is_connected(Carol) + end). + +interleave_requests_raw(Config) -> + escalus:story(Config, [{geralt, 1}], fun(Geralt) -> + User = ?config(user, Config), + Carol = start_client(Config, User, <<"bosh">>), + Rid = get_bosh_rid(Carol), + Sid = get_bosh_sid(Carol), + + NamedSpecs = escalus_config:get_config(escalus_users, Config), + UserSpec = [{keepalive, false} | proplists:get_value(User, NamedSpecs)], + + {ok, Carol2} = escalus_bosh:connect(UserSpec), + + Msg1 = <<"1st!">>, + Msg2 = <<"2nd!">>, + Msg3 = <<"3rd!">>, + Msg4 = <<"4th!">>, - Empty1 = escalus_bosh:empty_body(Rid, Sid), - Chat1 = Empty1#xmlel{ - children = [escalus_stanza:chat_to(Geralt, <<"1st!">>)]}, - escalus_bosh:send_raw(Carol, Chat1), + send_message_with_rid(Carol, Geralt, Rid + 1, Sid, Msg2), + send_message_with_rid(Carol2, Geralt, Rid, Sid, Msg1), - escalus:assert(is_chat_message, [<<"1st!">>], + send_message_with_rid(Carol2, Geralt, Rid + 2, Sid, Msg3), + send_message_with_rid(Carol, Geralt, Rid + 3, Sid, Msg4), + + escalus:assert(is_chat_message, [Msg1], + escalus_client:wait_for_stanza(Geralt)), + escalus:assert(is_chat_message, [Msg2], + escalus_client:wait_for_stanza(Geralt)), + escalus:assert(is_chat_message, [Msg3], + escalus_client:wait_for_stanza(Geralt)), + escalus:assert(is_chat_message, [Msg4], escalus_client:wait_for_stanza(Geralt)), - escalus:assert(is_chat_message, [<<"2nd!">>], - escalus_client:wait_for_stanza(Geralt)) + true = escalus_bosh:is_connected(Carol), + escalus_bosh:kill(Carol), + escalus_bosh:kill(Carol2) end). +send_message_with_rid(From, To, Rid, Sid, Msg) -> + Empty = escalus_bosh:empty_body(Rid, Sid), + Chat = Empty#xmlel{ + children = [escalus_stanza:chat_to(To, Msg)]}, + escalus_bosh:send_raw(From, Chat). + simple_chat(Config) -> escalus:story(Config, [{?config(user, Config), 1}, {geralt, 1}], fun(Carol, Geralt) -> From 3453c9d8935bcb1c57102efdfe6c6ee1c9604aa9 Mon Sep 17 00:00:00 2001 From: Michal Piotrowski Date: Mon, 20 Jun 2016 11:28:12 +0200 Subject: [PATCH 02/11] send message to BOSH session when requests sent out-of-order --- test/ejabberd_tests/tests/bosh_SUITE.erl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/ejabberd_tests/tests/bosh_SUITE.erl b/test/ejabberd_tests/tests/bosh_SUITE.erl index 53381ba392c..f4b423b3b51 100644 --- a/test/ejabberd_tests/tests/bosh_SUITE.erl +++ b/test/ejabberd_tests/tests/bosh_SUITE.erl @@ -309,7 +309,10 @@ interleave_requests_raw(Config) -> Msg3 = <<"3rd!">>, Msg4 = <<"4th!">>, + escalus:send(Geralt, escalus_stanza:chat_to(Carol, <<"Hi, Carol">>)), send_message_with_rid(Carol, Geralt, Rid + 1, Sid, Msg2), + escalus:assert(is_chat_message, [<<"Hi, Carol">>], + escalus_client:wait_for_stanza(Carol)), send_message_with_rid(Carol2, Geralt, Rid, Sid, Msg1), send_message_with_rid(Carol2, Geralt, Rid + 2, Sid, Msg3), From dbfd9e782e131edc6e9b464765a9d4d634ead745 Mon Sep 17 00:00:00 2001 From: Michal Piotrowski Date: Tue, 21 Jun 2016 17:06:47 +0200 Subject: [PATCH 03/11] fix stream prefix detection in some rare cases it is possible that stream:features arrive without the stream tag. In this case the xmlns:stream was not added which confused expath and probably other parsers --- apps/ejabberd/src/mod_bosh_socket.erl | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/apps/ejabberd/src/mod_bosh_socket.erl b/apps/ejabberd/src/mod_bosh_socket.erl index 2d63c5021ec..252b0efc478 100644 --- a/apps/ejabberd/src/mod_bosh_socket.erl +++ b/apps/ejabberd/src/mod_bosh_socket.erl @@ -894,7 +894,8 @@ bosh_wrap(Elements, Rid, #state{} = S) -> end, MaybeAck = maybe_ack(Rid, NS), {MaybeReport, NNS} = maybe_report(NS), - MaybeStreamPrefix = maybe_stream_prefix(Children), + HasStreamPrefix = (exml_query:attr(Body, <<"xmlns:stream">>) /= undefined), + MaybeStreamPrefix = maybe_stream_prefix(HasStreamPrefix, Children), ExtraAttrs = MaybeAck ++ MaybeReport ++ MaybeStreamPrefix, {Body#xmlel{attrs = Body#xmlel.attrs ++ ExtraAttrs, children = maybe_add_default_ns_to_children(Children)}, NNS}. @@ -965,16 +966,19 @@ bosh_stream_end_body() -> {<<"xmlns">>, ?NS_HTTPBIND}], children = []}. -maybe_stream_prefix(Stanzas) -> - case lists:any(fun is_stream_error/1, Stanzas) of +maybe_stream_prefix(true, _) -> + []; +maybe_stream_prefix(_, Stanzas) -> + case lists:any(fun is_stream_prefix/1, Stanzas) of false -> []; true -> [{<<"xmlns:stream">>, ?NS_STREAM}] end. -is_stream_error(#xmlel{name = Name}) -> - Name =:= <<"stream:error">>. +is_stream_prefix(#xmlel{name = <<"stream:error">>}) -> true; +is_stream_prefix(#xmlel{name = <<"stream:features">>}) -> true; +is_stream_prefix(_) -> false. %%-------------------------------------------------------------------- %% ejabberd_socket compatibility From 4b4223c46dd20968f3e6d4c0c1af7559abe4b01f Mon Sep 17 00:00:00 2001 From: Michal Piotrowski Date: Tue, 21 Jun 2016 17:09:12 +0200 Subject: [PATCH 04/11] add basic property test for messages sent over BOSH --- .../tests/bosh_interleave_reqs.erl | 173 ++++++++++++++++++ 1 file changed, 173 insertions(+) create mode 100644 test/ejabberd_tests/tests/bosh_interleave_reqs.erl diff --git a/test/ejabberd_tests/tests/bosh_interleave_reqs.erl b/test/ejabberd_tests/tests/bosh_interleave_reqs.erl new file mode 100644 index 00000000000..e9722ba6af3 --- /dev/null +++ b/test/ejabberd_tests/tests/bosh_interleave_reqs.erl @@ -0,0 +1,173 @@ +-module(bosh_interleave_reqs). + +-behaviour(proper_statem). + +-include_lib("proper/include/proper.hrl"). +-include_lib("escalus/include/escalus.hrl"). + +-export([test/1, sample/0, prop/1]). + +-export([initial_state/0, command/1, precondition/2, postcondition/3, + next_state/3]). + +-export([read_config/0, + connect_carol/0, + connect_geralt/0, + send_from_carol/2, + send_from_geralt/2, + wait_for_msgs_carol/2, + wait_for_msgs_geralt/2]). + +-export([ct_config_giver/1]). + +-record(state, {carol, + geralt, + msgs_to_carol, + msgs_to_geralt}). + +test(Config) -> + proper:quickcheck(?MODULE:prop(Config)). + +sample() -> + proper_gen:pick(commands(?MODULE)). + +prop(Config) -> + spawn_link(?MODULE, ct_config_giver, [Config]), + ?FORALL(Cmds, commands(?MODULE), + ?TRAPEXIT( + begin + ct:pal("commands: ~p", [Cmds]), + {History,State,Result} = run_commands(?MODULE, Cmds), + maybe_stop_client(State#state.carol), + maybe_stop_client(State#state.geralt), + escalus_fresh:clean(), + ?WHENFAIL(ct:pal(error, "History: ~p~nState: ~p\nResult: ~p~n", + [History,State,Result]), + aggregate(command_names(Cmds), Result =:= ok)) + end)). + +ct_config_giver(Config) -> + register(ct_config_giver, self()), + wait_for_request(Config). + +wait_for_request(Config) -> + receive + {give_me_config, Pid} -> + Pid ! {ok, Config}, + wait_for_request(Config) + end. + +maybe_stop_client(undefined) -> ok; +maybe_stop_client(Client) -> + escalus_client:stop(Client). + +initial_state() -> + #state{carol = undefined, + geralt = undefined, + msgs_to_carol = [], + msgs_to_geralt = []}. + +command(S) -> + Cmds = possible_commands(S), + oneof(Cmds). + +possible_commands(S) -> + Carol = (S#state.carol /= undefined), + Geralt = (S#state.geralt /= undefined), + Users = Carol andalso Geralt, + MsgsToCarol = (S#state.msgs_to_carol /= []), + MsgsToGeralt = (S#state.msgs_to_geralt /= []), + [{call, ?MODULE, connect_carol, []} || not Carol] ++ + [{call, ?MODULE, connect_geralt, []} || not Geralt] ++ + [{call, ?MODULE, send_from_carol, [S#state.carol, S#state.geralt]} || Users] ++ + [{call, ?MODULE, send_from_geralt, [S#state.geralt, S#state.carol]} || Users] ++ + [{call, ?MODULE, wait_for_msgs_carol, [S#state.carol, S#state.msgs_to_carol]} + || MsgsToCarol] ++ + [{call, ?MODULE, wait_for_msgs_geralt, [S#state.geralt, S#state.msgs_to_geralt]} + || MsgsToGeralt]. + + +precondition(_, _) -> true. +postcondition(_, _, _) -> true. + +next_state(S, V, {call, _, connect_carol, []}) -> + S#state{carol = V}; +next_state(S, V, {call, _, connect_geralt, []}) -> + S#state{geralt = V}; +next_state(#state{msgs_to_geralt = Msgs} = S, V, {call, _, send_from_carol, _}) -> + % ct:pal("msgs to geralt: ~p", [[V | Msgs]]), + S#state{msgs_to_geralt = [V | Msgs]}; +next_state(#state{msgs_to_carol = Msgs} = S, V, {call, _, send_from_geralt, _}) -> + % ct:pal("msgs to carol: ~p", [[V | Msgs]]), + S#state{msgs_to_carol = [V | Msgs]}; +next_state(S, _, {call, _, wait_for_msgs_carol, _}) -> + % ct:pal("msgs to carol2: ~p", [S#state.msgs_to_carol]), + S#state{msgs_to_carol = []}; +next_state(S, _, {call, _, wait_for_msgs_geralt, _}) -> + % ct:pal("msgs to geralt2: ~p", [S#state.msgs_to_geralt]), + S#state{msgs_to_geralt = []}; +next_state(S, _, _) -> + S. + +read_config() -> + ct_config_giver ! {give_me_config, self()}, + receive + {ok, Config} -> + Config + after 100 -> + error + end. + +connect_carol() -> + Spec = given_fresh_spec(read_config(), carol), + connect_user([{keepalive, true} | Spec]). + +connect_geralt() -> + Spec = given_fresh_spec(read_config(), geralt), + connect_user(Spec). + +given_fresh_spec(Config, User) -> + NewConfig = escalus_fresh:create_users(Config, [{User, 1}]), + escalus_users:get_userspec(NewConfig, User). + +connect_user(Spec) -> + Res = base64:encode(crypto:rand_bytes(4)), + {ok, Conn, Props, _} = escalus_connection:start([{resource, Res} | Spec]), + JID = make_jid(Props), + escalus:send(Conn, escalus_stanza:presence(<<"available">>)), + escalus:wait_for_stanza(Conn), + Conn#client{jid = JID}. + +make_jid(Proplist) -> + {username, U} = lists:keyfind(username, 1, Proplist), + {server, S} = lists:keyfind(server, 1, Proplist), + {resource, R} = lists:keyfind(resource, 1, Proplist), + <>. + +send_from_carol(Carol, Geralt) -> + Msg = gen_msg(), + escalus:send(Carol, escalus_stanza:chat_to(Geralt, Msg)), + Msg. + +send_from_geralt(Geralt, Carol) -> + Msg = gen_msg(), + escalus:send(Geralt, escalus_stanza:chat_to(Carol, Msg)), + Msg. + +gen_msg() -> + Msg = base64:encode(crypto:rand_bytes(15)), + Msg. + +wait_for_msgs_carol(Carol, Msgs) -> + wait_for_msgs(Carol, lists:reverse(Msgs)). + +wait_for_msgs_geralt(Geralt, Msgs) -> + wait_for_msgs(Geralt, lists:reverse(Msgs)). + +wait_for_msgs(_Client, []) -> + ok; +wait_for_msgs(Client, [Msg | Rest]) -> + escalus:assert(is_chat_message, [Msg], + escalus_client:wait_for_stanza(Client)), + wait_for_msgs(Client, Rest). + From f72cca6910f10124a902a977e2721846e5006214 Mon Sep 17 00:00:00 2001 From: Michal Piotrowski Date: Thu, 23 Jun 2016 10:29:23 +0200 Subject: [PATCH 05/11] enable PropEr test for interleave requests --- test/ejabberd_tests/tests/bosh_SUITE.erl | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/ejabberd_tests/tests/bosh_SUITE.erl b/test/ejabberd_tests/tests/bosh_SUITE.erl index f4b423b3b51..13b3a274ae4 100644 --- a/test/ejabberd_tests/tests/bosh_SUITE.erl +++ b/test/ejabberd_tests/tests/bosh_SUITE.erl @@ -63,6 +63,7 @@ essential_test_cases() -> chat_test_cases() -> [interleave_requests, + interleave_requests_statem, simple_chat, cdata_escape_chat, escape_attr_chat, @@ -292,7 +293,10 @@ interleave_requests(Config) -> true = escalus_bosh:is_connected(Carol) end). -interleave_requests_raw(Config) -> +interleave_requests_statem(Config) -> + true = bosh_interleave_reqs:test(Config). + +interleave_requests_escalus(Config) -> escalus:story(Config, [{geralt, 1}], fun(Geralt) -> User = ?config(user, Config), Carol = start_client(Config, User, <<"bosh">>), From 7c20f6930b3c95aa1cbdc6d2deb965a335036a8e Mon Sep 17 00:00:00 2001 From: Michal Piotrowski Date: Thu, 23 Jun 2016 10:42:20 +0200 Subject: [PATCH 06/11] generate more commands and add sender to msg history --- .../tests/bosh_interleave_reqs.erl | 34 ++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/test/ejabberd_tests/tests/bosh_interleave_reqs.erl b/test/ejabberd_tests/tests/bosh_interleave_reqs.erl index e9722ba6af3..f3bbc95c776 100644 --- a/test/ejabberd_tests/tests/bosh_interleave_reqs.erl +++ b/test/ejabberd_tests/tests/bosh_interleave_reqs.erl @@ -3,7 +3,10 @@ -behaviour(proper_statem). -include_lib("proper/include/proper.hrl"). +% -include_lib("eqc/include/eqc_statem.hrl"). +% -include_lib("eqc/include/eqc.hrl"). -include_lib("escalus/include/escalus.hrl"). +-include_lib("exml/include/exml.hrl"). -export([test/1, sample/0, prop/1]). @@ -33,7 +36,7 @@ sample() -> prop(Config) -> spawn_link(?MODULE, ct_config_giver, [Config]), - ?FORALL(Cmds, commands(?MODULE), + ?FORALL(Cmds, more_commands(2, commands(?MODULE)), ?TRAPEXIT( begin ct:pal("commands: ~p", [Cmds]), @@ -147,12 +150,26 @@ make_jid(Proplist) -> send_from_carol(Carol, Geralt) -> Msg = gen_msg(), escalus:send(Carol, escalus_stanza:chat_to(Geralt, Msg)), + {escalus_client:short_jid(Carol), + Msg + }. + +send_from_carol_bigger_rid(Carol, Geralt) -> + Msg = gen_msg(), + Rid = escalus_bosh:get_rid(Carol), + Sid = escalus_bosh:get_sid(Carol), + EmptyBody = escalus_bosh:empty_body(Rid + 1, Sid), + Stanza = EmptyBody#xmlel{ + children = [escalus_stanza:chat_to(Geralt, Msg)]}, + escalus_bosh:send_raw(Carol, Stanza), Msg. send_from_geralt(Geralt, Carol) -> Msg = gen_msg(), escalus:send(Geralt, escalus_stanza:chat_to(Carol, Msg)), - Msg. + {escalus_client:short_jid(Geralt), + Msg + }. gen_msg() -> Msg = base64:encode(crypto:rand_bytes(15)), @@ -166,8 +183,17 @@ wait_for_msgs_geralt(Geralt, Msgs) -> wait_for_msgs(_Client, []) -> ok; -wait_for_msgs(Client, [Msg | Rest]) -> +wait_for_msgs(Client, [{_, Msg} | Rest]) -> escalus:assert(is_chat_message, [Msg], - escalus_client:wait_for_stanza(Client)), + escalus:wait_for_stanza(Client)), wait_for_msgs(Client, Rest). +wait_for_stanza(#client{module = escalus_bosh} = Client) -> + Rid = escalus_bosh:get_rid(Client), + Sid = escalus_bosh:get_sid(Client), + EmptyBody = escalus_bosh:empty_body(Rid, Sid), + escalus_bosh:send(Client, EmptyBody), + esclus:wait_for_stanza(Client); +wait_for_stanza(Client) -> + escalus:wait_for_stanza(Client). + From 07f6c9073662288ef9223a44cd2b6149314804c9 Mon Sep 17 00:00:00 2001 From: Michal Piotrowski Date: Thu, 23 Jun 2016 14:07:33 +0200 Subject: [PATCH 07/11] do not assert order of messages recieved over BOSH it is not possible to guarantee order of msgs received from BOSH is 100% tests --- .../tests/bosh_interleave_reqs.erl | 28 ++++++------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/test/ejabberd_tests/tests/bosh_interleave_reqs.erl b/test/ejabberd_tests/tests/bosh_interleave_reqs.erl index f3bbc95c776..160f782c076 100644 --- a/test/ejabberd_tests/tests/bosh_interleave_reqs.erl +++ b/test/ejabberd_tests/tests/bosh_interleave_reqs.erl @@ -154,16 +154,6 @@ send_from_carol(Carol, Geralt) -> Msg }. -send_from_carol_bigger_rid(Carol, Geralt) -> - Msg = gen_msg(), - Rid = escalus_bosh:get_rid(Carol), - Sid = escalus_bosh:get_sid(Carol), - EmptyBody = escalus_bosh:empty_body(Rid + 1, Sid), - Stanza = EmptyBody#xmlel{ - children = [escalus_stanza:chat_to(Geralt, Msg)]}, - escalus_bosh:send_raw(Carol, Stanza), - Msg. - send_from_geralt(Geralt, Carol) -> Msg = gen_msg(), escalus:send(Geralt, escalus_stanza:chat_to(Carol, Msg)), @@ -176,7 +166,14 @@ gen_msg() -> Msg. wait_for_msgs_carol(Carol, Msgs) -> - wait_for_msgs(Carol, lists:reverse(Msgs)). + L = length(Msgs), + Stanzas = escalus:wait_for_stanzas(Carol, L), + RMsgs = [exml_query:path(E, [{element, <<"body">>}, cdata]) + || E <- Stanzas], + SortedMsgs = lists:sort([Msg || {_, Msg} <- Msgs]), + ct:pal("waiting for: ~p~nreceived: ~p", [SortedMsgs, RMsgs]), + SortedMsgs = lists:sort(RMsgs), + ok. wait_for_msgs_geralt(Geralt, Msgs) -> wait_for_msgs(Geralt, lists:reverse(Msgs)). @@ -188,12 +185,3 @@ wait_for_msgs(Client, [{_, Msg} | Rest]) -> escalus:wait_for_stanza(Client)), wait_for_msgs(Client, Rest). -wait_for_stanza(#client{module = escalus_bosh} = Client) -> - Rid = escalus_bosh:get_rid(Client), - Sid = escalus_bosh:get_sid(Client), - EmptyBody = escalus_bosh:empty_body(Rid, Sid), - escalus_bosh:send(Client, EmptyBody), - esclus:wait_for_stanza(Client); -wait_for_stanza(Client) -> - escalus:wait_for_stanza(Client). - From 3133e66ba53203def82eb390851e10963c4333b7 Mon Sep 17 00:00:00 2001 From: Michal Piotrowski Date: Mon, 12 Sep 2016 08:59:19 +0200 Subject: [PATCH 08/11] fix for the main bosh issue with unexpected RID The fix is only half line of code. Cached responses should be resent only if there is a handler, otherwise the RID is not incremented correctly when new request arrives and the session is terminated incorrectly. See PR #869 for more details about the bug. --- apps/ejabberd/src/mod_bosh_socket.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/ejabberd/src/mod_bosh_socket.erl b/apps/ejabberd/src/mod_bosh_socket.erl index 252b0efc478..a3521a91c63 100644 --- a/apps/ejabberd/src/mod_bosh_socket.erl +++ b/apps/ejabberd/src/mod_bosh_socket.erl @@ -417,7 +417,7 @@ handle_stream_event({EventTag, Body, Rid} = Event, Handler, is_expected_rid(Rid, ExpectedRid), is_acceptable_rid(Rid, ExpectedRid)} of - {_, {true, CachedResponse}, _, _} -> + {_, {true, CachedResponse}, _, _} when Handler /= none -> case CachedResponse of none -> NS; From ea3653bb135da20de8f01cde36f12461885254b4 Mon Sep 17 00:00:00 2001 From: Michal Piotrowski Date: Mon, 12 Sep 2016 11:11:32 +0200 Subject: [PATCH 09/11] move PropEr tests to separate, parallel group --- test/ejabberd_tests/tests/bosh_SUITE.erl | 51 +++-------------- .../tests/bosh_interleave_reqs.erl | 55 +++++++++---------- 2 files changed, 34 insertions(+), 72 deletions(-) diff --git a/test/ejabberd_tests/tests/bosh_SUITE.erl b/test/ejabberd_tests/tests/bosh_SUITE.erl index 13b3a274ae4..eb793827600 100644 --- a/test/ejabberd_tests/tests/bosh_SUITE.erl +++ b/test/ejabberd_tests/tests/bosh_SUITE.erl @@ -38,7 +38,8 @@ all() -> {group, acks}, {group, essential_https}, - {group, chat_https} + {group, chat_https}, + {group, interleave_requests_statem} ]. groups() -> @@ -47,7 +48,9 @@ groups() -> {chat, [shuffle], chat_test_cases()}, {chat_https, [shuffle], chat_test_cases()}, {time, [parallel], time_test_cases()}, - {acks, [shuffle], acks_test_cases()}]. + {acks, [shuffle], acks_test_cases()}, + {interleave_requests_statem, [parallel], [interleave_requests_statem]} + ]. suite() -> escalus:suite(). @@ -63,7 +66,6 @@ essential_test_cases() -> chat_test_cases() -> [interleave_requests, - interleave_requests_statem, simple_chat, cdata_escape_chat, escape_attr_chat, @@ -294,47 +296,10 @@ interleave_requests(Config) -> end). interleave_requests_statem(Config) -> - true = bosh_interleave_reqs:test(Config). - -interleave_requests_escalus(Config) -> - escalus:story(Config, [{geralt, 1}], fun(Geralt) -> - User = ?config(user, Config), - Carol = start_client(Config, User, <<"bosh">>), - Rid = get_bosh_rid(Carol), - Sid = get_bosh_sid(Carol), - - NamedSpecs = escalus_config:get_config(escalus_users, Config), - UserSpec = [{keepalive, false} | proplists:get_value(User, NamedSpecs)], - - {ok, Carol2} = escalus_bosh:connect(UserSpec), + true = bosh_interleave_reqs:test([{user, carol} | Config]). - Msg1 = <<"1st!">>, - Msg2 = <<"2nd!">>, - Msg3 = <<"3rd!">>, - Msg4 = <<"4th!">>, - - escalus:send(Geralt, escalus_stanza:chat_to(Carol, <<"Hi, Carol">>)), - send_message_with_rid(Carol, Geralt, Rid + 1, Sid, Msg2), - escalus:assert(is_chat_message, [<<"Hi, Carol">>], - escalus_client:wait_for_stanza(Carol)), - send_message_with_rid(Carol2, Geralt, Rid, Sid, Msg1), - - send_message_with_rid(Carol2, Geralt, Rid + 2, Sid, Msg3), - send_message_with_rid(Carol, Geralt, Rid + 3, Sid, Msg4), - - escalus:assert(is_chat_message, [Msg1], - escalus_client:wait_for_stanza(Geralt)), - escalus:assert(is_chat_message, [Msg2], - escalus_client:wait_for_stanza(Geralt)), - escalus:assert(is_chat_message, [Msg3], - escalus_client:wait_for_stanza(Geralt)), - escalus:assert(is_chat_message, [Msg4], - escalus_client:wait_for_stanza(Geralt)), - - true = escalus_bosh:is_connected(Carol), - escalus_bosh:kill(Carol), - escalus_bosh:kill(Carol2) - end). +interleave_requests_statem_https(Config) -> + interleave_requests_statem([{user, carol_s} | Config]). send_message_with_rid(From, To, Rid, Sid, Msg) -> Empty = escalus_bosh:empty_body(Rid, Sid), diff --git a/test/ejabberd_tests/tests/bosh_interleave_reqs.erl b/test/ejabberd_tests/tests/bosh_interleave_reqs.erl index 160f782c076..f4ec2cc627a 100644 --- a/test/ejabberd_tests/tests/bosh_interleave_reqs.erl +++ b/test/ejabberd_tests/tests/bosh_interleave_reqs.erl @@ -10,12 +10,12 @@ -export([test/1, sample/0, prop/1]). --export([initial_state/0, command/1, precondition/2, postcondition/3, +-export([initial_state/1, command/1, precondition/2, postcondition/3, next_state/3]). --export([read_config/0, - connect_carol/0, - connect_geralt/0, +-export([read_config/1, + connect_carol/1, + connect_geralt/1, send_from_carol/2, send_from_geralt/2, wait_for_msgs_carol/2, @@ -26,7 +26,8 @@ -record(state, {carol, geralt, msgs_to_carol, - msgs_to_geralt}). + msgs_to_geralt, + config_pid}). test(Config) -> proper:quickcheck(?MODULE:prop(Config)). @@ -35,40 +36,35 @@ sample() -> proper_gen:pick(commands(?MODULE)). prop(Config) -> - spawn_link(?MODULE, ct_config_giver, [Config]), - ?FORALL(Cmds, more_commands(2, commands(?MODULE)), + Pid = spawn_link(?MODULE, ct_config_giver, [Config]), + ?FORALL(Cmds, commands(?MODULE, initial_state(Pid)), ?TRAPEXIT( begin - ct:pal("commands: ~p", [Cmds]), {History,State,Result} = run_commands(?MODULE, Cmds), maybe_stop_client(State#state.carol), maybe_stop_client(State#state.geralt), - escalus_fresh:clean(), ?WHENFAIL(ct:pal(error, "History: ~p~nState: ~p\nResult: ~p~n", [History,State,Result]), aggregate(command_names(Cmds), Result =:= ok)) end)). ct_config_giver(Config) -> - register(ct_config_giver, self()), - wait_for_request(Config). - -wait_for_request(Config) -> receive {give_me_config, Pid} -> Pid ! {ok, Config}, - wait_for_request(Config) + ct_config_giver(Config) end. maybe_stop_client(undefined) -> ok; maybe_stop_client(Client) -> escalus_client:stop(Client). -initial_state() -> +initial_state(Pid) -> #state{carol = undefined, geralt = undefined, msgs_to_carol = [], - msgs_to_geralt = []}. + msgs_to_geralt = [], + config_pid = Pid}. command(S) -> Cmds = possible_commands(S), @@ -80,8 +76,9 @@ possible_commands(S) -> Users = Carol andalso Geralt, MsgsToCarol = (S#state.msgs_to_carol /= []), MsgsToGeralt = (S#state.msgs_to_geralt /= []), - [{call, ?MODULE, connect_carol, []} || not Carol] ++ - [{call, ?MODULE, connect_geralt, []} || not Geralt] ++ + Pid = S#state.config_pid, + [{call, ?MODULE, connect_carol, [Pid]} || not Carol] ++ + [{call, ?MODULE, connect_geralt, [Pid]} || not Geralt] ++ [{call, ?MODULE, send_from_carol, [S#state.carol, S#state.geralt]} || Users] ++ [{call, ?MODULE, send_from_geralt, [S#state.geralt, S#state.carol]} || Users] ++ [{call, ?MODULE, wait_for_msgs_carol, [S#state.carol, S#state.msgs_to_carol]} @@ -93,9 +90,9 @@ possible_commands(S) -> precondition(_, _) -> true. postcondition(_, _, _) -> true. -next_state(S, V, {call, _, connect_carol, []}) -> +next_state(S, V, {call, _, connect_carol, [_]}) -> S#state{carol = V}; -next_state(S, V, {call, _, connect_geralt, []}) -> +next_state(S, V, {call, _, connect_geralt, [_]}) -> S#state{geralt = V}; next_state(#state{msgs_to_geralt = Msgs} = S, V, {call, _, send_from_carol, _}) -> % ct:pal("msgs to geralt: ~p", [[V | Msgs]]), @@ -112,8 +109,8 @@ next_state(S, _, {call, _, wait_for_msgs_geralt, _}) -> next_state(S, _, _) -> S. -read_config() -> - ct_config_giver ! {give_me_config, self()}, +read_config(Pid) -> + Pid ! {give_me_config, self()}, receive {ok, Config} -> Config @@ -121,12 +118,12 @@ read_config() -> error end. -connect_carol() -> - Spec = given_fresh_spec(read_config(), carol), +connect_carol(Pid) -> + Spec = given_fresh_spec(read_config(Pid), carol), connect_user([{keepalive, true} | Spec]). -connect_geralt() -> - Spec = given_fresh_spec(read_config(), geralt), +connect_geralt(Pid) -> + Spec = given_fresh_spec(read_config(Pid), alice), connect_user(Spec). given_fresh_spec(Config, User) -> @@ -138,7 +135,7 @@ connect_user(Spec) -> {ok, Conn, Props, _} = escalus_connection:start([{resource, Res} | Spec]), JID = make_jid(Props), escalus:send(Conn, escalus_stanza:presence(<<"available">>)), - escalus:wait_for_stanza(Conn), + escalus:wait_for_stanza(Conn, timer:seconds(5)), Conn#client{jid = JID}. make_jid(Proplist) -> @@ -167,7 +164,7 @@ gen_msg() -> wait_for_msgs_carol(Carol, Msgs) -> L = length(Msgs), - Stanzas = escalus:wait_for_stanzas(Carol, L), + Stanzas = escalus:wait_for_stanzas(Carol, L, timer:seconds(5)), RMsgs = [exml_query:path(E, [{element, <<"body">>}, cdata]) || E <- Stanzas], SortedMsgs = lists:sort([Msg || {_, Msg} <- Msgs]), @@ -182,6 +179,6 @@ wait_for_msgs(_Client, []) -> ok; wait_for_msgs(Client, [{_, Msg} | Rest]) -> escalus:assert(is_chat_message, [Msg], - escalus:wait_for_stanza(Client)), + escalus:wait_for_stanza(Client, timer:seconds(5))), wait_for_msgs(Client, Rest). From a91ee20a1c0636593d7aaca0892f60765a808b69 Mon Sep 17 00:00:00 2001 From: Michal Piotrowski Date: Thu, 29 Sep 2016 13:38:09 +0200 Subject: [PATCH 10/11] improve coding style according to Elvis --- .../tests/bosh_interleave_reqs.erl | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/test/ejabberd_tests/tests/bosh_interleave_reqs.erl b/test/ejabberd_tests/tests/bosh_interleave_reqs.erl index f4ec2cc627a..feaed2c1c51 100644 --- a/test/ejabberd_tests/tests/bosh_interleave_reqs.erl +++ b/test/ejabberd_tests/tests/bosh_interleave_reqs.erl @@ -11,7 +11,7 @@ -export([test/1, sample/0, prop/1]). -export([initial_state/1, command/1, precondition/2, postcondition/3, - next_state/3]). + next_state/3]). -export([read_config/1, connect_carol/1, @@ -24,7 +24,7 @@ -export([ct_config_giver/1]). -record(state, {carol, - geralt, + geralt, msgs_to_carol, msgs_to_geralt, config_pid}). @@ -37,16 +37,16 @@ sample() -> prop(Config) -> Pid = spawn_link(?MODULE, ct_config_giver, [Config]), - ?FORALL(Cmds, commands(?MODULE, initial_state(Pid)), - ?TRAPEXIT( - begin - {History,State,Result} = run_commands(?MODULE, Cmds), + ?FORALL(Cmds, commands(?MODULE, initial_state(Pid)), + ?TRAPEXIT( + begin + {History, State, Result} = run_commands(?MODULE, Cmds), maybe_stop_client(State#state.carol), maybe_stop_client(State#state.geralt), - ?WHENFAIL(ct:pal(error, "History: ~p~nState: ~p\nResult: ~p~n", - [History,State,Result]), - aggregate(command_names(Cmds), Result =:= ok)) - end)). + ?WHENFAIL(ct:log(error, "History: ~p~nState: ~p\nResult: ~p~n", + [History, State, Result]), + aggregate(command_names(Cmds), Result =:= ok)) + end)). ct_config_giver(Config) -> receive @@ -60,8 +60,8 @@ maybe_stop_client(Client) -> escalus_client:stop(Client). initial_state(Pid) -> - #state{carol = undefined, - geralt = undefined, + #state{carol = undefined, + geralt = undefined, msgs_to_carol = [], msgs_to_geralt = [], config_pid = Pid}. @@ -95,16 +95,12 @@ next_state(S, V, {call, _, connect_carol, [_]}) -> next_state(S, V, {call, _, connect_geralt, [_]}) -> S#state{geralt = V}; next_state(#state{msgs_to_geralt = Msgs} = S, V, {call, _, send_from_carol, _}) -> - % ct:pal("msgs to geralt: ~p", [[V | Msgs]]), S#state{msgs_to_geralt = [V | Msgs]}; next_state(#state{msgs_to_carol = Msgs} = S, V, {call, _, send_from_geralt, _}) -> - % ct:pal("msgs to carol: ~p", [[V | Msgs]]), S#state{msgs_to_carol = [V | Msgs]}; next_state(S, _, {call, _, wait_for_msgs_carol, _}) -> - % ct:pal("msgs to carol2: ~p", [S#state.msgs_to_carol]), S#state{msgs_to_carol = []}; next_state(S, _, {call, _, wait_for_msgs_geralt, _}) -> - % ct:pal("msgs to geralt2: ~p", [S#state.msgs_to_geralt]), S#state{msgs_to_geralt = []}; next_state(S, _, _) -> S. @@ -115,7 +111,7 @@ read_config(Pid) -> {ok, Config} -> Config after 100 -> - error + error end. connect_carol(Pid) -> @@ -168,7 +164,6 @@ wait_for_msgs_carol(Carol, Msgs) -> RMsgs = [exml_query:path(E, [{element, <<"body">>}, cdata]) || E <- Stanzas], SortedMsgs = lists:sort([Msg || {_, Msg} <- Msgs]), - ct:pal("waiting for: ~p~nreceived: ~p", [SortedMsgs, RMsgs]), SortedMsgs = lists:sort(RMsgs), ok. From 96098126e7eabc1dafa0898c5fa24d0fd1af21a0 Mon Sep 17 00:00:00 2001 From: Michal Piotrowski Date: Fri, 30 Sep 2016 14:35:26 +0200 Subject: [PATCH 11/11] use PropEr 1.2 in tests as well --- test/ejabberd_tests/rebar.config | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ejabberd_tests/rebar.config b/test/ejabberd_tests/rebar.config index b1b08178c50..b47a1403f20 100644 --- a/test/ejabberd_tests/rebar.config +++ b/test/ejabberd_tests/rebar.config @@ -11,7 +11,7 @@ {katt, ".*", {git, "git://github.com/for-GET/katt.git", {tag, "1.5.2"}}}, {erlsh, ".*", {git, "git://github.com/proger/erlsh.git", "2fce513"}}, {jiffy, ".*", {git, "git://github.com/davisp/jiffy.git", "0.14.8"}}, - {proper, ".*", {git, "https://github.com/manopapad/proper.git", "20e62bc32f9bd43fe2ff52944a4ef99eb71d1399"}}, + {proper, ".*", {git, "https://github.com/manopapad/proper.git", "v1.2"}}, {cowboy, ".*", {git, "git://github.com/ninenines/cowboy.git", {tag, "1.0.4"}}} ]}.