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

Dedicate fed1 node for s2s tests #1736

Merged
merged 1 commit into from Feb 28, 2018
Merged

Dedicate fed1 node for s2s tests #1736

merged 1 commit into from Feb 28, 2018

Conversation

arcusfelis
Copy link
Contributor

Separate s2s and global distribution test nodes.
Global distribution tests should use new node reg1 (region 1)
localhost should not be loaded on fed1, because it would make
any additional s2s tests hard to implement.
An example of such test is muc and s2s combination.
Remove debug printing to console from mod_global_distrib_SUITE.
Use generic rpc function provided by escalus from mod_global_distrib_SUITE.
Update docs.

This PR addresses "I can't merge #1311" without this patch.

@kzemek
Copy link
Contributor

kzemek commented Feb 19, 2018

This PR addresses "I can't merge #1311" without this patch.

Can you elaborate?

FWIW, I reused fed1 for global distribution because the pain of testing increases nonlinearly with the number of nodes (esp. for local tests).

@arcusfelis
Copy link
Contributor Author

arcusfelis commented Feb 20, 2018

@kzemek sure, that PR has additional tests for MUC over s2s.
So, the tests would not work, because stanzas, routed to "muc.localhost" would e routed to the local node (i.e. fed1). I need to remove "localhost" from fed1 to fix that tests.

This lovely group of tests uses s2s https://github.com/esl/MongooseIM/pull/1311/files#r117275074

@arcusfelis
Copy link
Contributor Author

But overall, any XMPP interaction should work over s2s.
The fact that we don't test every feature over s2s can hide some bugs.
So, one of the way to start testing, is to override user specs in init_per_group and test several user-configurations.

@arcusfelis
Copy link
Contributor Author

yeah, a bit too many nodes :)
But separate nodes provide better isolation - that means easier debugging/refactoring.

%% This node is for global distribution testing.
%% reg is short for region.
%% "localhost" host should be defined.
{hosts, "[ \"reg1\", \"localhost\" ]"}.
Copy link
Contributor

Choose a reason for hiding this comment

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

"reg1" domain not needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The funny thing is that tests want this host, otherwise they would fail with reason:

  • "reg1 is not a member of the host list".

Copy link
Member

Choose a reason for hiding this comment

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

It's not funny. Every host in GD setup needs both global domain and local domain.

@@ -96,13 +96,13 @@ init_per_suite(Config0) ->

Config1 = [{s2s_opts, S2S} | escalus:init_per_suite(Config0)],
Config2 = [{escalus_user_db, xmpp} | Config1],
escalus:create_users(Config2, escalus:get_users([alice2, alice_bis, bob])).
escalus:create_users(Config2, escalus:get_users([alice2, alice, bob])).
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason behind these changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverts s2s_SUITE back before 85cc372#diff-b7f4c309c3aa8e798c16028cd03d1e3e.
In that commit, alice was replaced with alice_bis, because it's was not possible to use alice in s2s.
I've reverted it back, because we use alice everywhere else.

Separate s2s and global distribution test nodes.
Global distribution tests should use new node reg1 (region 1)
localhost should not be loaded on fed1, because it would make
any additional s2s tests hard to implement.
An example of such test is muc and s2s combination.
Remove debug printing to console from mod_global_distrib_SUITE.
Use generic rpc function provided by escalus from mod_global_distrib_SUITE.
Update docs.
@arcusfelis
Copy link
Contributor Author

arcusfelis commented Feb 27, 2018

@fenek ok, I've updated "header" of reg1.vars.config, so we would not remove some of the hosts, trying to "optimize" stuff :)
Still not sure, how mim1 node works without a "local" host.

@fenek
Copy link
Member

fenek commented Feb 27, 2018

@arcusfelis
localhost.bis is the local host of mim1.

@arcusfelis arcusfelis merged commit f9443c0 into master Feb 28, 2018
@fenek fenek deleted the reg1-node branch February 28, 2018 09:18
@fenek fenek added this to the 3.0.0 milestone Mar 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants