Hi Martin,
On 03/16/2016 03:35 PM, Martin Basti wrote:
>
>
> On 16.03.2016 15:13, Martin Basti wrote:
>>
>>
>> On 16.03.2016 14:59, Oleg Fayans wrote:
>>> Hi Martin
>>>
>>> On 03/16/2016 02:39 PM, Martin Basti wrote:
>>>>
>>>> On 16.03.2016 10:59, Oleg Fayans wrote:
>>>>> With this patch applied integration tests pass and in-tree tests are
>>>>> gracefully skipped.
>>>>>
>>>>> @mkubik, It is not possible to put the decorator to util.py as per our
>>>>> discussion, because it uses tasks, so tasks must be imported. But
>>>>> tasks
>>>>> already import util, which leads to circular imports. So I've put
>>>>> it to
>>>>> tasks.py
>>>>>
>>>>>
>>>>>
>>>> NACK
>>>>
>>>> 1)
>>>> Use right ticket in commit message (#5723)
>>> But (#5736) is exactly the issue that is being addressed. Probably note
>>> both tickets in the commit message?
>> But as I wrote in ticket #5736, this ticket should be closed, because
>> issue is caused by ticket which is not finished yet, so we should
>> continue just with original ticket.
Done
>>
>>>
>>>> 2)
>>>> Link to ticket should be last in the commit message
Done
>>>>
>>>> 3)
>>>> dereplicafy
>>>>
>>>> 3a)
>>>> wrong doc string, it removes *only* replicas not clients
>>> No, in fact it removes both:
>>> uninstall_replica(args[0].master, host)
>>> uninstall_client(host)
>>>
>>> Both tasks have raiseonerr set to False, which means that even if
>>> replica was not installed but the client was - it will also be removed
>> I see just
>> for host in args[0].replicas
>>
>> I don't see any
>> for host in args[0].clients
>> there
>>
>> Also uninstall_client should not be there. ipa-server-install
>> --uninstall removes client too. The extra call of uninstall client is
>> IMO there just because an ancient bug that is already fixed.
That's done because some tests install client separately and then
deliberately install replica the wrong way to test that the installer
fails in a predicted way. That's why this separate uninstall_client
call. The doc string was corrected.
>>
>>>
>>>> 3b)
>>>> can we rename it to something different? (replicas_cleanup,
>>>> replicas_uninstall, replicas_teardown)
>>> replicas_cleanup, or even topo_cleanup sounds OK to me.
replicas_cleanup it is
>>>
>>>> 4)
>>>> Please fix commit message
>>>> - Wile trated correctly
>>>> - followiong
>>>> - rewrote -> rewrite
>>> Will do
Done
>>>
>>>> 5)
>>>> decorator
>>>> + def wrapped(*args):
>>>> + func(*args)
>>>> + for host in args[0].replicas:
>>>>
>>>> Shouldn't be there try-finally around func() call, or something?
>>> No, the wrapped function is a test_* method: if it fails we need to see
>>> the original failure
>> but if something raise an exception in func(), cleanup will not be
>> executed.
>>
>> You can do
>> In [4]: try:
>> ...: raise ValueError('Hello')
>> ...: finally:
>> ...: try:
>> ...: raise ValueError('Cleanup')
>> ...: except Exception:
>> ...: pass
>> ...:
>> ---------------------------------------------------------------------------
>>
>> ValueError Traceback (most recent call
>> last)
>> <ipython-input-4-affb927f7603> in <module>()
>> 1 try:
>> ----> 2 raise ValueError('Hello')
>> 3 finally:
>> 4 try:
>> 5 raise ValueError('Cleanup')
>>
>> ValueError: Hello
> On the other hand, I do not want cleanup with --pdb option, so maybe it
> should just fail
>
>>
>>>
>>>> Are you sure that there is no need to return result of func()?
>>> The same applies here: we never return results from test_* methods
>> ok
>>>
>>>> *) Please create additional patch that will add licence there
>>>>
>>>>
>>> Will do :)
>>>
>>>
>>
>
The license-related patch is attached too
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 02ffe2514252e51744a6691bfb6692cadcc69a8d Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Mon, 21 Mar 2016 08:46:11 +0100
Subject: [PATCH] rewrite a misprocessed teardown_method method as a custom
decorator
teardown_method is a standard pytest method used to put any code to be executed
after each test method is executed. While treated correctly by our integration
tests, this method is misinterpreted by in-tree tests in the following way:
in-tree tests try to execute it even if all the test methods are skipped due to
test resources being not configured. This causes the tests, that otherwise would
have been skipped, to fail
https://fedorahosted.org/freeipa/ticket/5723
---
ipatests/test_integration/tasks.py | 22 ++++++++++++++++++++++
.../test_integration/test_replica_promotion.py | 17 ++++-------------
2 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index c2129708b5e75d9b492fb8b33784478b50122115..2aea4f14c01538bf77dbe45a947c5d415c9ea744 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -1153,3 +1153,25 @@ def uninstall_replica(master, replica):
"-p", master.config.dirman_password,
replica.hostname], raiseonerr=False)
uninstall_master(replica)
+
+
+def replicas_cleanup(func):
+ """
+ replicas_cleanup decorator, applied to any test method in integration tests
+ uninstalls all replicas in the topology leaving only master
+ configured
+ """
+ def wrapped(*args):
+ func(*args)
+ for host in args[0].replicas:
+ uninstall_replica(args[0].master, host)
+ uninstall_client(host)
+ result = args[0].master.run_command(
+ ["ipa", "host-del", "--updatedns", host.hostname],
+ raiseonerr=False)
+ # Workaround for 5627
+ if "host not found" in result.stderr_text:
+ args[0].master.run_command(["ipa",
+ "host-del",
+ host.hostname], raiseonerr=False)
+ return wrapped
diff --git a/ipatests/test_integration/test_replica_promotion.py b/ipatests/test_integration/test_replica_promotion.py
index 45bfdd22e79845f7f091ce2c75a7148dc1963dba..7dc925383a517145543351706f6412b134fc8778 100644
--- a/ipatests/test_integration/test_replica_promotion.py
+++ b/ipatests/test_integration/test_replica_promotion.py
@@ -4,6 +4,7 @@ from ipatests.test_integration.test_caless import assert_error
from ipalib.constants import DOMAIN_LEVEL_0
from ipalib.constants import DOMAIN_LEVEL_1
from ipalib.constants import DOMAIN_SUFFIX_NAME
+from ipatests.test_integration.tasks import replicas_cleanup
class ReplicaPromotionBase(IntegrationTest):
@@ -12,19 +13,6 @@ class ReplicaPromotionBase(IntegrationTest):
def install(cls, mh):
tasks.install_master(cls.master, domain_level=cls.domain_level)
- def teardown_method(self, method):
- for host in self.replicas:
- tasks.uninstall_replica(self.master, host)
- tasks.uninstall_client(host)
- result = self.master.run_command(
- ["ipa", "host-del", "--updatedns", host.hostname],
- raiseonerr=False)
- # Workaround for 5627
- if "host not found" in result.stderr_text:
- self.master.run_command(["ipa",
- "host-del",
- host.hostname], raiseonerr=False)
-
def test_kra_install_master(self):
result1 = tasks.install_kra(self.master,
first_instance=True,
@@ -43,6 +31,7 @@ class TestReplicaPromotionLevel0(ReplicaPromotionBase):
num_replicas = 1
domain_level = DOMAIN_LEVEL_0
+ @replicas_cleanup
def test_promotion_disabled(self):
"""
Testcase:
@@ -60,6 +49,7 @@ class TestReplicaPromotionLevel0(ReplicaPromotionBase):
'You must provide a file generated by ipa-replica-prepare'
' to create a replica when the domain is at level 0', 1)
+ @replicas_cleanup
def test_backup_restore(self):
"""
TestCase:
@@ -168,6 +158,7 @@ class TestReplicaPromotionLevel1(ReplicaPromotionBase):
num_replicas = 1
domain_level = DOMAIN_LEVEL_1
+ @replicas_cleanup
def test_replica_prepare_disabled(self):
replica = self.replicas[0]
args = ['ipa-replica-prepare',
--
1.8.3.1
From 587050889e6af68816a87299db0b6ed709daecc9 Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Fri, 18 Mar 2016 15:45:42 +0100
Subject: [PATCH] Added copyright info to replica promotion tests
---
ipatests/test_integration/test_replica_promotion.py | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/ipatests/test_integration/test_replica_promotion.py b/ipatests/test_integration/test_replica_promotion.py
index c45af72160e7e93e0f8d3c75e3173fca98907925..7945446dd575cc3dbadf23c0a90ab6aae4a339f7 100644
--- a/ipatests/test_integration/test_replica_promotion.py
+++ b/ipatests/test_integration/test_replica_promotion.py
@@ -1,3 +1,7 @@
+#
+# Copyright (C) 2016 FreeIPA Contributors see COPYING for license
+#
+
from ipatests.test_integration.base import IntegrationTest
from ipatests.test_integration import tasks
from ipatests.test_integration.test_caless import assert_error
--
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
