Reenable Elvis checks in CI #4598

tomaszwojcikowski · 2025-12-11T10:20:55Z

Updated Elvis rules.
Added a job for Elvis in the CircleCI configuration to automate style checks.

This PR also fixes a CircleCI “rerun failed tests” edge case in big tests. When CircleCI reruns only a subset of suites (e.g. graphql_stanza_SUITE), it rewrites the Common Test spec to include only the allowed suites. The existing spec-filtering script only removed {suites, …}, {groups, …}, {cases, …} entries and left skip_groups / skip_cases terms untouched. Those skip_* terms still reference suites that are no longer present in the filtered spec, and Common Test fails early with “Suite X not found” and ct_error {make_failed, …} (0 tests executed).

The fix extends the spec filtering in select_suites_to_run.erl to also drop skip_groups / skip_cases entries for suites that are not in the allowed set (and to rewrite skip_suites lists accordingly). This makes reruns deterministic and prevents “missing suites” crashes during partial reruns.

(Important nuance: the gap existed already; enabling/using rerun-on-failure is what exposed it.)

codecov · 2025-12-11T10:32:48Z

Codecov Report

❌ Patch coverage is 72.72727% with 69 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.04%. Comparing base (9eea324) to head (3c700cf).

Files with missing lines	Patch %	Lines
src/mongoose_xmpp_errors.erl	58.33%	20 Missing ⚠️
src/eldap_filter.erl	23.52%	13 Missing ⚠️
src/eldap_utils.erl	50.00%	10 Missing ⚠️
src/amp.erl	77.27%	5 Missing ⚠️
src/mam/mod_mam_muc.erl	33.33%	4 Missing ⚠️
src/pubsub/mod_pubsub.erl	71.42%	4 Missing ⚠️
src/ejabberd_ctl.erl	66.66%	2 Missing ⚠️
src/ejabberd_sm_redis.erl	81.81%	2 Missing ⚠️
.../event_pusher/mod_event_pusher_hook_translator.erl	50.00%	2 Missing ⚠️
src/async_pools/mongoose_aggregator_worker.erl	50.00%	1 Missing ⚠️
... and 6 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4598      +/-   ##
==========================================
- Coverage   86.06%   86.04%   -0.02%     
==========================================
  Files         563      563              
  Lines       33732    33761      +29     
==========================================
+ Hits        29031    29051      +20     
- Misses       4701     4710       +9

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

chrzaszcz

The changes look good, so we can merge this PR, but I found an important flaw in our elvis checks - see comments.

elvis.config

Fix OTP 27 dialyzer issues, normalize IQ specs, handle CircleCI failed-test reruns safely (spec filtering + avoid skewed Codecov uploads).

pawlooss1

Thanks for all the effort ❤️ I have left a few comments, mostly about indentation. I think we could also discuss with the team if we wanted to have a format check in the CI as well to prevent such indentation issues.

pawlooss1 · 2025-12-18T12:30:12Z

src/pubsub/mod_pubsub.erl

+-spec iq_command(binary(), any(), jid:jid(), jlib:iq(), any(), [binary()]) ->
+    {result, [exml:element()]} | {error, exml:element()}.


This spec should be above its function.

pawlooss1 · 2025-12-18T12:34:00Z

src/cert_utils.erl

    catch
        Class:Exception:StackTrace ->
-            log_exception(Cert, Class, Exception,StackTrace),
+                log_exception(Cert, Class, Exception, StackTrace),


Is this indent needed by elvis? It makes no sense that the indent is different than in the line bellow. I think that only the whitespace between argument was needed.

pawlooss1 · 2025-12-18T12:37:06Z

