Re: [Freeipa-devel] [TEST][patch-0033] Added assertion errors to topology tests, track N 5772

2016-04-20 Thread Martin Basti 



On 12.04.2016 16:15, Martin Babinsky wrote:

On 04/06/2016 02:40 PM, Oleg Fayans wrote:

Hi Martin,

The updated patches are attached

On 04/04/2016 06:46 PM, Martin Babinsky wrote:

On 03/31/2016 05:15 PM, Oleg Fayans wrote:

Hi Martin,

Thanks for the review. The updated patch(es) are included

Testrun output can be found here:

http://fpaste.org/347800/59421745/

On 03/31/2016 01:10 PM, Martin Basti wrote:



On 31.03.2016 12:07, Oleg Fayans wrote:

Please, disregard it for a while, it does not pass lint.

On 03/31/2016 12:05 PM, Oleg Fayans wrote:




NACK

Please send unrelated changes in separate patches. I do not see 
relation
between changing variable names, adding assertion messages and 
setting

replication sleep-a-bit wait.


Agreed. There are two necessary bugfixes for the testsuite to run. 
They

were put into a separate patch



IMO to the ticket in the patch only assertion changes are related.

For the pylint related errors:
assert ('any value', 'in tuple')
is always true.
right syntax is
assert (any test), ('error msg')


thank you!



Martin^2





Hi Oleg,

Patch 0033:

1.)
First of all, the commit message does not tell much about what the 
patch

does, I would prefer something like "Improve reporting of failed tests
in topology test suite".


Done



2.) Since what you are doing is basically comparing the contents of a
list of dicts containing expected data with a list of dicts with data
parsed from test results, why are you not using "assert_deepequal"
function from 'ipatests/util.py' module? It is also used in e.g. XMLRPC
tests and it reports the failures fairly well (e.g. redundant keys,
missing keys, expected value mismatch, container length mismatch etc.)


Indeed :) Done.



Patch 034:

I think this is a good use case for 'wait_for_replication' function 
from

'ipatests/test_integration/tasks.py' module. You need to establish LDAP
connection to the host but this is very easy, see how it's done in
'test_simple_replication' suite[1].

I would prefer this approach (if applicable) instead of using sleep().


Implemented.



In any way, your formatting of assertions is wrong and makes the code
nearly unreadable. See the attached patch for an example of pep8
compliant and pleasant formatting


Thank you, that looks really way more readable.



[1]
https://git.fedorahosted.org/cgit/freeipa.git/tree/ipatests/test_integration/test_simple_replication.py#n42 










Thanks, ACK.


Pushed

master:
* 1974f20aec8de61d0e8d5a550df6a5fabd4b383a Improve reporting of failed 
tests in topology test suite
* 1c79c1ea2d077d8699c7e3190526a45e627a7a18 Bugfixes in managed topology 
tests

ipa-4-3:
* b41164f237341ccf2546b73585e804aac4cffc8e Improve reporting of failed 
tests in topology test suite
* 10e9e33ac057d5ad4e42cd7207b7203ce1d100d1 Bugfixes in managed topology 
tests


--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [TEST][patch-0033] Added assertion errors to topology tests, track N 5772

2016-04-12 Thread Martin Babinsky 

On 04/06/2016 02:40 PM, Oleg Fayans wrote:

Hi Martin,

The updated patches are attached

On 04/04/2016 06:46 PM, Martin Babinsky wrote:

On 03/31/2016 05:15 PM, Oleg Fayans wrote:

Hi Martin,

Thanks for the review. The updated patch(es) are included

Testrun output can be found here:

http://fpaste.org/347800/59421745/

On 03/31/2016 01:10 PM, Martin Basti wrote:



On 31.03.2016 12:07, Oleg Fayans wrote:

Please, disregard it for a while, it does not pass lint.

On 03/31/2016 12:05 PM, Oleg Fayans wrote:




NACK

Please send unrelated changes in separate patches. I do not see relation
between changing variable names, adding assertion messages and setting
replication sleep-a-bit wait.


Agreed. There are two necessary bugfixes for the testsuite to run. They
were put into a separate patch



IMO to the ticket in the patch only assertion changes are related.

For the pylint related errors:
assert ('any value', 'in tuple')
is always true.
right syntax is
assert (any test), ('error msg')


thank you!



Martin^2





Hi Oleg,

Patch 0033:

1.)
First of all, the commit message does not tell much about what the patch
does, I would prefer something like "Improve reporting of failed tests
in topology test suite".


Done



2.) Since what you are doing is basically comparing the contents of a
list of dicts containing expected data with a list of dicts with data
parsed from test results, why are you not using "assert_deepequal"
function from 'ipatests/util.py' module? It is also used in e.g. XMLRPC
tests and it reports the failures fairly well (e.g. redundant keys,
missing keys, expected value mismatch, container length mismatch etc.)


Indeed :) Done.



Patch 034:

I think this is a good use case for 'wait_for_replication' function from
'ipatests/test_integration/tasks.py' module. You need to establish LDAP
connection to the host but this is very easy, see how it's done in
'test_simple_replication' suite[1].

I would prefer this approach (if applicable) instead of using sleep().


Implemented.



In any way, your formatting of assertions is wrong and makes the code
nearly unreadable. See the attached patch for an example of pep8
compliant and pleasant formatting


Thank you, that looks really way more readable.



[1]
https://git.fedorahosted.org/cgit/freeipa.git/tree/ipatests/test_integration/test_simple_replication.py#n42








Thanks, ACK.

--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [TEST][patch-0033] Added assertion errors to topology tests, track N 5772

2016-04-06 Thread Oleg Fayans 
Hi Martin,

The updated patches are attached

On 04/04/2016 06:46 PM, Martin Babinsky wrote:
> On 03/31/2016 05:15 PM, Oleg Fayans wrote:
>> Hi Martin,
>>
>> Thanks for the review. The updated patch(es) are included
>>
>> Testrun output can be found here:
>>
>> http://fpaste.org/347800/59421745/
>>
>> On 03/31/2016 01:10 PM, Martin Basti wrote:
>>>
>>>
>>> On 31.03.2016 12:07, Oleg Fayans wrote:
 Please, disregard it for a while, it does not pass lint.

 On 03/31/2016 12:05 PM, Oleg Fayans wrote:
>
>
>>> NACK
>>>
>>> Please send unrelated changes in separate patches. I do not see relation
>>> between changing variable names, adding assertion messages and setting
>>> replication sleep-a-bit wait.
>>
>> Agreed. There are two necessary bugfixes for the testsuite to run. They
>> were put into a separate patch
>>
>>>
>>> IMO to the ticket in the patch only assertion changes are related.
>>>
>>> For the pylint related errors:
>>> assert ('any value', 'in tuple')
>>> is always true.
>>> right syntax is
>>> assert (any test), ('error msg')
>>
>> thank you!
>>
>>>
>>> Martin^2
>>
>>
>>
> Hi Oleg,
> 
> Patch 0033:
> 
> 1.)
> First of all, the commit message does not tell much about what the patch
> does, I would prefer something like "Improve reporting of failed tests
> in topology test suite".

Done

> 
> 2.) Since what you are doing is basically comparing the contents of a
> list of dicts containing expected data with a list of dicts with data
> parsed from test results, why are you not using "assert_deepequal"
> function from 'ipatests/util.py' module? It is also used in e.g. XMLRPC
> tests and it reports the failures fairly well (e.g. redundant keys,
> missing keys, expected value mismatch, container length mismatch etc.)

Indeed :) Done.

> 
> Patch 034:
> 
> I think this is a good use case for 'wait_for_replication' function from
> 'ipatests/test_integration/tasks.py' module. You need to establish LDAP
> connection to the host but this is very easy, see how it's done in
> 'test_simple_replication' suite[1].
> 
> I would prefer this approach (if applicable) instead of using sleep().

Implemented.

> 
> In any way, your formatting of assertions is wrong and makes the code
> nearly unreadable. See the attached patch for an example of pep8
> compliant and pleasant formatting

Thank you, that looks really way more readable.

> 
> [1]
> https://git.fedorahosted.org/cgit/freeipa.git/tree/ipatests/test_integration/test_simple_replication.py#n42
> 
> 
> 
> 

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 6ce1ad871d2f83558ee64f58bd010d0219c8e3e7 Mon Sep 17 00:00:00 2001
From: Oleg Fayans 
Date: Wed, 6 Apr 2016 10:46:58 +0200
Subject: [PATCH] Improve reporting of failed tests in topology test suite

