Improve mod_pubsub test coverage #1241

fenek · 2017-03-24T12:16:34Z

New PR for rebased+squashed branch

gadgetci · 2017-03-24T12:19:14Z

apps/ejabberd/src/mod_pubsub.erl

+                                                                   case Affiliation of
+                                                                       owner ->
+                                                                           NewOwner = jid:to_bare(JID),
+                                                                           NewOwners = [NewOwner | Owners],


According to Elvis:

Line 2721 is too long: NewOwners = [NewOwner | Owners],.

gadgetci · 2017-03-24T12:19:15Z

apps/ejabberd/src/mod_pubsub.erl

+                                                                                   NewOwners = Owners -- [OldOwner],
+                                                                                   tree_call(Host,
+                                                                                             set_node,
+                                                                                             [N#pubsub_node{owners = NewOwners}]);


According to Elvis:

Line 2732 is too long: [N#pubsub_node{owners = NewOwners}]);.

gadgetci · 2017-03-24T12:19:16Z

apps/ejabberd/src/mod_pubsub.erl

+                                                                       attrs = [{<<"jid">>, jid:to_binary(Unchanged)}, {<<"affiliation">>, <<"owner">>}] }
+                                                               || Unchanged <- Owners ],
+                                             AffiliationsPayload = #xmlel{ name = <<"affiliations">>, attrs = [{<<"node">>, NodeId}],
+                                                                           children = OwnersPayload },


According to Elvis:

Line 2711 is too long: children = OwnersPayload },.

gadgetci · 2017-03-24T12:19:17Z

apps/ejabberd/src/mod_pubsub.erl

+                                             AffiliationsPayload = #xmlel{ name = <<"affiliations">>, attrs = [{<<"node">>, NodeId}],
+                                                                           children = OwnersPayload },
+                                             NewPubSubPayload = #xmlel{ name = <<"pubsub">>, attrs = [{<<"xmlns">>, ?NS_PUBSUB_OWNER}],
+                                                                        children = [AffiliationsPayload] },


According to Elvis:

Line 2713 is too long: children = [AffiliationsPayload] },.

gadgetci · 2017-03-24T12:19:17Z

apps/ejabberd/src/mod_pubsub.erl

@@ -2660,41 +2690,56 @@ set_affiliations(Host, Node, From, EntitiesEls) ->
        error ->
            {error, ?ERR_BAD_REQUEST};
        _ ->
-            Action = fun (#pubsub_node{type = Type, id = Nidx, owners = O} = N) ->
+            Action = fun (#pubsub_node{type = Type, id = Nidx, owners = O, nodeid = {_, NodeId}} = N) ->


According to Elvis:

Line 2693 is too long: Action = fun (#pubsub_node{type = Type, id = Nidx, owners = O, nodeid = {_, NodeId}} = N) ->.

gadgetci · 2017-03-24T12:19:18Z

apps/ejabberd/src/mod_pubsub.erl

                                                                           tree_call(Host,
                                                                                     set_node,
                                                                                     [N#pubsub_node{owners = NewOwners}]);
+                                                                       none ->
+                                                                           OldOwner = jid:to_bare(JID),


According to Elvis:

Line 2726 is too long: OldOwner = jid:to_bare(JID),.

gadgetci · 2017-03-24T12:19:19Z

apps/ejabberd/src/mod_pubsub.erl

+                                             {error, {?ERR_NOT_ACCEPTABLE, [NewPubSubPayload]}};
+                                         _ ->
+                                             lists:foreach(fun ({JID, Affiliation}) ->
+                                                                   node_call(Host, Type, set_affiliation, [Nidx, JID, Affiliation]),


According to Elvis:

Line 2717 is too long: node_call(Host, Type, set_affiliation, [Nidx, JID, Affiliation]),.

gadgetci · 2017-03-24T12:19:20Z

apps/ejabberd/src/mod_pubsub.erl

+                                     % other invalid affiliation transitions
+                                     OwnersDryRun = lists:foldl(
+                                                      fun({JID, owner}, Acc) ->
+                                                              sets:add_element(jid:to_bare(JID), Acc);


According to Elvis:

Line 2701 is too long: sets:add_element(jid:to_bare(JID), Acc);.

gadgetci · 2017-03-24T12:19:21Z

apps/ejabberd/src/mod_pubsub.erl

                                                                           tree_call(Host,
                                                                                     set_node,
                                                                                     [N#pubsub_node{owners = NewOwners}]);
+                                                                       none ->
+                                                                           OldOwner = jid:to_bare(JID),
+                                                                           case lists:member(OldOwner, Owners) of


According to Elvis:

Line 2727 is too long: case lists:member(OldOwner, Owners) of.

gadgetci · 2017-03-24T12:19:22Z

apps/ejabberd/src/mod_pubsub.erl

@@ -2876,6 +2924,7 @@ get_subscriptions(Host, Node, JID, Plugins) when is_list(Plugins) ->
                                                _ ->
                                                    []
                                            end;
+                                         %% used by node_flat (therefore by dag, hometree and push as well)


According to Elvis:

Line 2927 is too long: %% used by node_flat (therefore by dag, hometree and push as well).

gadgetci · 2017-03-24T12:19:23Z

apps/ejabberd/src/mod_pubsub.erl

+                                     case sets:size(OwnersDryRun) of
+                                         0 ->
+                                             OwnersPayload = [ #xmlel{ name = <<"affiliation">>,
+                                                                       attrs = [{<<"jid">>, jid:to_binary(Unchanged)}, {<<"affiliation">>, <<"owner">>}] }


According to Elvis:

Line 2708 is too long: attrs = [{<<"jid">>, jid:to_binary(Unchanged)}, {<<"affiliation">>, <<"owner">>}] }.

gadgetci · 2017-03-24T12:19:24Z

apps/ejabberd/src/mod_pubsub.erl

+                                                               || Unchanged <- Owners ],
+                                             AffiliationsPayload = #xmlel{ name = <<"affiliations">>, attrs = [{<<"node">>, NodeId}],
+                                                                           children = OwnersPayload },
+                                             NewPubSubPayload = #xmlel{ name = <<"pubsub">>, attrs = [{<<"xmlns">>, ?NS_PUBSUB_OWNER}],


According to Elvis:

Line 2712 is too long: NewPubSubPayload = #xmlel{ name = <<"pubsub">>, attrs = [{<<"xmlns">>, ?NS_PUBSUB_OWNER}],.

gadgetci · 2017-03-24T12:19:25Z

apps/ejabberd/src/mod_pubsub.erl

+                                             OwnersPayload = [ #xmlel{ name = <<"affiliation">>,
+                                                                       attrs = [{<<"jid">>, jid:to_binary(Unchanged)}, {<<"affiliation">>, <<"owner">>}] }
+                                                               || Unchanged <- Owners ],
+                                             AffiliationsPayload = #xmlel{ name = <<"affiliations">>, attrs = [{<<"node">>, NodeId}],


According to Elvis:

Line 2710 is too long: AffiliationsPayload = #xmlel{ name = <<"affiliations">>, attrs = [{<<"node">>, NodeId}],.

gadgetci · 2017-03-24T12:19:25Z

apps/ejabberd/src/mod_pubsub.erl

                                         ({_, none, _}) ->
                                            [];
+                                         %% sometimes used by node_pep
                                         ({#pubsub_node{nodeid = {_, SubsNode}}, Sub, SubId, SubJID}) ->


According to Elvis:

Line 2909 is too long: ({#pubsub_node{nodeid = {_, SubsNode}}, Sub, SubId, SubJID}) ->.

gadgetci · 2017-03-24T12:19:26Z

apps/ejabberd/src/mod_pubsub.erl

+                                                                   node_call(Host, Type, set_affiliation, [Nidx, JID, Affiliation]),
+                                                                   case Affiliation of
+                                                                       owner ->
+                                                                           NewOwner = jid:to_bare(JID),


According to Elvis:

Line 2720 is too long: NewOwner = jid:to_bare(JID),.

gadgetci · 2017-03-24T12:19:27Z

apps/ejabberd/src/mod_pubsub.erl

@@ -1526,7 +1556,7 @@ send_pending_auth_events(Request, Host, Node, Owner) ->
 send_authorization_request(#pubsub_node{nodeid = {Host, Node}, type = Type, id = Nidx, owners = O},
                           Subscriber) ->
    Lang = <<"en">>,
-    Stanza = #xmlel{name = <<"message">>, attrs = [],
+    Stanza = #xmlel{name = <<"message">>, attrs = [{<<"id">>, list_to_binary(randoms:get_string())}],


According to Elvis:

Line 1559 is too long: Stanza = #xmlel{name = <<"message">>, attrs = [{<<"id">>, list_to_binary(randoms:get_string())}],.

gadgetci · 2017-03-24T12:19:28Z

apps/ejabberd/src/mod_pubsub.erl

@@ -1005,6 +1034,8 @@ do_route(ServerHost, Access, Plugins, Host, From, To, Packet) ->
                                  of
                                      {result, IQRes} ->
                                          jlib:iq_to_xml(IQ#iq{type = result, sub_el = IQRes});
+                                      {error, {Error, NewPayload}} ->
+                                          jlib:make_error_reply(Packet#xmlel{ children = NewPayload }, Error);


According to Elvis:

Line 1038 is too long: jlib:make_error_reply(Packet#xmlel{ children = NewPayload }, Error);.

gadgetci · 2017-03-24T12:19:29Z

apps/ejabberd/src/mod_pubsub.erl

@@ -2876,6 +2924,7 @@ get_subscriptions(Host, Node, JID, Plugins) when is_list(Plugins) ->
                                                _ ->
                                                    []
                                            end;
+                                         %% used by node_flat (therefore by dag, hometree and push as well)
                                         ({#pubsub_node{nodeid = {_, SubsNode}}, Sub, SubJID}) ->
                                            case Node of


According to Elvis:

The expression on line 2929 and column 45 is nested beyond the maximum level of 3.

gadgetci · 2017-03-24T12:19:30Z

apps/ejabberd/src/mod_pubsub.erl

+                                                                   case Affiliation of
+                                                                       owner ->
+                                                                           NewOwner = jid:to_bare(JID),
+                                                                           NewOwners = [NewOwner | Owners],
                                                                           tree_call(Host,
                                                                                     set_node,
                                                                                     [N#pubsub_node{owners = NewOwners}]);


According to Elvis:

Line 2724 is too long: [N#pubsub_node{owners = NewOwners}]);.

gadgetci · 2017-03-24T12:19:31Z

apps/ejabberd/src/mod_pubsub.erl

@@ -2660,41 +2690,56 @@ set_affiliations(Host, Node, From, EntitiesEls) ->
        error ->
            {error, ?ERR_BAD_REQUEST};
        _ ->
-            Action = fun (#pubsub_node{type = Type, id = Nidx, owners = O} = N) ->
+            Action = fun (#pubsub_node{type = Type, id = Nidx, owners = O, nodeid = {_, NodeId}} = N) ->
                             Owners = node_owners_call(Host, Type, Nidx, O),
                             case lists:member(Owner, Owners) of


According to Elvis:

The expression on line 2695 and column 30 is nested beyond the maximum level of 3.

gadgetci · 2017-03-24T12:19:32Z

apps/ejabberd/src/mod_pubsub.erl

                                         ({_, none, _}) ->
                                            [];
+                                         %% sometimes used by node_pep
                                         ({#pubsub_node{nodeid = {_, SubsNode}}, Sub, SubId, SubJID}) ->
                                            case Node of


According to Elvis:

The expression on line 2910 and column 45 is nested beyond the maximum level of 3.

gadgetci · 2017-03-24T12:19:33Z

apps/ejabberd/src/mod_pubsub.erl

+                                                                               true ->
+                                                                                   NewOwners = Owners -- [OldOwner],
+                                                                                   tree_call(Host,
+                                                                                             set_node,


According to Elvis:

Line 2731 is too long: set_node,.

gadgetci · 2017-03-24T12:19:34Z

apps/ejabberd/src/mod_pubsub.erl

+                                                      fun({JID, owner}, Acc) ->
+                                                              sets:add_element(jid:to_bare(JID), Acc);
+                                                         ({JID, _}, Acc) ->
+                                                              sets:del_element(jid:to_bare(JID), Acc)


According to Elvis:

Line 2703 is too long: sets:del_element(jid:to_bare(JID), Acc).

gadgetci · 2017-03-24T12:19:35Z

apps/ejabberd/src/mod_pubsub.erl

+                                                                           OldOwner = jid:to_bare(JID),
+                                                                           case lists:member(OldOwner, Owners) of
+                                                                               true ->
+                                                                                   NewOwners = Owners -- [OldOwner],


According to Elvis:

Line 2729 is too long: NewOwners = Owners -- [OldOwner],.

gadgetci · 2017-03-24T12:19:36Z

apps/ejabberd/src/mod_pubsub.erl

@@ -3742,11 +3793,12 @@ set_configure(Host, Node, From, Els, Lang) ->
                                                               end,
                                                     case set_xoption(Host, XData, OldOpts) of
                                                         NewOpts when is_list(NewOpts) ->
+                                                             NewNode = N#pubsub_node{options = NewOpts},


According to Elvis:

Line 3796 is too long: NewNode = N#pubsub_node{options = NewOpts},.

gadgetci · 2017-03-24T12:19:37Z

apps/ejabberd/src/mod_pubsub.erl

@@ -3758,7 +3810,7 @@ set_configure(Host, Node, From, Els, Lang) ->
                                     end
                             end,
                    case transaction(Host, Node, Action, transaction) of


According to Elvis:

The expression on line 3812 and column 21 is nested beyond the maximum level of 3.

gadgetci · 2017-03-29T10:39:34Z

apps/ejabberd/src/mod_pubsub.erl

@@ -3742,11 +3778,12 @@ set_configure(Host, Node, From, Els, Lang) ->
                                                               end,
                                                     case set_xoption(Host, XData, OldOpts) of
                                                         NewOpts when is_list(NewOpts) ->
+                                                             NewNode = N#pubsub_node{options = NewOpts},


According to Elvis:

Line 3781 is too long: NewNode = N#pubsub_node{options = NewOpts},.

gadgetci · 2017-03-29T10:39:35Z

apps/ejabberd/src/mod_pubsub.erl

@@ -3758,7 +3795,7 @@ set_configure(Host, Node, From, Els, Lang) ->
                                     end
                             end,
                    case transaction(Host, Node, Action, transaction) of


According to Elvis:

The expression on line 3797 and column 21 is nested beyond the maximum level of 3.

michalwski · 2017-03-30T12:59:02Z

This is massive coverage improvement! Thanks a lot!

fenek added tests WIP 🚧 labels Mar 24, 2017

gadgetci reviewed Mar 24, 2017

View reviewed changes

fenek force-pushed the pubsub_SUITE-to-cover-refactor_rebase branch from d1a225a to 135ef8e Compare March 28, 2017 14:11

Improve mod_pubsub test coverage

dce679c

fenek added 4 commits March 29, 2017 12:38

Refactor tests: pubsub_tools and pubsub_SUITE

9928098

Refactor pep_SUITE

3cf4617

Fix PubSub for Redis session backend

6c896d7

Revert shaper rate template

b46491d

fenek force-pushed the pubsub_SUITE-to-cover-refactor_rebase branch from 135ef8e to b46491d Compare March 29, 2017 10:39

gadgetci reviewed Mar 29, 2017

View reviewed changes

fenek added waiting-for-review and removed WIP 🚧 labels Mar 29, 2017

Use escalus @ 1091afe (master)

433b94e

michalwski merged commit 6fc47a8 into master Mar 30, 2017

michalwski deleted the pubsub_SUITE-to-cover-refactor_rebase branch March 30, 2017 12:58

fenek mentioned this pull request Jul 4, 2017

MongooseIM 2.1.0beta2 #1379

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve mod_pubsub test coverage #1241

Improve mod_pubsub test coverage #1241

fenek commented Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 24, 2017

gadgetci Mar 29, 2017

gadgetci Mar 29, 2017

michalwski commented Mar 30, 2017

Improve mod_pubsub test coverage #1241

Improve mod_pubsub test coverage #1241

Conversation

fenek commented Mar 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalwski commented Mar 30, 2017