Simplify stream management suite #3453

arcusfelis · 2021-12-14T16:07:17Z

Proposed changes include:

Remove code duplication by introducing a function connect_fresh and connect_spec.
Explicitly specify which cases use manual_ack, when creating a connection
Make sm_helper module with helpers

Motivation:

that suite contains a lot of escalus_connection:connect calls. And 3 lines syntax makes harder to compare test cases.

Reduce complexity to keep in mind that:

sm is enabled in test.config, unless we provide steps separately.
manual_ack could be specified in the group settings or in test cases. Once we starting to pass Spec, it is harder to keep in mind which connections need manual acking.
connections_steps_to_something is harder to read comparing to just something.
a separate sm_helper allows to focus just on test cases and CT init logic.

…ing_for_result

codecov · 2021-12-14T16:16:07Z

Codecov Report

Merging #3453 (0538006) into master (96f2fd6) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 0538006 differs from pull request most recent head 7bb63ff. Consider uploading reports for the commit 7bb63ff to get more accurate results

@@            Coverage Diff             @@
##           master    #3453      +/-   ##
==========================================
- Coverage   80.86%   80.84%   -0.02%     
==========================================
  Files         415      415              
  Lines       32312    32312              
==========================================
- Hits        26129    26124       -5     
- Misses       6183     6188       +5

Impacted Files	Coverage Δ
src/cassandra/mongoose_cassandra.erl	`77.77% <0.00%> (-3.71%)`	⬇️
src/mod_private.erl	`79.66% <0.00%> (-3.39%)`	⬇️
src/cassandra/mongoose_cassandra_worker.erl	`65.72% <0.00%> (-2.82%)`	⬇️
src/logger/mongoose_log_filter.erl	`78.08% <0.00%> (-1.37%)`	⬇️
src/rdbms/mongoose_rdbms.erl	`62.94% <0.00%> (-0.80%)`	⬇️
src/ejabberd_c2s.erl	`89.34% <0.00%> (-0.08%)`	⬇️
src/pubsub/mod_pubsub.erl	`73.13% <0.00%> (-0.06%)`	⬇️
src/mod_muc_log.erl	`78.11% <0.00%> (ø)`
src/pubsub/mod_pubsub_db_rdbms.erl	`95.34% <0.00%> (+0.25%)`	⬆️
src/mod_muc_room.erl	`77.09% <0.00%> (+0.34%)`	⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96f2fd6...7bb63ff. Read the comment docs.

NelsonVides

Amazing clean-up, thank you!
Just a few comments below :)

big_tests/tests/mongoose_helper.erl

big_tests/tests/sm_SUITE.erl

Code style

mongoose-im · 2021-12-16T12:28:25Z

small_tests_24 / small_tests / 9a15e27
Reports root / small

small_tests_23 / small_tests / 9a15e27
Reports root / small

dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 9a15e27
Reports root/ big
OK: 2727 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0

dynamic_domains_mysql_redis_24 / mysql_redis / 9a15e27
Reports root/ big
OK: 2710 / Failed: 0 / User-skipped: 203 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 9a15e27
Reports root/ big
OK: 2727 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 9a15e27
Reports root/ big
OK: 2727 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0

ldap_mnesia_23 / ldap_mnesia / 9a15e27
Reports root/ big
OK: 1503 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0

ldap_mnesia_24 / ldap_mnesia / 9a15e27
Reports root/ big
OK: 1503 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0

internal_mnesia_24 / internal_mnesia / 9a15e27
Reports root/ big
OK: 1588 / Failed: 0 / User-skipped: 301 / Auto-skipped: 0

elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 9a15e27
Reports root/ big
OK: 1881 / Failed: 0 / User-skipped: 313 / Auto-skipped: 0

mysql_redis_24 / mysql_redis / 9a15e27
Reports root/ big
OK: 3109 / Failed: 0 / User-skipped: 200 / Auto-skipped: 0

pgsql_mnesia_23 / pgsql_mnesia / 9a15e27
Reports root/ big
OK: 3114 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / 9a15e27
Reports root/ big
OK: 3114 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0

mssql_mnesia_24 / odbc_mssql_mnesia / 9a15e27
Reports root/ big
OK: 3114 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / 9a15e27
Reports root/ big
OK: 1727 / Failed: 0 / User-skipped: 314 / Auto-skipped: 0

NelsonVides · 2021-12-16T18:16:08Z

big_tests/tests/sm_helper.erl

+    receive
+        {'DOWN', MRef, _Type, _C2SPid, _Info} ->
+            ok
+    after 5000 ->
+              ct:fail(wait_for_process_termination_timeout)
+    end,
+    ok.


Suggested change

receive

{'DOWN', MRef, _Type, _C2SPid, _Info} ->

ok

after 5000 ->

ct:fail(wait_for_process_termination_timeout)

end,

ok.

receive

{'DOWN', MRef, _Type, _C2SPid, _Info} ->

ok

after 5000 ->

ct:fail(wait_for_process_termination_timeout)

end.

NelsonVides · 2021-12-16T18:16:59Z

big_tests/tests/sm_SUITE.erl

@@ -1,22 +1,38 @@
+%% Session Management tests


Not needed

Suggested change

%% Session Management tests

omg, actually needed.
It is Stream Management.

There are two things called SM in MongooseIM (Stream and Session).

mongoose-im · 2021-12-17T09:46:05Z

small_tests_24 / small_tests / 0538006
Reports root / small

small_tests_23 / small_tests / 0538006
Reports root / small

dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 0538006
Reports root/ big
OK: 2727 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 0538006
Reports root/ big
OK: 2727 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 0538006
Reports root/ big
OK: 2727 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0

dynamic_domains_mysql_redis_24 / mysql_redis / 0538006
Reports root/ big
OK: 2710 / Failed: 0 / User-skipped: 203 / Auto-skipped: 0

ldap_mnesia_23 / ldap_mnesia / 0538006
Reports root/ big
OK: 1503 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0

ldap_mnesia_24 / ldap_mnesia / 0538006
Reports root/ big
OK: 1503 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0

internal_mnesia_24 / internal_mnesia / 0538006
Reports root/ big
OK: 1588 / Failed: 0 / User-skipped: 301 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / 0538006
Reports root/ big
OK: 3114 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0

elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 0538006
Reports root/ big
OK: 1881 / Failed: 0 / User-skipped: 313 / Auto-skipped: 0

pgsql_mnesia_23 / pgsql_mnesia / 0538006
Reports root/ big
OK: 3114 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0

mssql_mnesia_24 / odbc_mssql_mnesia / 0538006
Reports root/ big
OK: 3114 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0

mysql_redis_24 / mysql_redis / 0538006
Reports root/ big
OK: 3109 / Failed: 0 / User-skipped: 200 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / 0538006
Reports root/ big
OK: 1727 / Failed: 0 / User-skipped: 314 / Auto-skipped: 0

Enable preserve_order testcase (it was not running) Fix resume_expired_session_returns_correct_h Define suite() callback first

The proper fix would require extra tests to reproduce alternative orders reliably and fix them

mongoose-im · 2021-12-17T10:14:26Z

small_tests_24 / small_tests / 7bb63ff
Reports root / small

small_tests_23 / small_tests / 7bb63ff
Reports root / small

dynamic_domains_mysql_redis_24 / mysql_redis / 7bb63ff
Reports root/ big
OK: 2707 / Failed: 0 / User-skipped: 203 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 7bb63ff
Reports root/ big
OK: 2724 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 7bb63ff
Reports root/ big
OK: 2724 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 7bb63ff
Reports root/ big
OK: 2724 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0

ldap_mnesia_24 / ldap_mnesia / 7bb63ff
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0

internal_mnesia_24 / internal_mnesia / 7bb63ff
Reports root/ big
OK: 1585 / Failed: 0 / User-skipped: 301 / Auto-skipped: 0

ldap_mnesia_23 / ldap_mnesia / 7bb63ff
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0

elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 7bb63ff
Reports root/ big
OK: 1878 / Failed: 0 / User-skipped: 313 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / 7bb63ff
Reports root/ big
OK: 3111 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0

mysql_redis_24 / mysql_redis / 7bb63ff
Reports root/ big
OK: 3106 / Failed: 0 / User-skipped: 200 / Auto-skipped: 0

mssql_mnesia_24 / odbc_mssql_mnesia / 7bb63ff
Reports root/ big
OK: 3111 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0

pgsql_mnesia_23 / pgsql_mnesia / 7bb63ff
Reports root/ big
OK: 3111 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / 7bb63ff
Reports root/ big
OK: 1724 / Failed: 0 / User-skipped: 314 / Auto-skipped: 0

NelsonVides

Excellent 👌🏽

arcusfelis added 15 commits December 14, 2021 14:57

Make some common functions

6496381

Use connect_fresh function

61c294c

Cleaner short cases with connect_fresh

2e9b40a

Use connect_fresh for simple cases

7cafc30

Remove sleep in preserve_order

e3c0fe9

Use connect_spec

fff5c6d

Add connect_same

6cb92ee

Simplify buffer_unacked_messages_and_die

927ee04

Specify manual_ack each time

31a3eb5

Provide ack argument into connect_fresh

c406be8

More connect_spec usage

48f99df

Remove node argument from get_session_pid

00d4153

Remove wait_for_c2s_state

2705b1a

Move wait_for_c2s_state_name into mongoose_helper

f202966

Use connect_same in kill_and_connect_with_resume_session_without_wait…

aaee737

…ing_for_result

arcusfelis force-pushed the mu-simplify-sm-suite branch from a2a6e31 to aaee737 Compare December 14, 2021 16:08

This comment has been minimized.

Sign in to view

arcusfelis added the waiting-for-review label Dec 15, 2021

NelsonVides reviewed Dec 15, 2021

View reviewed changes

big_tests/tests/mongoose_helper.erl Outdated Show resolved Hide resolved

big_tests/tests/mongoose_helper.erl Outdated Show resolved Hide resolved

big_tests/tests/sm_SUITE.erl Outdated Show resolved Hide resolved

arcusfelis added 6 commits December 15, 2021 15:23

Use chat_to_short_jid where get_bjid has been used before

eb38790

Exterminate fully-qualified calls in mongoose_helper

edecf71

Remove default timer:seconds from wait_for_c2s_unacked_count

ced1ded

Code style

Move functions into sm_helper

94d43ae

Use connect_fresh with server_returns_failed

7f9159c

Remove clear_session_table

82b7113

This comment has been minimized.

Sign in to view

Get rid of last manual_ack confusion

e00a9ef

This comment has been minimized.

Sign in to view

arcusfelis added the WIP 🚧 label Dec 15, 2021

arcusfelis force-pushed the mu-simplify-sm-suite branch 2 times, most recently from bc0c50b to 2dafd14 Compare December 16, 2021 00:06

This comment has been minimized.

Sign in to view

mongoose-im mentioned this pull request Dec 16, 2021

CI build failures summary #3360

Closed

arcusfelis removed the WIP 🚧 label Dec 16, 2021

This comment has been minimized.

Sign in to view

NelsonVides reviewed Dec 16, 2021

View reviewed changes

arcusfelis added 4 commits December 17, 2021 11:02

Remove exports from sm_SUITE

3d7cf11

Enable preserve_order testcase (it was not running) Fix resume_expired_session_returns_correct_h Define suite() callback first

Dont import rarely used functions

b0b9748

Enforce order in preserve_order

27e5068

The proper fix would require extra tests to reproduce alternative orders reliably and fix them

Print expected and received texts in sm_helper:assert_messages

7bb63ff

arcusfelis force-pushed the mu-simplify-sm-suite branch from 0538006 to 7bb63ff Compare December 17, 2021 10:06

arcusfelis changed the title ~~Simplify sm suite~~ Simplify stream management suite Dec 17, 2021

arcusfelis requested a review from NelsonVides December 17, 2021 10:08

NelsonVides approved these changes Dec 17, 2021

View reviewed changes

NelsonVides merged commit 367f27f into master Dec 17, 2021

NelsonVides deleted the mu-simplify-sm-suite branch December 17, 2021 10:59

Premwoik added this to the 5.1.0 milestone May 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify stream management suite #3453

Simplify stream management suite #3453

arcusfelis commented Dec 14, 2021 •

edited

Loading

This comment has been minimized.

codecov bot commented Dec 14, 2021 •

edited

Loading

This comment has been minimized.

NelsonVides left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mongoose-im commented Dec 16, 2021 •

edited

Loading

NelsonVides Dec 16, 2021

NelsonVides Dec 16, 2021

arcusfelis Dec 17, 2021

mongoose-im commented Dec 17, 2021 •

edited

Loading

mongoose-im commented Dec 17, 2021 •

edited

Loading

NelsonVides left a comment

Simplify stream management suite #3453

Simplify stream management suite #3453

Conversation

arcusfelis commented Dec 14, 2021 • edited Loading

This comment has been minimized.

codecov bot commented Dec 14, 2021 • edited Loading

Codecov Report

This comment has been minimized.

NelsonVides left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mongoose-im commented Dec 16, 2021 • edited Loading

NelsonVides Dec 16, 2021

Choose a reason for hiding this comment

NelsonVides Dec 16, 2021

Choose a reason for hiding this comment

arcusfelis Dec 17, 2021

Choose a reason for hiding this comment

mongoose-im commented Dec 17, 2021 • edited Loading

mongoose-im commented Dec 17, 2021 • edited Loading

NelsonVides left a comment

Choose a reason for hiding this comment

arcusfelis commented Dec 14, 2021 •

edited

Loading

codecov bot commented Dec 14, 2021 •

edited

Loading

mongoose-im commented Dec 16, 2021 •

edited

Loading

mongoose-im commented Dec 17, 2021 •

edited

Loading

mongoose-im commented Dec 17, 2021 •

edited

Loading