https://fedorahosted.org/freeipa/ticket/5772
---
 ipatests/test_integration/test_topology.py | 39 --
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/ipatests/test_integration/test_topology.py b/ipatests/test_integration/test_topology.py
index 8119aa9d7c9a57e68a89ef1d8086920286e64316..c434c6c441a4d73662eb9805df67c51c69cbf3c0 100644
--- a/ipatests/test_integration/test_topology.py
+++ b/ipatests/test_integration/test_topology.py
@@ -11,6 +11,7 @@ from ipatests.test_integration.base import IntegrationTest
 from ipatests.test_integration import tasks
 from ipatests.test_integration.env_config import get_global_config
 from ipalib.constants import DOMAIN_SUFFIX_NAME
+from ipatests.util import assert_deepequal
 
 config = get_global_config()
 reasoning = "Topology plugin disabled due to domain level 0"
@@ -61,15 +62,16 @@ class TestTopologyOptions(IntegrationTest):
 """
 tasks.kinit_admin(self.master)
 result1 = self.master.run_command(['ipa', 'topologysegment-find',
-   DOMAIN_SUFFIX_NAME])
+   DOMAIN_SUFFIX_NAME]).stdout_text
 first_segment_name = "%s-to-%s" % (self.master.hostname,
self.replicas[0].hostname)
-output1 = result1.stdout_text
-firstsegment = self.tokenize_topologies(output1)[0]
-assert(firstsegment['name'] == first_segment_name)
-assert(self.noentries_re.search(output1).group(1) == "1")
-assert(firstsegment['leftnode'] == self.master.hostname)
-assert(firstsegment['rightnode'] == self.replicas[0].hostname)
+expected_segment = {
+'connectivity': 'both',
+'leftnode': self.master.hostname,
+'name': first_segment_name,
+'rightnode': self.replicas[0].hostname}
+firstsegment = self.tokenize_topologies(result1)[0]
+assert_deepequal(expected_segment, firstsegment)
 tasks.install_replica(self.master, self.replicas[1], setup_ca=False,
   setup_dns=False)
 # We need to make sure topology information is consistent

Re: [Freeipa-devel] [TEST][patch-0033] Added assertion errors to topology tests, track N 5772

2016-04-04 Thread Martin Babinsky 

On 03/31/2016 05:15 PM, Oleg Fayans wrote:

Hi Martin,

Thanks for the review. The updated patch(es) are included

Testrun output can be found here:

http://fpaste.org/347800/59421745/

On 03/31/2016 01:10 PM, Martin Basti wrote:



On 31.03.2016 12:07, Oleg Fayans wrote:

Please, disregard it for a while, it does not pass lint.

On 03/31/2016 12:05 PM, Oleg Fayans wrote:




NACK

Please send unrelated changes in separate patches. I do not see relation
between changing variable names, adding assertion messages and setting
replication sleep-a-bit wait.


Agreed. There are two necessary bugfixes for the testsuite to run. They
were put into a separate patch



IMO to the ticket in the patch only assertion changes are related.

For the pylint related errors:
assert ('any value', 'in tuple')
is always true.
right syntax is
assert (any test), ('error msg')


thank you!



Martin^2





Hi Oleg,

Patch 0033:

1.)
First of all, the commit message does not tell much about what the patch 
does, I would prefer something like "Improve reporting of failed tests 
in topology test suite".


2.) Since what you are doing is basically comparing the contents of a 
list of dicts containing expected data with a list of dicts with data 
parsed from test results, why are you not using "assert_deepequal" 
function from 'ipatests/util.py' module? It is also used in e.g. XMLRPC 
tests and it reports the failures fairly well (e.g. redundant keys, 
missing keys, expected value mismatch, container length mismatch etc.)


Patch 034:

I think this is a good use case for 'wait_for_replication' function from 
'ipatests/test_integration/tasks.py' module. You need to establish LDAP 
connection to the host but this is very easy, see how it's done in 
'test_simple_replication' suite[1].


I would prefer this approach (if applicable) instead of using sleep().

In any way, your formatting of assertions is wrong and makes the code 
nearly unreadable. See the attached patch for an example of pep8 
compliant and pleasant formatting


[1] 
https://git.fedorahosted.org/cgit/freeipa.git/tree/ipatests/test_integration/test_simple_replication.py#n42


--
Martin^3 Babinsky
diff --git a/ipatests/test_integration/test_topology.py b/ipatests/test_integration/test_topology.py
index 39e03ee..6799e26 100644
--- a/ipatests/test_integration/test_topology.py
+++ b/ipatests/test_integration/test_topology.py
@@ -72,15 +72,15 @@ class TestTopologyOptions(IntegrationTest):
 output1 = result1.stdout_text
 firstsegment = self.tokenize_topologies(output1)[0]
 num_of_entries = self.noentries_re.search(output1).group(1)
-assert(firstsegment['name'] == first_segment_name),\
-"A segment with non-standard name was created: %s" % \
-firstsegment['name']
-assert(num_of_entries == "1"),\
-"Unexpected number of segments found: %s" % num_of_entries
-assert(firstsegment['leftnode'] == master.hostname),\
-"Incorrect leftnode: %s" % firstsegment['leftnode']
-assert(firstsegment['rightnode'] == replica1.hostname),\
-"Incorrect rightnode: %s" % firstsegment['rightnode']
+assert(firstsegment['name'] == first_segment_name), (
+"A segment with non-standard name was created: %s"
+%firstsegment['name'])
+assert(num_of_entries == "1"), (
+"Unexpected number of segments found: %s" % num_of_entries)
+assert(firstsegment['leftnode'] == master.hostname), (
+"Incorrect leftnode: %s" % firstsegment['leftnode'])
+assert(firstsegment['rightnode'] == replica1.hostname), (
+"Incorrect rightnode: %s" % firstsegment['rightnode'])
 tasks.install_replica(master, replica2, setup_ca=False,
   setup_dns=False)
 # We need to make sure topology information is consistent across all
@@ -92,14 +92,14 @@ class TestTopologyOptions(IntegrationTest):
 result4 = replica2.run_command(['ipa', 'topologysegment-find',
 DOMAIN_SUFFIX_NAME])
 segments = self.tokenize_topologies(result2.stdout_text)
-assert(len(segments) == 2),\
-"Unexpected number of segments found: %i" % len(segments)
-assert(result2.stdout_text == result3.stdout_text),\
-"Failed replication between %s and %s" % (master.hostname,
-  replica1.hostname)
-assert(result3.stdout_text == result4.stdout_text),\
+assert(len(segments) == 2), (
+"Unexpected number of segments found: %i" % len(segments))
+assert(result2.stdout_text == result3.stdout_text), (
+"Failed replication between %s and %s" % (master.hostname,
+  replica1.hostname))
+assert(result3.stdout_text == result4.stdout_text), (
 "Failed replication between %s and %s" % (replica1.hostname,
-

Re: [Freeipa-devel] [TEST][patch-0033] Added assertion errors to topology tests, track N 5772

2016-03-31 Thread Oleg Fayans 
Hi Martin,

Thanks for the review. The updated patch(es) are included

Testrun output can be found here:

http://fpaste.org/347800/59421745/

On 03/31/2016 01:10 PM, Martin Basti wrote:
> 
> 
> On 31.03.2016 12:07, Oleg Fayans wrote:
>> Please, disregard it for a while, it does not pass lint.
>>
>> On 03/31/2016 12:05 PM, Oleg Fayans wrote:
>>>
>>>
> NACK
> 
> Please send unrelated changes in separate patches. I do not see relation
> between changing variable names, adding assertion messages and setting
> replication sleep-a-bit wait.

Agreed. There are two necessary bugfixes for the testsuite to run. They
were put into a separate patch

> 
> IMO to the ticket in the patch only assertion changes are related.
> 
> For the pylint related errors:
> assert ('any value', 'in tuple')
> is always true.
> right syntax is
> assert (any test), ('error msg')

thank you!

> 
> Martin^2

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 9109714a0706884d832f7abb3dd4c3252bc30c5a Mon Sep 17 00:00:00 2001
From: Oleg Fayans 
Date: Thu, 31 Mar 2016 16:53:54 +0200
Subject: [PATCH] Added assertion error messages in topology tests

https://fedorahosted.org/freeipa/ticket/5772
---
 ipatests/test_integration/test_topology.py | 88 ++
 1 file changed, 52 insertions(+), 36 deletions(-)

diff --git a/ipatests/test_integration/test_topology.py b/ipatests/test_integration/test_topology.py
index 8119aa9d7c9a57e68a89ef1d8086920286e64316..de27e6581f59660a8b28c789a31e04ddd2f3b7e4 100644
--- a/ipatests/test_integration/test_topology.py
+++ b/ipatests/test_integration/test_topology.py
@@ -59,18 +59,27 @@ class TestTopologyOptions(IntegrationTest):
 Test_plan#Test_case:
 _Replication_topology_should_be_saved_in_the_LDAP_tree
 """
