From 623a89f946a51cd61f8fdbe62721a3ecbf7fa180 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Wed, 21 Feb 2024 22:01:43 +0100 Subject: [PATCH] Apply review to tests and users sup init --- src/users/amoc_users_sup.erl | 13 +++++----- src/users/amoc_users_worker_sup.erl | 3 ++- test/controller_SUITE.erl | 40 ++++++++++++++--------------- 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/src/users/amoc_users_sup.erl b/src/users/amoc_users_sup.erl index 078ef954..16be9c5b 100644 --- a/src/users/amoc_users_sup.erl +++ b/src/users/amoc_users_sup.erl @@ -40,12 +40,12 @@ -spec start_link() -> supervisor:startlink_ret(). start_link() -> Ret = supervisor:start_link({local, ?MODULE}, ?MODULE, no_args), - UserSups = lists:reverse(supervisor:which_children(?MODULE)), - UserSupPids = [ Pid || {_, Pid, _, _} <- UserSups ], - UserSupPidsTuple = list_to_tuple(UserSupPids), + UserSups = supervisor:which_children(?MODULE), + IndexedSupsUnsorted = [ {Pid, N} || {{amoc_users_worker_sup, N}, Pid, _, _} <- UserSups], + IndexedSups = lists:keysort(2, IndexedSupsUnsorted), + UserSupPidsTuple = list_to_tuple([ Pid || {Pid, _} <- IndexedSups ]), SupCount = tuple_size(UserSupPidsTuple), Atomics = atomics:new(1 + SupCount, [{signed, false}]), - IndexedSups = lists:zip(UserSupPids, indexes()), Storage = #storage{user_count = Atomics, sups = UserSupPidsTuple, sups_indexed = IndexedSups, sups_count = SupCount}, persistent_term:put(?MODULE, Storage), @@ -164,9 +164,10 @@ assign_counts(Total) -> -spec distribute(count(), assignment()) -> {count(), assignment()}. distribute(Total, SupervisorsWithCounts) -> - Data = maps:from_list(SupervisorsWithCounts), + SupervisorWithPositiveCounts = [ T || T = {_, Count} <- SupervisorsWithCounts, Count =/= 0], + Data = maps:from_list(SupervisorWithPositiveCounts), N = remove_n(Total, Data), - distribute(#{}, Data, SupervisorsWithCounts, Total, N). + distribute(#{}, Data, SupervisorWithPositiveCounts, Total, N). -spec remove_n(count(), map()) -> non_neg_integer(). remove_n(Total, Data) when map_size(Data) > 0 -> diff --git a/src/users/amoc_users_worker_sup.erl b/src/users/amoc_users_worker_sup.erl index 16509c76..fd1c6dcb 100644 --- a/src/users/amoc_users_worker_sup.erl +++ b/src/users/amoc_users_worker_sup.erl @@ -67,7 +67,8 @@ get_all_children(Sup) -> -spec init(non_neg_integer()) -> {ok, term()}. init(N) -> process_flag(trap_exit, true), - Tid = ets:new(?MODULE, [ordered_set, protected]), + Name = list_to_atom(atom_to_list(?MODULE) ++ "_" ++ integer_to_list(N)), + Tid = ets:new(Name, [ordered_set, protected, named_table]), {ok, #state{index = N, tid = Tid}}. %% @private diff --git a/test/controller_SUITE.erl b/test/controller_SUITE.erl index 5e1a23da..18a3b54c 100644 --- a/test/controller_SUITE.erl +++ b/test/controller_SUITE.erl @@ -18,10 +18,10 @@ groups() -> distribute() -> [ - each_assignment_is_less_than_or_equal_in_the_input, - total_count_is_always_less_than_or_equal_the_requested_amount, + each_assignment_is_less_than_or_equal_than_the_request, total_count_is_equal_to_the_requested_amount_or_the_sum_of_values_in_the_input, - total_count_is_the_exact_sum_of_the_values_in_the_input + total_count_is_the_exact_sum_of_the_values_in_the_output, + distribution_is_done_among_maximum_number_of_workers ]. all_tests() -> @@ -70,43 +70,43 @@ end_per_testcase(_TestCase, Config) -> %% test cases -each_assignment_is_less_than_or_equal_in_the_input(_) -> - Prop = ?FORALL({Total, Counts}, {pos_integer(), sups_with_counts()}, +each_assignment_is_less_than_or_equal_than_the_request(_) -> + Prop = ?FORALL({Request, Counts}, {non_neg_integer(), sups_with_counts()}, begin SupervisorWithCounts = [ {dummy_pid(), Count} || Count <- Counts ], - {_, Assignments} = amoc_users_sup:distribute(Total, SupervisorWithCounts), + {_, Assignments} = amoc_users_sup:distribute(Request, SupervisorWithCounts), Data = maps:from_list(SupervisorWithCounts), lists:all(fun({Pid, N}) -> N =< maps:get(Pid, Data) end, Assignments) end), run_prop(?FUNCTION_NAME, Prop, 100, 1). -total_count_is_always_less_than_or_equal_the_requested_amount(_) -> - Prop = ?FORALL({Total, Counts}, {pos_integer(), sups_with_counts()}, +total_count_is_equal_to_the_requested_amount_or_the_sum_of_values_in_the_input(_) -> + Prop = ?FORALL({Request, Counts}, {non_neg_integer(), sups_with_counts()}, begin SupervisorWithCounts = [ {dummy_pid(), Count} || Count <- Counts ], - {Count, _} = amoc_users_sup:distribute(Total, SupervisorWithCounts), - Count =< Total + {Result, _} = amoc_users_sup:distribute(Request, SupervisorWithCounts), + Available = lists:sum([ N || {_, N} <- SupervisorWithCounts ]), + Result =:= min(Request, Available) end), run_prop(?FUNCTION_NAME, Prop, 100, 1). -total_count_is_equal_to_the_requested_amount_or_the_sum_of_values_in_the_input(_) -> - Prop = ?FORALL({Total, Counts}, {pos_integer(), sups_with_counts()}, +total_count_is_the_exact_sum_of_the_values_in_the_output(_) -> + Prop = ?FORALL({Request, Counts}, {non_neg_integer(), sups_with_counts()}, begin SupervisorWithCounts = [ {dummy_pid(), Count} || Count <- Counts ], - {Count, Assignments} = amoc_users_sup:distribute(Total, SupervisorWithCounts), + {Result, Assignments} = amoc_users_sup:distribute(Request, SupervisorWithCounts), Assinged = lists:sum([ N || {_, N} <- Assignments ]), - Count =:= Total orelse Count =:= Assinged + Result =:= Assinged end), run_prop(?FUNCTION_NAME, Prop, 100, 1). -% • The total count of the users is the exact sum of the values in the KV list -total_count_is_the_exact_sum_of_the_values_in_the_input(_) -> - Prop = ?FORALL({Total, Counts}, {pos_integer(), sups_with_counts()}, +distribution_is_done_among_maximum_number_of_workers(_) -> + Prop = ?FORALL({Request, Counts}, {pos_integer(), sups_with_counts()}, begin SupervisorWithCounts = [ {dummy_pid(), Count} || Count <- Counts ], - {Count, Assignments} = amoc_users_sup:distribute(Total, SupervisorWithCounts), - Assinged = lists:sum([ N || {_, N} <- Assignments ]), - Count =:= Assinged + {_, Assignments} = amoc_users_sup:distribute(Request, SupervisorWithCounts), + NonZeroInInput = length([ Count || Count <- Counts, Count =/= 0 ]), + length(Assignments) =:= min(Request, NonZeroInInput) end), run_prop(?FUNCTION_NAME, Prop, 100, 1).