Make CETS joining logic more reliable

[arcusfelis]

This issue addresses MIM-1968.

The changes include:

• Tests for cets_join
• Step function to write tests for joining procedure
• Fixes for the logic - we do more validation before applying the changes
• We set join_ref when we join - all nodes should have the same join_ref after joining.
• We actually check join_ref before sending messages, check check_server in cets module.
• We don't catch errors for long calls anymore - it confuses result types. It also leads to nested {error, {error, _}_} when there is long call inside of a long operation
• There is an issue in cover code reports, so we have to make a map outside of LOG_ERROR call and pass it into it. I don't think there would be any performance issues (unless someone disables all log messages), but still we could fix it once OTP fixes cover tool. For now I prefer good cover reports - they actually allow to find errors in logic Coverage is broken for multiline logging call erlang/otp#7531

[codecov]

Codecov Report

Patch coverage: 97.22% and project coverage change: +4.40% 🎉

Comparison is base (b4fefc9) 86.29% compared to head (2cbb2d4) 90.69%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #12      +/-   ##
==========================================
+ Coverage   86.29%   90.69%   +4.40%     
==========================================
  Files           8        8              
  Lines         372      430      +58     
==========================================
+ Hits          321      390      +69     
+ Misses         51       40      -11     

Files Changed	Coverage Δ
src/cets_discovery_file.erl	62.50% <0.00%> (ø)
src/cets_call.erl	80.00% <75.00%> (+4.00%)	⬆️
src/cets.erl	99.41% <100.00%> (+3.25%)	⬆️
src/cets_join.erl	97.93% <100.00%> (+5.21%)	⬆️
src/cets_long.erl	84.37% <100.00%> (+11.04%)	⬆️

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

[chrzaszcz]

The code itself is OK for me 👍
I only have an issue with the actual concept of steps, which are checkpoints between steps. I think a cleaner option would be to use a fold operation over a list of steps.

For example, see:

• https://github.com/esl/MongooseIM/blob/master/src/system_metrics/mongoose_system_metrics_collector.erl#L46
• https://github.com/esl/MongooseIM/blob/master/src/config/mongoose_config_parser_toml.erl#L142
• https://github.com/esl/MongooseIM/blob/master/src/graphql/mongoose_graphql_commands.erl#L70

I also consider the new syntax for logging errors a bit of verbose, and now we contruct the terms even if logs are disabled for the given level, which kind of defeats the purpose of the macro...

[chrzaszcz]

For me this is counter-intuitive, because this function isn't actually running the step, but runs some custom injected logic between steps. It's a lot of bending the code to fit the tests, and gives the impression of being overly complicated...

[chrzaszcz]

For me it would be cleaner if there was a lists:foldl over a list of functions performing the actual steps, and in tests you could use meck to inject some logic when needed - by changing the actual step function.

[chrzaszcz]

Looks cleaner with the checkpoints