Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compile and test MongooseIM with Erlang/OTP 20 #1430

Merged
merged 2 commits into from
Oct 3, 2017
Merged

Compile and test MongooseIM with Erlang/OTP 20 #1430

merged 2 commits into from
Oct 3, 2017

Conversation

michalwski
Copy link
Contributor

@michalwski michalwski commented Aug 8, 2017

With this PR it's possible to compile and test MongooseIM with Erlang/OTP from 18 to 20

Proposed changes include:

  • updated dependencies to be compatible with Erlang/OTP from 18 to 20
    • fix to backwards incompatible changes in worker_pool
  • update test to be able to run with Erlang/OTP 20:
    • updated escalus as in esl/escalus/150
    • updated meck to 0.8.6
    • change remaining crypt:rand_bytes to strong_rand_bytes

TODO:

  • fix (or even remove) SASL-DIGEST authentication (there is one test case failing in login_SUITE)
  • fix dialyzer errors
  • configure travis jobs so that most of them run with Erlang/OTP 19.3, dialyzer and one other job runs with Erlang/OTP 20 and another job for Erlang/OTP 18.3
  • remove possible hacks for Erlang/OTP 17.x
  • update documentation so that it's clear MongooseIM will work with Erlang/OTP form 18 to 20
  • try testing MongooseIM with FIPS mode enabled
  • update documentation regarding FIPS mode (doc/developers-guide/OpenSSL-and-FIPS.md)
  • investigate decreased coverage