src/ejabberd_sm_mnesia.erl

    mnesia:dirty_select(
        session,
-          [{#session{usr = '$1', sid='$2', priority='$3', info='$4', _ = '_' },
+                    [{#session{usr = '$1', sid = '$2', priority = '$3', info = '$4', _ = '_'},


Similar to the previous comment, the indent looks weird.

pawlooss1 · 2025-12-18T12:41:17Z

src/eldap_utils.erl

+    TailLength = byte_size(P) - (First + 1),
+    binary:part(S, First, byte_size(S) - TailLength - First + 1)


This indent should be the same as in the line {First, _} = binary:match(P, <<"%u">>),, so the whole fun's body has the same indent.

pawlooss1 · 2025-12-18T12:43:46Z

src/eldap_utils.erl

 maybe_b2list(B) when is_binary(B) ->
-  binary_to_list(B);
+        binary_to_list(B);
 maybe_b2list(L) when is_list(L) ->
-  L;
+        L;
 maybe_b2list(O) ->
-  {error, {unknown_type, O}}.
+        {error, {unknown_type, O}}.

 maybe_list2b(L) when is_list(L) ->
-  list_to_binary(L);
+        list_to_binary(L);
 maybe_list2b(B) when is_binary(B) ->
-  B;
+        B;
 maybe_list2b(O) ->
-  {error, {unknown_type, O}}.
+        {error, {unknown_type, O}}.


Here, the indent should be as in all other functions, i.e. 4 spaces.

pawlooss1 · 2025-12-18T14:58:00Z

src/mod_bosh_socket.erl

+    _ = try mongoose_c2s:stop(S#state.c2s_pid, normal)
+        catch
+            _:_ -> ok
+        end,


This looks even worse than before IMO. However, when I looked at mongoose_c2s:stop/2 code it only calls gen_statem:cast/2 which according to documentation should return ok immediately. If you don't know a reason that it could crash then maybe we should remove this catch entirely?

pawlooss1 · 2025-12-18T15:07:07Z

test/component_reg_SUITE.erl

 opts() ->
    #{hosts => [],
-      host_types => [],   
+    host_types => [],


This indent feels wrong too.

mongoose-im · 2025-12-19T06:29:26Z

elasticsearch_and_cassandra_28 / elasticsearch_and_cassandra_mnesia / 1387311
Reports root/ big
OK: 683 / Failed: 0 / User-skipped: 72 / Auto-skipped: 0

small_tests_27 / small_tests / 1387311
Reports root / small

small_tests_28 / small_tests / 1387311
Reports root / small

small_tests_28_arm64 / small_tests / 1387311
Reports root / small

ldap_mnesia_27 / ldap_mnesia / 1387311
Reports root/ big
OK: 2358 / Failed: 0 / User-skipped: 1376 / Auto-skipped: 0

ldap_mnesia_28 / ldap_mnesia / 1387311
Reports root/ big
OK: 2358 / Failed: 0 / User-skipped: 1376 / Auto-skipped: 0

dynamic_domains_mysql_redis_28 / mysql_redis / 1387311
Reports root/ big
OK: 5196 / Failed: 0 / User-skipped: 157 / Auto-skipped: 0

internal_mnesia_28 / internal_mnesia / 1387311
Reports root/ big
OK: 2506 / Failed: 0 / User-skipped: 1228 / Auto-skipped: 0

pgsql_cets_28 / pgsql_cets / 1387311
Reports root/ big
OK: 5321 / Failed: 0 / User-skipped: 202 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_27 / pgsql_mnesia / 1387311
Reports root/ big
OK: 5231 / Failed: 0 / User-skipped: 122 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_28 / pgsql_mnesia / 1387311
Reports root/ big
OK: 5231 / Failed: 0 / User-skipped: 122 / Auto-skipped: 0

cockroachdb_cets_28 / cockroachdb_cets / 1387311
Reports root/ big
OK: 5335 / Failed: 7 / User-skipped: 202 / Auto-skipped: 0

pubsub_SUITE:dag+last_item_cache:send_last_published_item_no_items_test

{error,
  {timeout_when_waiting_for_stanza,
    [{escalus_client,wait_for_stanza,
       [{client,
          <<"alice_send_last_published_item_no_items_test_3600@localhost/res1">>,
          escalus_tcp,<0.115616.0>,
          [{event_manager,<0.115604.0>},
           {server,<<"localhost">>},
           {username,
             <<"alicE_send_last_published_item_no_items_test_3600">>},
           {resource,<<"res1">>}],
          [{event_client,
             [{event_manager,<0.115604.0>},
            {server,<<"localhost">>},
            {username,
              <<"alicE_send_last_published_item_no_items_test_3600">>},
            {resource,<<"res1">>}]},
           {resource,<<"res1">>},
           {username,
             <<"alice_send_last_published_item_no_items_test_3600">>},
           {server,<<"localhost">>},
           {host,<<"localhost">>},
           {port,5222},
           {auth,fun escalus_auth:auth_plain/2},
           {wspath,undefined},
           {username,
             <<"alicE_send_last_published_item_no_items_test_3600">>},
           {server,<<"localhost">>},
           {password,<<"matygrysa">>},
           {stream_id,<<"7a25a70c702d3358">>}]},
        5000],
       [{file,
          "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
        {line,136}]},
     {pubsub_tools,receive_response,3,
       [{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
        {line,444}]},
     {pubsub_tools,receive_and_c...

		-spec iq_command(binary(), any(), jid:jid(), jlib:iq(), any(), [binary()]) ->
		{result, [exml:element()]} \| {error, exml:element()}.

		TailLength = byte_size(P) - (First + 1),
		binary:part(S, First, byte_size(S) - TailLength - First + 1)

                                       % changes list applied to old list should produce the same result
-                                      {ok, NewAffUsers1, _, _, _} = mod_muc_light_utils:change_aff_users(host_type(), AffUsers, AffUsersChanged),
+                                      ChangeRes = mod_muc_light_utils:change_aff_users(host_type(), AffUsers,
+                                                                                        AffUsersChanged),

+                         {elvis_style, god_modules, disable},
+                         {elvis_style, no_throw, disable},
+                         {elvis_style, no_operation_on_same_value, disable},

		#{strategy => Strategy, rule => Rule},
		#{}),

Reenable Elvis checks in CI #4598

Are you sure you want to change the base?

Reenable Elvis checks in CI #4598

Conversation

tomaszwojcikowski commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment was marked as outdated.

chrzaszcz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

pawlooss1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mongoose-im commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrzaszcz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mongoose-im commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mongoose-im commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mongoose-im commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tomaszwojcikowski commented Dec 11, 2025 •

edited

Loading

codecov bot commented Dec 11, 2025 •

edited

Loading

mongoose-im commented Dec 19, 2025 •

edited

Loading

chrzaszcz left a comment •

edited

Loading

mongoose-im commented Dec 19, 2025 •

edited

Loading

mongoose-im commented Dec 19, 2025 •

edited

Loading

mongoose-im commented Dec 19, 2025 •

edited

Loading

mongoose-im commented Dec 19, 2025 •

edited

Loading

mongoose-im commented Dec 22, 2025 •

edited

Loading