From 422ef89163c0f4a66cf40001079c1b8e282e40a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Tue, 7 Oct 2025 11:20:28 +0200 Subject: [PATCH 1/8] CI: Enable Erlang problem matchers In order to get the right subdirectory for GH to find our files we use `sed` to modify paths in the output. We use `pipefail` to make sure failures are propagated. We use `NON_DETERMINISTIC` to make sure our paths are full and not just the file.erl. We disable styling in non-parallel CT runs. This is needed in order to pick up errors. Currently these errors will not be annotated directly in PRs, a pending OTP PR will improve that. Parallel CT runs write GH annotations directly. --- .github/workflows/test-make-target.yaml | 13 ++++++++++++- deps/rabbit_common/mk/rabbitmq-early-plugin.mk | 4 +++- .../rabbitmq_ct_helpers/src/ct_master_fork.erl | 18 ++++++++++++++++-- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test-make-target.yaml b/.github/workflows/test-make-target.yaml index d19905d6c140..aa5c1e59d9db 100644 --- a/.github/workflows/test-make-target.yaml +++ b/.github/workflows/test-make-target.yaml @@ -54,6 +54,16 @@ jobs: # restricted to the build jobs to avoid duplication in output. disable_problem_matchers: true + # We install Erlang problem matchers from ci.erlang.mk. + - name: CHECKOUT ERLANG PROBLEM MATCHERS + uses: actions/checkout@v4 + with: + repository: ninenines/ci.erlang.mk + path: ci.erlang.mk + + - name: INSTALL ERLANG PROBLEM MATCHERS + run: echo "::add-matcher::ci.erlang.mk/.github/matchers/erlang-matchers.json" + - name: MIXED CLUSTERS - FETCH SIGNING KEYS uses: dsaltares/fetch-gh-release-asset@master if: inputs.mixed_clusters @@ -120,7 +130,8 @@ jobs: if: inputs.plugin != 'rabbitmq_cli' run: | sudo netstat -ntp - make -C deps/${{ inputs.plugin }} ${{ inputs.make_target }} RABBITMQ_METADATA_STORE=${{ inputs.metadata_store }} + set -o pipefail + make -C deps/${{ inputs.plugin }} ${{ inputs.make_target }} RABBITMQ_METADATA_STORE=${{ inputs.metadata_store }} NON_DETERMINISTIC=1 | sed "s/src\//deps\/${{ inputs.plugin }}\/src\//" | sed "s/test\//deps\/${{ inputs.plugin }}\/test\//" - name: CACHE ACTIVEMQ uses: actions/cache/save@v4 diff --git a/deps/rabbit_common/mk/rabbitmq-early-plugin.mk b/deps/rabbit_common/mk/rabbitmq-early-plugin.mk index 65dcd621ba3f..e59da1e4b06a 100644 --- a/deps/rabbit_common/mk/rabbitmq-early-plugin.mk +++ b/deps/rabbit_common/mk/rabbitmq-early-plugin.mk @@ -36,9 +36,11 @@ CT_OPTS += -kernel net_ticktime 5 # # cth_styledout # This hook will change the output of common_test to something more -# concise and colored. +# concise and colored. Not used on GitHub Actions except in parallel CT. +ifndef GITHUB_ACTIONS CT_HOOKS += cth_styledout +endif TEST_DEPS += cth_styledout ifdef CONCOURSE diff --git a/deps/rabbitmq_ct_helpers/src/ct_master_fork.erl b/deps/rabbitmq_ct_helpers/src/ct_master_fork.erl index 50cfbac4c662..27be13619e22 100644 --- a/deps/rabbitmq_ct_helpers/src/ct_master_fork.erl +++ b/deps/rabbitmq_ct_helpers/src/ct_master_fork.erl @@ -723,14 +723,28 @@ master_print_summary_for(Title,List) -> _ = case List of [] -> ok; _ -> - Chars = [ + Chars = [[ + master_format_gh_anno(Reason), io_lib:format("Node: ~w~nCase: ~w:~w~nReason: ~p~n~n", [Node, Suite, FuncOrGroup, Reason]) - || {Node, Suite, FuncOrGroup, Reason} <- List], + ] || {Node, Suite, FuncOrGroup, Reason} <- List], log(all,Title,Chars,[]) end, ok. +master_format_gh_anno({error, {{exception, Reason, [{Mod, Fun, Arity, Loc}|_]}, _}}) -> + %% We use .github because that file exists in our repository + %% so GH will still put annotations in pull requests even + %% if we don't have the real file name. + File = proplists:get_value(file, Loc, ".github"), + Line = proplists:get_value(line, Loc, 0), + io_lib:format("::error file=~s,line=~b::~w:~tw/~b: ~w~n", + [File, Line, Mod, Fun, Arity, Reason]); +master_format_gh_anno(Reason) -> + %% Do the bare minimum if we don't know the error reason. + io_lib:format("::error file=.github,line=0::~w~n", + [Reason]). + update_queue(take,Node,From,Lock={Op,Resource},Locks,Blocked) -> %% Locks: [{{Operation,Resource},Node},...] %% Blocked: [{{Operation,Resource},Node,WaitingPid},...] From 30aa44e3da961593649125b4620d63cb9a2171ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Wed, 15 Oct 2025 14:45:57 +0200 Subject: [PATCH 2/8] Don't find queue crashes in logs in crashing_queues_SUITE --- deps/rabbit/test/crashing_queues_SUITE.erl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/deps/rabbit/test/crashing_queues_SUITE.erl b/deps/rabbit/test/crashing_queues_SUITE.erl index 054ec8f86631..f4c6ffa91d9a 100644 --- a/deps/rabbit/test/crashing_queues_SUITE.erl +++ b/deps/rabbit/test/crashing_queues_SUITE.erl @@ -39,7 +39,8 @@ end_per_suite(Config) -> init_per_group(cluster_size_2, Config) -> rabbit_ct_helpers:set_config(Config, [ - {rmq_nodes_count, 2} + {rmq_nodes_count, 2}, + {find_crashes, false} %% we crash queues on purpose ]). end_per_group(_, Config) -> From 78af8e92088b1e0dcf0ab69eb9cbfdaa1923fb2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Wed, 15 Oct 2025 15:18:10 +0200 Subject: [PATCH 3/8] Don't find crashes in logs in cluster_SUITE There's one test that kills the queue on purpose. --- deps/rabbit/test/cluster_SUITE.erl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/deps/rabbit/test/cluster_SUITE.erl b/deps/rabbit/test/cluster_SUITE.erl index b47c8269ebe2..f74ab9d63ff3 100644 --- a/deps/rabbit/test/cluster_SUITE.erl +++ b/deps/rabbit/test/cluster_SUITE.erl @@ -63,7 +63,8 @@ init_per_group(Group, Config) -> true -> Config1 = rabbit_ct_helpers:set_config(Config, [ {rmq_nodename_suffix, Group}, - {rmq_nodes_count, 2} + {rmq_nodes_count, 2}, + {find_crashes, false} %% we crash some queues on purpose ]), rabbit_ct_helpers:run_steps(Config1, rabbit_ct_broker_helpers:setup_steps() ++ From d7e4881eb9e6c704128205b8320aa2cb0c9e5657 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Thu, 16 Oct 2025 10:41:49 +0200 Subject: [PATCH 4/8] Drop dependency on xmerl where it isn't needed UTF-8 validation that used xmerl_ucs:from_bin/1 now uses unicode:characters_to_binary/3. --- deps/amqp_client/Makefile | 2 +- deps/amqp_client/src/amqp_connection.erl | 2 +- deps/rabbit/Makefile | 2 +- deps/rabbit_common/Makefile | 4 ++-- deps/rabbit_common/src/rabbit_binary_parser.erl | 10 ++++++---- deps/rabbit_common/src/rabbit_misc.erl | 11 +++++++---- deps/rabbitmq_cli/test/test_helper.exs | 1 - deps/rabbitmq_management_agent/Makefile | 2 +- 8 files changed, 19 insertions(+), 15 deletions(-) diff --git a/deps/amqp_client/Makefile b/deps/amqp_client/Makefile index 654a62d905ad..8d811e4dbccb 100644 --- a/deps/amqp_client/Makefile +++ b/deps/amqp_client/Makefile @@ -39,7 +39,7 @@ endef # Release artifacts are put in $(PACKAGES_DIR). PACKAGES_DIR ?= $(abspath PACKAGES) -LOCAL_DEPS = xmerl ssl public_key +LOCAL_DEPS = ssl public_key DEPS = rabbit_common credentials_obfuscation TEST_DEPS = rabbitmq_ct_helpers rabbit meck diff --git a/deps/amqp_client/src/amqp_connection.erl b/deps/amqp_client/src/amqp_connection.erl index c5f4223ce113..8851e59a8b96 100644 --- a/deps/amqp_client/src/amqp_connection.erl +++ b/deps/amqp_client/src/amqp_connection.erl @@ -203,7 +203,7 @@ set_connection_name(ConnName, %% application controller is in the process of shutting down the very %% application which is making this call. ensure_started() -> - [ensure_started(App) || App <- [syntax_tools, compiler, xmerl, + [ensure_started(App) || App <- [syntax_tools, compiler, rabbit_common, amqp_client, credentials_obfuscation]], ok. diff --git a/deps/rabbit/Makefile b/deps/rabbit/Makefile index 89048261923c..076296979e65 100644 --- a/deps/rabbit/Makefile +++ b/deps/rabbit/Makefile @@ -126,7 +126,7 @@ define PROJECT_ENV ] endef -LOCAL_DEPS = sasl os_mon inets compiler public_key crypto ssl syntax_tools xmerl +LOCAL_DEPS = sasl os_mon inets compiler public_key crypto ssl syntax_tools BUILD_DEPS = rabbitmq_cli DEPS = ranch cowlib rabbit_common amqp10_common rabbitmq_prelaunch ra sysmon_handler stdout_formatter recon redbug observer_cli osiris syslog systemd seshat horus khepri khepri_mnesia_migration cuttlefish gen_batch_server diff --git a/deps/rabbit_common/Makefile b/deps/rabbit_common/Makefile index 87be767cd70d..f74485549695 100644 --- a/deps/rabbit_common/Makefile +++ b/deps/rabbit_common/Makefile @@ -25,7 +25,7 @@ define HEX_TARBALL_EXTRA_METADATA } endef -LOCAL_DEPS = compiler crypto public_key sasl ssl syntax_tools tools xmerl runtime_tools +LOCAL_DEPS = compiler crypto public_key sasl ssl syntax_tools tools runtime_tools DEPS = thoas ranch recon credentials_obfuscation # Variables and recipes in development.*.mk are meant to be used from @@ -43,7 +43,7 @@ DEP_EARLY_PLUGINS = $(PROJECT)/mk/rabbitmq-early-plugin.mk DEP_PLUGINS = $(PROJECT)/mk/rabbitmq-build.mk \ $(PROJECT)/mk/rabbitmq-hexpm.mk -PLT_APPS += mnesia crypto ssl xmerl +PLT_APPS += mnesia crypto ssl include ../../rabbitmq-components.mk include ../../erlang.mk diff --git a/deps/rabbit_common/src/rabbit_binary_parser.erl b/deps/rabbit_common/src/rabbit_binary_parser.erl index 9cc337235dbe..a6df3700f436 100644 --- a/deps/rabbit_common/src/rabbit_binary_parser.erl +++ b/deps/rabbit_common/src/rabbit_binary_parser.erl @@ -164,9 +164,11 @@ assert_utf8(B) -> end. validate_utf8(Bin) -> - try - _ = xmerl_ucs:from_utf8(Bin), - ok - catch exit:{ucs, _} -> + case unicode:characters_to_binary(Bin, unicode, unicode) of + B when is_binary(B) -> + ok; + {error, _, _} -> + error; + {incomplete, _, _} -> error end. diff --git a/deps/rabbit_common/src/rabbit_misc.erl b/deps/rabbit_common/src/rabbit_misc.erl index 998e1c9c402f..f64d71d6c5c1 100644 --- a/deps/rabbit_common/src/rabbit_misc.erl +++ b/deps/rabbit_common/src/rabbit_misc.erl @@ -505,10 +505,13 @@ b64decode_or_throw(B64) -> end. utf8_safe(V) -> - try - _ = xmerl_ucs:from_utf8(V), - V - catch exit:{ucs, _} -> + case unicode:characters_to_binary(V, unicode, unicode) of + B when is_binary(B) -> + B; + {error, _, _} -> + Enc = split_lines(base64:encode(V)), + <<"Not UTF-8, base64 is: ", Enc/binary>>; + {incomplete, _, _} -> Enc = split_lines(base64:encode(V)), <<"Not UTF-8, base64 is: ", Enc/binary>> end. diff --git a/deps/rabbitmq_cli/test/test_helper.exs b/deps/rabbitmq_cli/test/test_helper.exs index d7f218715530..2ad0faa8c305 100644 --- a/deps/rabbitmq_cli/test/test_helper.exs +++ b/deps/rabbitmq_cli/test/test_helper.exs @@ -35,7 +35,6 @@ if function_exported?(Mix, :ensure_application!, 1) do Mix.ensure_application!(:public_key) Mix.ensure_application!(:runtime_tools) Mix.ensure_application!(:sasl) - Mix.ensure_application!(:xmerl) end defmodule TestHelper do diff --git a/deps/rabbitmq_management_agent/Makefile b/deps/rabbitmq_management_agent/Makefile index a1a3b064b832..ead74ac45f55 100644 --- a/deps/rabbitmq_management_agent/Makefile +++ b/deps/rabbitmq_management_agent/Makefile @@ -19,7 +19,7 @@ endef DEPS = rabbit_common rabbit rabbitmq_web_dispatch TEST_DEPS = rabbitmq_ct_helpers rabbitmq_ct_client_helpers -LOCAL_DEPS += xmerl ranch ssl crypto public_key +LOCAL_DEPS += ranch ssl crypto public_key PLT_APPS += rabbitmq_cli From d08dec145c9d827ef8771e7cacd51ff09d9b85b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Thu, 16 Oct 2025 11:24:34 +0200 Subject: [PATCH 5/8] Fix unused variable warnings --- deps/oauth2_client/test/system_SUITE.erl | 4 ++-- .../test/rabbit_oauth2_provider_SUITE.erl | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/deps/oauth2_client/test/system_SUITE.erl b/deps/oauth2_client/test/system_SUITE.erl index 8153fd73d895..8dddf763f7c4 100644 --- a/deps/oauth2_client/test/system_SUITE.erl +++ b/deps/oauth2_client/test/system_SUITE.erl @@ -598,14 +598,14 @@ get_oauth_provider_given_oauth_provider_id(Config) -> Jwks_uri) end. -jwks_url_is_used_in_absense_of_jwks_uri(Config) -> +jwks_url_is_used_in_absense_of_jwks_uri(_Config) -> {ok, #oauth_provider{ jwks_uri = Jwks_uri}} = oauth2_client:get_oauth_provider([jwks_uri]), ?assertEqual( proplists:get_value(jwks_url, get_env(key_config, []), undefined), Jwks_uri). -jwks_uri_takes_precedence_over_jwks_url(Config) -> +jwks_uri_takes_precedence_over_jwks_url(_Config) -> {ok, #oauth_provider{ jwks_uri = Jwks_uri}} = oauth2_client:get_oauth_provider([jwks_uri]), ?assertEqual(get_env(jwks_uri), Jwks_uri). diff --git a/deps/rabbitmq_auth_backend_oauth2/test/rabbit_oauth2_provider_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/rabbit_oauth2_provider_SUITE.erl index 7e81cda4f773..5bd0bc59e9b6 100644 --- a/deps/rabbitmq_auth_backend_oauth2/test/rabbit_oauth2_provider_SUITE.erl +++ b/deps/rabbitmq_auth_backend_oauth2/test/rabbit_oauth2_provider_SUITE.erl @@ -479,7 +479,7 @@ start_https_oauth_server(Port, CertsDir, Expectations) when is_list(Expectations {'_', [{Path, oauth2_http_mock, Expected} || #{request := #{path := Path}} = Expected <- Expectations ]} ]), - {ok, Pid} = cowboy:start_tls( + {ok, _Pid} = cowboy:start_tls( mock_http_auth_listener, [{port, Port}, {certfile, filename:join([CertsDir, "server", "cert.pem"])}, From 7803ba494de6d07bc8ed4000107007770f75c415 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Thu, 16 Oct 2025 12:08:29 +0200 Subject: [PATCH 6/8] Remove a clause that never matches in tests Identical clause was found in the same function. --- deps/rabbitmq_management/test/rabbit_mgmt_wm_auth_SUITE.erl | 3 --- 1 file changed, 3 deletions(-) diff --git a/deps/rabbitmq_management/test/rabbit_mgmt_wm_auth_SUITE.erl b/deps/rabbitmq_management/test/rabbit_mgmt_wm_auth_SUITE.erl index eff751803315..4b792c33726d 100644 --- a/deps/rabbitmq_management/test/rabbit_mgmt_wm_auth_SUITE.erl +++ b/deps/rabbitmq_management/test/rabbit_mgmt_wm_auth_SUITE.erl @@ -538,9 +538,6 @@ end_per_group(with_oauth_disable_basic_auth_false, Config) -> end_per_group(with_resource_server_id_rabbit, Config) -> unset_env(rabbitmq_auth_backend_oauth2, resource_server_id), Config; -end_per_group(with_default_oauth_provider_idp1, Config) -> - unset_env(rabbitmq_auth_backend_oauth2, default_oauth_provider), - Config; end_per_group(with_mgt_oauth_provider_url_url0, Config) -> unset_env(rabbitmq_management, oauth_provider_url), Config; From 8d34f3e4ec4e41aa10d93347278ed9be97ca322b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Thu, 16 Oct 2025 12:31:37 +0200 Subject: [PATCH 7/8] Temporarily ignore the enough.erl warning --- .github/workflows/test-make-target.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-make-target.yaml b/.github/workflows/test-make-target.yaml index aa5c1e59d9db..6bc06903fd48 100644 --- a/.github/workflows/test-make-target.yaml +++ b/.github/workflows/test-make-target.yaml @@ -131,7 +131,7 @@ jobs: run: | sudo netstat -ntp set -o pipefail - make -C deps/${{ inputs.plugin }} ${{ inputs.make_target }} RABBITMQ_METADATA_STORE=${{ inputs.metadata_store }} NON_DETERMINISTIC=1 | sed "s/src\//deps\/${{ inputs.plugin }}\/src\//" | sed "s/test\//deps\/${{ inputs.plugin }}\/test\//" + make -C deps/${{ inputs.plugin }} ${{ inputs.make_target }} RABBITMQ_METADATA_STORE=${{ inputs.metadata_store }} NON_DETERMINISTIC=1 2>&1 | sed "s/src\//deps\/${{ inputs.plugin }}\/src\//" | sed "s/test\//deps\/${{ inputs.plugin }}\/test\//" | sed "s/src\/enough\.erl:22:2:/IGNORED src\/enough.erl:22:2:/" - name: CACHE ACTIVEMQ uses: actions/cache/save@v4 From f05b18daf66937fa84084af3cdd23472f5b27264 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Thu, 16 Oct 2025 17:08:51 +0200 Subject: [PATCH 8/8] Remove a test that wasn't working Changing the path to a non-existent /ws2 still succeeded. Other tests properly check connecting. --- deps/rabbitmq_web_stomp/test/cowboy_websocket_SUITE.erl | 9 --------- 1 file changed, 9 deletions(-) diff --git a/deps/rabbitmq_web_stomp/test/cowboy_websocket_SUITE.erl b/deps/rabbitmq_web_stomp/test/cowboy_websocket_SUITE.erl index 1dd08b38fe60..be0db0606f44 100644 --- a/deps/rabbitmq_web_stomp/test/cowboy_websocket_SUITE.erl +++ b/deps/rabbitmq_web_stomp/test/cowboy_websocket_SUITE.erl @@ -21,7 +21,6 @@ groups() -> [ {integration, [], [ - connection_succeeds, connection_fails, pubsub, pubsub_binary, @@ -91,14 +90,6 @@ end_per_testcase(http_auth, Config) -> end_per_testcase(_, Config) -> Config. -connection_succeeds(Config) -> - PortStr = rabbit_ws_test_util:get_web_stomp_port_str(Config), - Protocol = ?config(protocol, Config), - WS = rfc6455_client:new(Protocol ++ "://127.0.0.1:" ++ PortStr ++ "/ws", self()), - {ok, _} = rfc6455_client:open(WS), - {close, _} = rfc6455_client:close(WS), - ok. - connection_fails(Config) -> PortStr = rabbit_ws_test_util:get_web_stomp_port_str(Config), Protocol = ?config(protocol, Config),