{error, Reason} ->
String = io_lib:format("Can't restore backup from ~p at node ~p: ~p",
[filename:absname(Path), node(), Reason]),
{cannot_restore, String};
{aborted, {no_exists, Table}} ->
String = io_lib:format("Can't restore backup from ~p at node ~p: Table ~p does not exist.",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to Elvis:

Line 498 is too long: String = io_lib:format("Can't restore backup from ~p at node ~p: Table ~p does not exist.",.

```

### Building MongooseIM with a custom OpenSSL

Before running `./rebar3 compile` or `make rel` please export the CFLAGS and LDFLAGS env vars pointing to a FIPS compliant OpenSSL, f.e.
If you want to use a different OpenSSL than the default one, before running `./rebar3 compile` or `make rel` please export the CFLAGS and LDFLAGS env vars pointing to a FIPS compliant OpenSSL, f.e.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to use a custom OpenSSL, please export the CFLAGS and LDFLAGS env vars pointing to a FIPS compliant OpenSSL before running ./rebar3 compile or make rel.
F.e:

@fenek fenek added this to the MongooseIM 2.1.0 milestone Sep 25, 2017
@DenysGonchar
Copy link
Collaborator

DenysGonchar commented Sep 29, 2017

coverage decrease analysis

analysed coverage decrease between base master commit and latest otp-20 commit

file_name cov1* cov2* d_cov* d_l1* d_l2*
mongoose_fips.erl 75.0 41.176 -33.823 9 13
ELDAPv3.erl 22.712 14.708 -8.004 690 690
mongoose_client_api_sse.erl 81.818 75.757 -6.060 2 0
eldap_filter_yecc.erl 20.232 14.262 -5.970 180 180
mod_aws_sns.erl 91.860 87.209 -4.651 4 0
mod_mam_odbc_prefs.erl 98.901 95.604 -3.296 3 0
mod_mam_utils.erl 88.086 85.920 -2.166 6 0
mod_muc.erl 68.508 67.127 -1.381 5 0
mod_mam.erl 89.802 89.473 -0.328 1 0
mod_muc_log.erl 77.948 77.692 -0.256 1 0
node_flat.erl 74.006 73.993 -0.012 -1 -4
  • cov1 - coverage for base master commit
    cov2 - coverage for the last otp-20 commit
    d_cov - change of the coverage
    d_l1 - change of covered lines No
    d_l2 - change of lines No in the file

@michalwski
Copy link
Contributor Author

@DenysGonchar this is impressive what you've prepared! I'm wondering why there are these differences in coverage reports. Most of the modules didn't change in this PR... Maybe we can live with the way coverage is reported now?

@fenek
Copy link
Member

fenek commented Sep 29, 2017

@michalwski

ELDAPv3 and eldap_filter_yecc are generated by OTP so it seems they have changed the algorithm to create more verbose files and we don't use these new parts. mongoose_fips has a ifndef flag that depends on FIPS support and it seems we don't have any specific tests for it.

@DenysGonchar

What are these lines that are suddenly not covered in the remaining files?

@michalwski
Copy link
Contributor Author

Yes, coverage change in mongoose_fips is expected. Regarding the LDAP-related modules, will we still need them when the old PR #1216 is merged?

@DenysGonchar
Copy link
Collaborator

DenysGonchar commented Sep 29, 2017 via email

@DenysGonchar
Copy link
Collaborator

DenysGonchar commented Sep 29, 2017

some comments on the changed files:

ELDAPv3.erl and eldap_filter_yecc.erl are generated and there's no sense to analyse them, also we might want to exclude that modules from coverage analysis at all.

mongoose_fips.erl - as mentioned above, otp20 has support of fips_mode so rebar adds the corresponding compilation flag, from here we have new lines. so far we don't have tests for fips_mode so decrease of coverage is expected.

mongoose_client_api_sse.erl - uncovered lines are:

  • dummy handle_info/2 clause
  • and handle_error/3 function

mod_aws_sns.erl - uncovered lines are:

  • catch block in try_publish/5 function. more over for base commit catch block is not fully covered, it means that calc_backoff_time/2 function fails & generates exception. we should take a closer look on this!!!
  • calc_backoff_time/2 function (seems that exception is generated at line 294)

mod_mam_odbc_prefs - uncovered lines are:

  • catch block in run_transaction_or_retry_on_deadlock/3 function

mod_mam_utils.elr - uncovered lines are:

  • is_mam_result_message/1 function and 2 more internal functions called from this one (maybe_get_result_namespace/1 and is_mam_namespace/1)
  • error clause of error_on_sql_error/3 function

mod_muc.erl - uncovered lines are:

  • processing function passed to lists:foreach/2 inside load_permanent_rooms/6, RoomsToLoad is an empty list
  • default case clause in iq_disco_items/4 (line 805, clause just returns false)

mod_mam.erl - uncovered lines are:

  • sm_filter_offline_message/4

mod_muc_log.erl - difference in coverage is weekdays clauses in get_dateweek/2 function, this is explained by the simple fact that tests were running in different weekdays. and for base commit
it seems that one of the travis jobs failed and was restarted on the next day :)

node_flat.erl - difference in coverage caused by removed code in init/1, removed code is case block that has 2 clauses, one returns - ok, another one wasn't covered in base commit. so nothing to worry about.

none of these looks critical to me

PS: regarding the diff table in the comment above - original script to extract the data was made by @fenek (thank you, it saved a lot of time), i have just modified it a bit to generate markdown table. some time later i will create executable escript version of it, so it could be easily used by everyone who needs to analyse the coverage diffs

@michalwski
Copy link
Contributor Author

Thanks @DenysGonchar the analysation you did is impressive. Looks like the decrease in coverage will not be harmful for us. Merging this PR now has bigger value then fighting with the coverage. The sad thing is that we'll have less 70% coverage again. There is a quick fix to this situation - merging #1216 :)

@DenysGonchar
Copy link
Collaborator

i'm not 100% sure that it brings us back to 70+% but we can try exclude those generated files from coverage analysis.

update deps to compile with Erlang/OTP 20

test all on travis with Erlang/OTP 20

update big tests to compile and run with Erlang/OTP 20

use wpool_sup to start and stop a pool in a consistent way

use meck 0.8.6 to run small tests properly on Erlang/OTP 20

change crypto:rand_bytes to crypto:strong_rand_bytes

use fips related function according to Erlang/OTP doc

mnesia:delete_object always returns ok or throws exits signal

matching on anything else than 'ok' doesn't make sense

remove workarounds for Erlang older than 18 in tests

Use Erlang/OTP 19.3 for most of the jobs

One job with Erlang/OTP 18.3 and 20.0
Dialyzer job on Erlang/OTP 20.0

fix dialyzer warnings in mod_roster_mnesia

fix dialyzer warning for ejabberd_admin module

remove broken and not used mnesia update table logic

fix dialyzer warning in ejabberd_app module

run riak_mnesia preset with Erlang/OTP 18.3 instead of internal_mnesia

we can do this trick because riak driver is 3rd party library and
doesn't come with Erlang/OTP

this changes gives additional benefits:
* whole build is shorter by one job which is 30 minutes
* converage  on covaralls is not wrongly decreased because of the same result
  (as from other interl_mnesia job)

spefiy min OTP version in rebar.config

make elvis happy with line length

documentation update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants