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

Safer CT hook error handling #3133

Merged
merged 1 commit into from
May 27, 2021
Merged

Safer CT hook error handling #3133

merged 1 commit into from
May 27, 2021

Conversation

arcusfelis
Copy link
Contributor

When I was testing suites, I was generating errors from all/0 and groups/0 callbacks. And CT works really strange and not informative.

Two problems:

  • it does not print the error, just "location is unknown"
  • it passes that error to CT hooks, and that's why this PR.

Before CTH was crashing when handling SUITE crash during the init:

*** CT 2021-05-24 12:32:43.391 *** Suite Hook
Call to CTH failed: error:{{test_case_failed,
                               {open_file_failed,'test@Mikhails-MacBook-Pro',
                                   "/Users/mikhailuvarov/erlang/esl/MongooseIM/big_tests/ct_report/ct_run.test@Mikhails-MacBook-Pro.2021-05-24_12.32.42/undefined/../../../mongooseim@localhost.log.html",
                                   {error,enoent}}},
                           [{ct_mongoose_log_hook,ensure_initialized,2,
                                [{file,
                                     "/Users/mikhailuvarov/erlang/esl/MongooseIM/big_tests/src/ct_mongoose_log_hook.erl"},
                                 {line,176}]},
                            {ct_mongoose_log_hook,pre_init_per_testcase,3,
                                [{file,
                                     "/Users/mikhailuvarov/erlang/esl/MongooseIM/big_tests/src/ct_mongoose_log_hook.erl"},
                                 {line,81}]},
                            {ct_hooks,catch_apply,3,
                                [{file,"ct_hooks.erl"},{line,491}]},
                            {ct_hooks,do_call_generic,4,
                                [{file,"ct_hooks.erl"},{line,247}]},
                            {ct_hooks,call,4,
                                [{file,"ct_hooks.erl"},{line,301}]},
                            {ct_hooks,call,4,
                                [{file,"ct_hooks.erl"},{line,304}]},
                            {ct_hooks,call,3,
                                [{file,"ct_hooks.erl"},{line,257}]}]}

*** CT 2021-05-24 12:32:43.391 *** Suite Hook
Call to CTH failed: error:{function_clause,
                           [{proplists,get_value,
                             [priv_dir,
                              {fail,
                               "ct_mongoose_log_hook:pre_init_per_testcase/3 CTH call failed"},
                              undefined],
                             [{file,"proplists.erl"},{line,215}]},
                            {ct_mongoose_log_hook,keep_priv_dir,2,
                             [{file,
                               "/Users/mikhailuvarov/erlang/esl/MongooseIM/big_tests/src/ct_mongoose_log_hook.erl"},
                              {line,193}]},
                            {ct_mongoose_log_hook,pre_init_per_testcase,3,
                             [{file,
                               "/Users/mikhailuvarov/erlang/esl/MongooseIM/big_tests/src/ct_mongoose_log_hook.erl"},
                              {line,80}]},
                            {ct_hooks,catch_apply,3,
                             [{file,"ct_hooks.erl"},{line,491}]},
                            {ct_hooks,do_call_generic,4,
                             [{file,"ct_hooks.erl"},{line,247}]},
                            {ct_hooks,call,4,
                             [{file,"ct_hooks.erl"},{line,301}]},
                            {ct_hooks,call,4,
                             [{file,"ct_hooks.erl"},{line,304}]},
                            {ct_hooks,call,3,
                             [{file,"ct_hooks.erl"},{line,257}]}]}

*** CT Error Notification 2021-05-24 12:32:43.391 ***
Error detected: 'mam_SUITE:groups/0 failed'

Full error description and stacktrace

=== Ended at 2021-05-24 12:32:43
=== Location: unknown
=== === Reason: 'mam_SUITE:groups/0 failed'

After it is not crashing in the handler. Theoretically, it allows CT to fail with the origin SUITE error, but CT is broken, so it just eats the original error location anyway. We should go and fix it in otp upstream, probably it got broken during introducing line numbering.

@codecov
Copy link

codecov bot commented May 27, 2021

Codecov Report

Merging #3133 (d5ca4d0) into master (5166ab3) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3133   +/-   ##
=======================================
  Coverage   79.46%   79.47%           
=======================================
  Files         395      395           
  Lines       32168    32168           
=======================================
+ Hits        25563    25566    +3     
+ Misses       6605     6602    -3     
Impacted Files Coverage Δ
src/pubsub/node_hometree.erl 77.77% <0.00%> (-5.56%) ⬇️
...bal_distrib/mod_global_distrib_hosts_refresher.erl 71.69% <0.00%> (-1.89%) ⬇️
src/wpool/mongoose_wpool.erl 84.00% <0.00%> (-1.00%) ⬇️
src/ejabberd_sm.erl 84.17% <0.00%> (-0.34%) ⬇️
src/mam/mod_mam_utils.erl 89.56% <0.00%> (-0.34%) ⬇️
src/mod_muc_room.erl 77.45% <0.00%> (-0.06%) ⬇️
src/mod_muc_log.erl 77.72% <0.00%> (ø)
src/ejabberd_c2s.erl 89.57% <0.00%> (+0.29%) ⬆️
src/pubsub/mod_pubsub_db_mnesia.erl 92.82% <0.00%> (+0.42%) ⬆️
...c/global_distrib/mod_global_distrib_server_mgr.erl 77.40% <0.00%> (+0.56%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5166ab3...d5ca4d0. Read the comment docs.

Copy link
Collaborator

@NelsonVides NelsonVides left a comment

Choose a reason for hiding this comment

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

LGTM

@NelsonVides NelsonVides merged commit 404f6a0 into master May 27, 2021
@NelsonVides NelsonVides deleted the mu-safer-ct-hooks branch May 27, 2021 09:24
NelsonVides added a commit that referenced this pull request May 31, 2021
Multitenancy for MAM/MUC

This PR addresses "MAM/MUC test suites are passing for dynamic domains".

Proposed changes include:
    hook refactoring
    Common Tests Hooks for big tests fail, if init_per_suite fails. Added some more strictness there. Separate PR: #3133
    More granular rules in domain_isolation_SUITE, also converted it for HostTypes.
    We COULD pass hosttype as a separate arg to muclight DB backend (and we probably should), but it would increase complexity of the current PR, I would like to do it in a separate one.
    ejabberd_users refactoring would be a separate PR.
    REST API should be done in a separate PR.
    commands API should be done in a separate PR.

Moved into #3137:
    Add reason to some XMPP and HTTP errors.
    New module mongoose_record_pp to pretty print records in logs (it should be called manually though).
@Premwoik Premwoik added this to the 5.0.0 milestone Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants