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 <ofay...@redhat.com>
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', 'topologysegment-find',
- DOMAIN_SUFFIX_NAME])
- assert(self.noentries_re.search(result5.stdout_text).group(1) == "1")
+ tasks.uninstall_master(replica2)
+ tasks.clean_replication_agreement(master, replica2)
def test_add_remove_segment(self):
"""
@@ -99,38 +110,43 @@ class TestTopologyOptions(IntegrationTest):
Testcase http://www.freeipa.org/page/V4/Manage_replication_topology/
Test_plan#Test_case:_Basic_CRUD_test
"""
- tasks.kinit_admin(self.master)
+
+ master = self.master
+ replica1 = self.replicas[0]
+ replica2 = self.replicas[1]
+ tasks.kinit_admin(master)
# Install the second replica
- tasks.install_replica(self.master, self.replicas[1], setup_ca=False,
+ tasks.install_replica(master, replica2, setup_ca=False,
setup_dns=False)
# turn a star into a ring
- segment, err = tasks.create_segment(self.master,
- self.replicas[0],
- self.replicas[1])
+ segment, err = tasks.create_segment(master, replica1, replica2)
assert err == "", err
# Make sure the new segment is shown by `ipa topologysegment-find`
- result1 = self.master.run_command(['ipa', 'topologysegment-find',
- DOMAIN_SUFFIX_NAME])
- assert(result1.stdout_text.find(segment['name']) > 0)
+ result1 = master.run_command(['ipa', 'topologysegment-find',
+ DOMAIN_SUFFIX_NAME])
+ assert(result1.stdout_text.find(segment['name']) > 0),\
+ "The segment %s was not found on master" % segment['name']
# Remove master <-> replica2 segment and make sure that the changes get
# there through replica1
- deleteme = "%s-to-%s" % (self.master.hostname,
- self.replicas[1].hostname)
- returncode, error = tasks.destroy_segment(self.master, deleteme)
+ deleteme = "%s-to-%s" % (master.hostname,
+ replica2.hostname)
+ returncode, error = tasks.destroy_segment(master, deleteme)
assert returncode == 0, error
# make sure replica1 does not have segment that was deleted on master
- result3 = self.replicas[0].run_command(['ipa', 'topologysegment-find',
- DOMAIN_SUFFIX_NAME])
- assert(result3.stdout_text.find(deleteme) < 0)
+ result3 = replica1.run_command(['ipa', 'topologysegment-find',
+ DOMAIN_SUFFIX_NAME])
+ assert(result3.stdout_text.find(deleteme) < 0),\
+ "The segment %s deleted on master is still shown on replica" % deleteme
# Create test data on master and make sure it gets all the way down to
# replica2 through replica1
- self.master.run_command(['ipa', 'user-add', 'someuser',
- '--first', 'test',
- '--last', 'user'])
+ master.run_command(['ipa', 'user-add', 'someuser',
+ '--first', 'test',
+ '--last', 'user'])
time.sleep(60) # replication requires some time
users_on_replica2 = self.replicas[1].run_command(['ipa',
'user-find'])
- assert(users_on_replica2.find('someuser') > 0)
+ assert(users_on_replica2.find('someuser') > 0),\
+ "Replication was not completed within 60 seconds"
# We end up having a line topology: master <-> replica1 <-> replica2
def test_remove_the_only_connection(self):
--
1.8.3.1
From ba188ea6e39de6bfc6b633b941e0b9b66fa23de4 Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Thu, 31 Mar 2016 17:08:05 +0200
Subject: [PATCH] Bugfixes in replication_topology tests
Fixed a false negative related to replication taking some time
Fixed a bug with regex applied to ssh response object instead of stdout_text
---
ipatests/test_integration/test_topology.py | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/ipatests/test_integration/test_topology.py b/ipatests/test_integration/test_topology.py
index de27e6581f59660a8b28c789a31e04ddd2f3b7e4..39e03ee689475285f2467d3c36896672968b1d8f 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):
@@ -83,12 +85,12 @@ class TestTopologyOptions(IntegrationTest):
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),\
"Unexpected number of segments found: %i" % len(segments)
@@ -132,7 +134,9 @@ class TestTopologyOptions(IntegrationTest):
replica2.hostname)
returncode, error = tasks.destroy_segment(master, deleteme)
assert returncode == 0, error
- # make sure replica1 does not have segment that was deleted on master
+ # wait till the changes get replicated and make sure replica1 does not
+ # have segment that was deleted on master
+ time.sleep(REPLICATION_SLEEP_TIME)
result3 = replica1.run_command(['ipa', 'topologysegment-find',
DOMAIN_SUFFIX_NAME])
assert(result3.stdout_text.find(deleteme) < 0),\
@@ -142,11 +146,12 @@ class TestTopologyOptions(IntegrationTest):
master.run_command(['ipa', 'user-add', 'someuser',
'--first', 'test',
'--last', 'user'])
- time.sleep(60) # replication requires some time
- users_on_replica2 = self.replicas[1].run_command(['ipa',
- 'user-find'])
+ time.sleep(REPLICATION_SLEEP_TIME) # replication requires some time
+ users_on_replica2 = replica2.run_command(['ipa',
+ 'user-find']).stdout_text
assert(users_on_replica2.find('someuser') > 0),\
- "Replication was not completed within 60 seconds"
+ "Replication was not completed within %s seconds" % \
+ REPLICATION_SLEEP_TIME
# We end up having a line topology: master <-> replica1 <-> replica2
def test_remove_the_only_connection(self):
--
1.8.3.1
--
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
