-
Notifications
You must be signed in to change notification settings - Fork 428
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
Avoid duplicated close_session #3983
Conversation
Ensuring that the session is closed should be the responsibility of the c2s code, which will handle the case on an external state machine state. Running close_session when the session is resumed means running close_session twice.
6e9ce92
to
e8c15cb
Compare
small_tests_24 / small_tests / e8c15cb small_tests_25 / small_tests / e8c15cb dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / e8c15cb ldap_mnesia_24 / ldap_mnesia / e8c15cb ldap_mnesia_25 / ldap_mnesia / e8c15cb dynamic_domains_mysql_redis_25 / mysql_redis / e8c15cb dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / e8c15cb pgsql_mnesia_24 / pgsql_mnesia / e8c15cb elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / e8c15cb riak_mnesia_24 / riak_mnesia / e8c15cb mysql_redis_25 / mysql_redis / e8c15cb muc_SUITE:hibernation:hibernated_room_can_be_queried_for_archive{error,{{assertion_failed,assert,is_groupchat_message,
[<<"Restorable message">>],
undefined,"undefined"},
[{escalus_new_assert,assert_true,2,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
{line,84}]},
{muc_SUITE,wait_for_mam_result,3,
[{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
{line,4394}]},
{muc_SUITE,'-hibernated_room_can_be_queried_for_archive/1-fun-0-',3,
[{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
{line,4130}]},
{escalus_story,story,4,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{muc_SUITE,hibernated_room_can_be_queried_for_archive,1,
[{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
{line,4126}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} pgsql_mnesia_25 / pgsql_mnesia / e8c15cb mssql_mnesia_25 / odbc_mssql_mnesia / e8c15cb dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / e8c15cb service_mongoose_system_metrics_SUITE:system_metrics_are_reported_to_configurable_google_analytics{error,
{{assertEqual,
[{module,service_mongoose_system_metrics_SUITE},
{line,470},
{expression,"ActualTrackingIds"},
{expected,[<<"UA-151671255-1">>,<<"UA-EXTRA-TRACKING-ID">>]},
{value,[<<"UA-151671255-1">>]}]},
[{service_mongoose_system_metrics_SUITE,
events_are_reported_to_tracking_ids,1,
[{file,
"/home/circleci/project/big_tests/tests/service_mongoose_system_metrics_SUITE.erl"},
{line,470}]},
{service_mongoose_system_metrics_SUITE,
system_metrics_are_reported_to_configurable_google_analytics,1,
[{file,
"/home/circleci/project/big_tests/tests/service_mongoose_system_metrics_SUITE.erl"},
{line,204}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / e8c15cb mysql_redis_25 / mysql_redis / e8c15cb |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #3983 +/- ##
==========================================
+ Coverage 83.55% 83.57% +0.01%
==========================================
Files 538 538
Lines 33975 33970 -5
==========================================
+ Hits 28387 28389 +2
+ Misses 5588 5581 -7
... and 14 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍 I'm just wondering if we can come up with a test case wich would reproduce it, fail on previous version and pass on the new version. What do you think?
After discussion with @NelsonVides it turned out that such case is not straightforward to reproduce and we decided to merge it without such test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
Ensuring that the session is closed should be the responsibility of the c2s code, which will handle the case on an external state machine state. Running close_session when the session is resumed means running close_session twice.
This PR is a quickfix for that case.