-tasks.kinit_admin(self.master)
-result1 = self.master.run_command(['ipa', 'topologysegment-find',
-   DOMAIN_SUFFIX_NAME])
-first_segment_name = "%s-to-%s" % (self.master.hostname,
-   self.replicas[0].hostname)
+master = self.master
+replica1 = self.replicas[0]
+replica2 = self.replicas[1]
+tasks.kinit_admin(master)
+result1 = master.run_command(['ipa', 'topologysegment-find',
+  DOMAIN_SUFFIX_NAME])
+first_segment_name = "%s-to-%s" % (master.hostname,
+   replica1.hostname)
 output1 = result1.stdout_text
 firstsegment = self.tokenize_topologies(output1)[0]
-assert(firstsegment['name'] == first_segment_name)
-assert(self.noentries_re.search(output1).group(1) == "1")
-assert(firstsegment['leftnode'] == self.master.hostname)
-assert(firstsegment['rightnode'] == self.replicas[0].hostname)
-tasks.install_replica(self.master, self.replicas[1], setup_ca=False,
+num_of_entries = self.noentries_re.search(output1).group(1)
+assert(firstsegment['name'] == first_segment_name),\
+"A segment with non-standard name was created: %s" % \
+firstsegment['name']
+assert(num_of_entries == "1"),\
+"Unexpected number of segments found: %s" % num_of_entries
+assert(firstsegment['leftnode'] == master.hostname),\
+"Incorrect leftnode: %s" % firstsegment['leftnode']
+assert(firstsegment['rightnode'] == replica1.hostname),\
+"Incorrect rightnode: %s" % firstsegment['rightnode']
+tasks.install_replica(master, replica2, setup_ca=False,
   setup_dns=False)
 # We need to make sure topology information is consistent across all
 # replicas
@@ -81,16 +90,18 @@ class TestTopologyOptions(IntegrationTest):
 result4 = self.replicas[1].run_command(['ipa', 'topologysegment-find',
 DOMAIN_SUFFIX_NAME])
 segments = self.tokenize_topologies(result2.stdout_text)
-assert(len(segments) == 2)
-assert(result2.stdout_text == result3.stdout_text)
-assert(result3.stdout_text == result4.stdout_text)
+assert(len(segments) == 2),\
+"Unexpected number of segments found: %i" % len(segments)
+assert(result2.stdout_text == result3.stdout_text),\
+"Failed replication between %s and %s" % (master.hostname,
+  replica1.hostname)
+assert(result3.stdout_text == result4.stdout_text),\
+"Failed replication between %s and %s" % (replica1.hostname,
+  replica2.hostname)
 # Now let's check that uninstalling the replica will update the topology
 # info on the rest of replicas.
-tasks.uninstall_master(self.replicas[1])
-tasks.clean_replication_agreement(self.master, self.replicas[1])
-result5 = self.master.run_command(['ipa',

Re: [Freeipa-devel] [TEST][patch-0033] Added assertion errors to topology tests, track N 5772

2016-03-31 Thread Martin Basti 



On 31.03.2016 12:07, Oleg Fayans wrote:

Please, disregard it for a while, it does not pass lint.

On 03/31/2016 12:05 PM, Oleg Fayans wrote:




NACK

Please send unrelated changes in separate patches. I do not see relation 
between changing variable names, adding assertion messages and setting 
replication sleep-a-bit wait.


IMO to the ticket in the patch only assertion changes are related.

For the pylint related errors:
assert ('any value', 'in tuple')
is always true.
right syntax is
assert (any test), ('error msg')

Martin^2

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [TEST][patch-0033] Added assertion errors to topology tests, track N 5772

2016-03-31 Thread Oleg Fayans 
Please, disregard it for a while, it does not pass lint.

On 03/31/2016 12:05 PM, Oleg Fayans wrote:
> 
> 
> 

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [TEST][patch-0033] Added assertion errors to topology tests, track N 5772

2016-03-31 Thread Oleg Fayans 

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From f1821a41498f7829df4d1bdffd338cb70b413667 Mon Sep 17 00:00:00 2001
From: Oleg Fayans 
Date: Thu, 31 Mar 2016 11:58:26 +0200
Subject: [PATCH] Added assertion error messages in topology tests

Added a global sleep interval used to wait for replication
https://fedorahosted.org/freeipa/ticket/5772
---
 ipatests/test_integration/test_topology.py | 113 +
 1 file changed, 67 insertions(+), 46 deletions(-)

diff --git a/ipatests/test_integration/test_topology.py b/ipatests/test_integration/test_topology.py
index 8119aa9d7c9a57e68a89ef1d8086920286e64316..ea03fb6eac7db9e4acc0f8ffcfa38e0fba92d761 100644
--- a/ipatests/test_integration/test_topology.py
+++ b/ipatests/test_integration/test_topology.py
@@ -14,6 +14,8 @@ from ipalib.constants import DOMAIN_SUFFIX_NAME
 
 config = get_global_config()
 reasoning = "Topology plugin disabled due to domain level 0"
+REPLICATION_SLEEP_TIME = 10
+
 
 @pytest.mark.skipif(config.domain_level == 0, reason=reasoning)
 class TestTopologyOptions(IntegrationTest):
@@ -59,38 +61,49 @@ class TestTopologyOptions(IntegrationTest):
 Test_plan#Test_case:
 _Replication_topology_should_be_saved_in_the_LDAP_tree
 """
-tasks.kinit_admin(self.master)
-result1 = self.master.run_command(['ipa', 'topologysegment-find',
-   DOMAIN_SUFFIX_NAME])
-first_segment_name = "%s-to-%s" % (self.master.hostname,
-   self.replicas[0].hostname)
+master = self.master
+replica1 = self.replicas[0]
+replica2 = self.replicas[1]
+tasks.kinit_admin(master)
+result1 = master.run_command(['ipa', 'topologysegment-find',
+  DOMAIN_SUFFIX_NAME])
+first_segment_name = "%s-to-%s" % (master.hostname,
+   replica1.hostname)
 output1 = result1.stdout_text
 firstsegment = self.tokenize_topologies(output1)[0]
-assert(firstsegment['name'] == first_segment_name)
-assert(self.noentries_re.search(output1).group(1) == "1")
-assert(firstsegment['leftnode'] == self.master.hostname)
-assert(firstsegment['rightnode'] == self.replicas[0].hostname)
-tasks.install_replica(self.master, self.replicas[1], setup_ca=False,
+num_of_entries = self.noentries_re.search(output1).group(1)
+assert(firstsegment['name'] == first_segment_name,
+   "A segment with non-standard name was created: %s" %
+   firstsegment['name'])
+assert(num_of_entries == "1",
+   "Unexpected number of segments found: %s" % num_of_entries)
+assert(firstsegment['leftnode'] == master.hostname,
+   "Incorrect leftnode: %s" % firstsegment['leftnode'])
+assert(firstsegment['rightnode'] == replica1.hostname,
+   "Incorrect rightnode: %s" % firstsegment['rightnode'])
+tasks.install_replica(master, replica2, setup_ca=False,
   setup_dns=False)
 # We need to make sure topology information is consistent across all
 # replicas
-result2 = self.master.run_command(['ipa', 'topologysegment-find',
-   DOMAIN_SUFFIX_NAME])
-result3 = self.replicas[0].run_command(['ipa', 'topologysegment-find',
-DOMAIN_SUFFIX_NAME])
-result4 = self.replicas[1].run_command(['ipa', 'topologysegment-find',
-DOMAIN_SUFFIX_NAME])
+result2 = master.run_command(['ipa', 'topologysegment-find',
+  DOMAIN_SUFFIX_NAME])
+result3 = replica1.run_command(['ipa', 'topologysegment-find',
+DOMAIN_SUFFIX_NAME])
+result4 = replica2.run_command(['ipa', 'topologysegment-find',
+DOMAIN_SUFFIX_NAME])
 segments = self.tokenize_topologies(result2.stdout_text)
-assert(len(segments) == 2)
-assert(result2.stdout_text == result3.stdout_text)
-assert(result3.stdout_text == result4.stdout_text)
+assert(len(segments) == 2,
+   "Unexpected number of segments found: %i" % len(segments))
+assert(result2.stdout_text == result3.stdout_text,
+   "Failed replication between %s and %s" % (master.hostname,
+ replica1.hostname))
+assert(result3.stdout_text == result4.stdout_text,
+   "Failed replication between %s and %s" % (replica1.hostname,
+ replica2.hostname))
 # Now let's check that uninstalling the replica will update the topology
 # info on the rest of replicas.
-