Refactor mam suite #4237
Refactor mam suite #4237
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4237 +/- ##
==========================================
+ Coverage 84.00% 84.37% +0.36%
==========================================
Files 552 552
Lines 33531 33531
==========================================
+ Hits 28169 28293 +124
+ Misses 5362 5238 -124 ☔ View full report in Codecov by Sentry. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
We do this for other suites, and the long export lists are tedious to maintain.
This simplifies test initialisation, and makes the whole setup more predictable.
chrzaszcz
force-pushed
the
refactor-mam-suite
branch
from
fabb86f to
dfe5530
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Reasons: code reuse, DRY, maintainability Also: - Rework and simplify end_per_testcase - Minor fix: don't start room for a few non-MUC cases
chrzaszcz
force-pushed
the
refactor-mam-suite
branch
from
3d5ffe0 to
5ea8c57
Compare
|
elasticsearch_and_cassandra_26 / elasticsearch_and_cassandra_mnesia / 5ea8c57 small_tests_25 / small_tests / 5ea8c57 small_tests_26 / small_tests / 5ea8c57 small_tests_26_arm64 / small_tests / 5ea8c57 ldap_mnesia_25 / ldap_mnesia / 5ea8c57 dynamic_domains_mysql_redis_26 / mysql_redis / 5ea8c57 ldap_mnesia_26 / ldap_mnesia / 5ea8c57 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 5ea8c57 dynamic_domains_pgsql_mnesia_26 / pgsql_mnesia / 5ea8c57 internal_mnesia_26 / internal_mnesia / 5ea8c57 pgsql_cets_26 / pgsql_cets / 5ea8c57 mysql_redis_26 / mysql_redis / 5ea8c57 dynamic_domains_mssql_mnesia_26 / odbc_mssql_mnesia / 5ea8c57 pgsql_mnesia_25 / pgsql_mnesia / 5ea8c57 pgsql_mnesia_26 / pgsql_mnesia / 5ea8c57 mssql_mnesia_26 / odbc_mssql_mnesia / 5ea8c57 mssql_mnesia_26 / odbc_mssql_mnesia / 5ea8c57 |
| fresh_users(C) -> | ||
| case lists:member(C, all_cases_with_room()) of | ||
| true -> [{alice, 1}, {bob, 1}]; | ||
| false -> [] | ||
| end. |
There was a problem hiding this comment.
Sometimes we have a list of cases and we check membership, some others we have a macro with a list of conditions, and in another one we have the list of conditions without the macro, directly coded into the clause. Can we have the clauses more consistent? Perhaps a map where the keys are the test cases and the values are configs as maps for the case? Maybe that is also repetitive, but requires_pm_archive preparing three users one of which is carol is not descriptive enough either 🤔
Just thinking out loud, this suite is really huge and complex, it won't be obvious how to clean it all up at once.
There was a problem hiding this comment.
I wanted to have it short and concise, and I tested different options, and this one was the shortest one.
Are the clauses really hard to understand for you? Maps with keys would be a lot of additional boilerplate code.
The macro requires_pm_archive prepares three users, because bootstrap_archive needs exactly these three users.
There is consistency:
- macro is used when the list of conditions repeats,
- there is only one list:
all_cases_with_room, because this is the only case where I could use already existing groups of cases without creating new ones - and I didn't want to create groups with overlap, because they would be confusing. - in other cases, I explicitly listed the test cases.
The goal of this PR is to reduce code repetition and improve maintainability in
mam_SUITE, making it possible to easily introduce instrumentation tests.init_per_testcaseandend_per_testcase.