-
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
Xeps/bind2 inlines #4114
Xeps/bind2 inlines #4114
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## feature/sasl2 #4114 +/- ##
=================================================
+ Coverage 84.02% 84.03% +0.01%
=================================================
Files 554 556 +2
Lines 33875 33941 +66
=================================================
+ Hits 28462 28521 +59
- Misses 5413 5420 +7
☔ View full report in Codecov by Sentry. |
26e64da
to
b7b3a53
Compare
c4ef17d
to
8e9958a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
b7b3a53
to
9ee0ac3
Compare
8e9958a
to
14a62f5
Compare
This comment was marked as outdated.
This comment was marked as outdated.
9ee0ac3
to
c460ef3
Compare
We will crash on the next call if there was no sid anyway.
14a62f5
to
41f35a9
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
6829950
to
17406fa
Compare
17406fa
to
b0904ed
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 0e92f49 small_tests_24 / small_tests / 0e92f49 small_tests_25 / small_tests / 0e92f49 small_tests_25_arm64 / small_tests / 0e92f49 ldap_mnesia_24 / ldap_mnesia / 0e92f49 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 0e92f49 ldap_mnesia_25 / ldap_mnesia / 0e92f49 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 0e92f49 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 0e92f49 pgsql_cets_25 / pgsql_cets / 0e92f49 internal_mnesia_25 / internal_mnesia / 0e92f49 mysql_redis_25 / mysql_redis / 0e92f49 pgsql_mnesia_24 / pgsql_mnesia / 0e92f49 pgsql_mnesia_25 / pgsql_mnesia / 0e92f49 mssql_mnesia_25 / odbc_mssql_mnesia / 0e92f49 |
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 👍
|
||
given_client_is_inactive_and_no_messages_arrive(Alice) -> | ||
escalus:send(Alice, csi_stanza(<<"inactive">>)), | ||
then_client_does_not_receive_any_message(Alice). |
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.
I think that given
/when
/then
prefixes in helpers are confusing, but it can be a matter of preference. For me they hinder code reuse, e.g. here we have a then
in given
, which already breaks the pattern. If the helper function was called assert_client_does_not_receive_any_message
, it would look cleaner.
So we could get rid of the given
/then
prefixes, but I see how this code was moved from a suite, so it can stay as it is.
This PR introduces BIND2 support for carbons, csi, and sm.
As described in
Note that the changes to SM are not accepted yet (at the moment of writing), and for a while they claimed the SM feature inlined into SASL2 should be announced as
resume
but then it was requested it should announcesm
, as other XMPP servers and clients have already done.