Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Domain admin tests in roster suite #3736

Merged
merged 1 commit into from
Sep 9, 2022
Merged

Conversation

jacekwegr
Copy link
Collaborator

Added tests for domain admin in graphql_roster_SUITE. The tests are similar to those for a global admin, but they also check for permissions.

@mongoose-im

This comment was marked as outdated.

@codecov
Copy link

codecov bot commented Aug 18, 2022

Codecov Report

Merging #3736 (ecd2cf3) into master (918421a) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3736      +/-   ##
==========================================
- Coverage   82.70%   82.69%   -0.01%     
==========================================
  Files         529      529              
  Lines       33947    33947              
==========================================
- Hits        28075    28073       -2     
- Misses       5872     5874       +2     
Impacted Files Coverage Δ
src/pubsub/node_pep.erl 77.77% <0.00%> (-1.86%) ⬇️
src/inbox/mod_inbox_rdbms_async.erl 72.05% <0.00%> (-1.48%) ⬇️
src/mod_muc_log.erl 62.82% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.


domain_admin_set_mutual_subscription_try_connect_users_no_permission(Config) ->
escalus:fresh_story_with_config(Config, [{alice_bis, 1}, {bob, 1}],
fun domain_admin_set_mutual_subscription_try_connect_users_no_permission_story/3).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please try to stick to the 100 characters per line limit.

Copy link
Contributor

@JanuszJakubiec JanuszJakubiec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, however, I think that copying all tests for domain_admin tests is unnecessary. A sufficient amount would be to test each graphql function two times for domain admin(one when it passes and one when an authentication error occurs). More details I will give later during the call.

@jacekwegr jacekwegr force-pushed the domain-admin-roster-tests branch from 721e9f8 to 4367b5f Compare August 19, 2022 08:23
@mongoose-im

This comment was marked as outdated.

@jacekwegr jacekwegr marked this pull request as ready for review August 19, 2022 13:15
@jacekwegr jacekwegr force-pushed the domain-admin-roster-tests branch from 4367b5f to c368c51 Compare August 19, 2022 14:41
@mongoose-im

This comment was marked as outdated.

@jacekwegr jacekwegr force-pushed the domain-admin-roster-tests branch from c368c51 to b10550e Compare August 24, 2022 15:01
@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@jacekwegr jacekwegr force-pushed the domain-admin-roster-tests branch from 44684ac to 6aca4dd Compare August 26, 2022 09:25
@mongoose-im

This comment was marked as outdated.

Copy link
Member

@chrzaszcz chrzaszcz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I like that all cases are checked, even the obvious ones (e.g. for existing and non-existent user/domain combinations). This is more than I expected, but it's almost always good to test more.

Res = admin_subscribe_to_all(Alice, [Bob, Kate], Config),
check_if_created_succ(?SUBSCRIBE_TO_ALL_PATH, Res, [true, false]),
?assertNotEqual(nomatch, binary:match(get_err_msg(Res), <<"does not exist">>)),

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove this empty line

Added tests for domain admin in graphql_roster_SUITE. The tests are similar to those for a global admin, but they also check for permissions.
@jacekwegr jacekwegr force-pushed the domain-admin-roster-tests branch from 6aca4dd to ecd2cf3 Compare September 8, 2022 07:12
@mongoose-im
Copy link
Collaborator

mongoose-im commented Sep 8, 2022

small_tests_24 / small_tests / ecd2cf3
Reports root / small


small_tests_25 / small_tests / ecd2cf3
Reports root / small


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / ecd2cf3
Reports root/ big
OK: 3578 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / ecd2cf3
Reports root/ big
OK: 1943 / Failed: 0 / User-skipped: 551 / Auto-skipped: 0


dynamic_domains_mysql_redis_25 / mysql_redis / ecd2cf3
Reports root/ big
OK: 3561 / Failed: 0 / User-skipped: 105 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / ecd2cf3
Reports root/ big
OK: 3578 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0


ldap_mnesia_25 / ldap_mnesia / ecd2cf3
Reports root/ big
OK: 1943 / Failed: 0 / User-skipped: 551 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / ecd2cf3
Reports root/ big
OK: 3578 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / ecd2cf3
Reports root/ big
OK: 3952 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0


internal_mnesia_25 / internal_mnesia / ecd2cf3
Reports root/ big
OK: 2064 / Failed: 0 / User-skipped: 430 / Auto-skipped: 0


elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / ecd2cf3
Reports root/ big
OK: 2404 / Failed: 0 / User-skipped: 425 / Auto-skipped: 0


pgsql_mnesia_25 / pgsql_mnesia / ecd2cf3
Reports root/ big
OK: 3952 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0


mysql_redis_25 / mysql_redis / ecd2cf3
Reports root/ big
OK: 3947 / Failed: 0 / User-skipped: 102 / Auto-skipped: 0


mssql_mnesia_25 / odbc_mssql_mnesia / ecd2cf3
Reports root/ big
OK: 3951 / Failed: 1 / User-skipped: 97 / Auto-skipped: 0

push_integration_SUITE:pubsub_ful:pm_notifications_with_inbox:inbox_msg_reset_unread_count_fcm
{error,
  {{assertMatch,
     [{module,push_integration_SUITE},
      {line,662},
      {expression,"Data"},
      {pattern,"# { << \"message-count\" >> := ExpectedCount }"},
      {value,
        #{<<"last-message-body">> => <<"SECOND MESSAGE">>,
        <<"last-message-sender">> =>
          <<"alice_inbox_msg_reset_unread_count_fcm_2611@localhost">>,
        <<"message-count">> => 1}}]},
   [{push_integration_SUITE,check_notification,2,
      [{file,
         "/home/circleci/project/big_tests/tests/push_integration_SUITE.erl"},
       {line,662}]},
    {push_integration_SUITE,'-inbox_msg_reset_unread_count/3-fun-0-',5,
      [{file,
         "/home/circleci/project/big_tests/tests/push_integration_SUITE.erl"},
       {line,598}]},
    {escalus_story,story,4,
      [{file,
         "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
       {line,72}]},
    {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
    {test_server,run_test_case_eval1,6,
      [{file,"test_server.erl"},{line,1291}]},
    {test_server,run_test_case_eval,9,
      [{file,"test_server.erl"},{line,1223}]}]}}

Report log


riak_mnesia_24 / riak_mnesia / ecd2cf3
Reports root/ big
OK: 2243 / Failed: 0 / User-skipped: 418 / Auto-skipped: 0


mssql_mnesia_25 / odbc_mssql_mnesia / ecd2cf3
Reports root/ big
OK: 3952 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0

@JanuszJakubiec JanuszJakubiec merged commit 77951fc into master Sep 9, 2022
@JanuszJakubiec JanuszJakubiec deleted the domain-admin-roster-tests branch September 9, 2022 06:36
@chrzaszcz chrzaszcz added this to the 6.0.0 milestone Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants