Hi Martin,

On 01/29/2016 10:33 AM, Martin Basti wrote:
> 
> 
> On 21.01.2016 12:59, Oleg Fayans wrote:
>>
>>
> Hello,
> 
> 1)
> I'm not sure if it is bug or it is related to this patch, but there is
> missing symetry in test_caless, I see there apply fixes but not
> unapply_fixes

test_caless is currently worked at, the patch containing a lot of
changes is on it's way, so the current version of this file can be ignored.

> 
> ipatests/ipa-test-task:378:            tasks.unapply_fixes(host)
> ipatests/test_integration/tasks.py:201:def unapply_fixes(host):
> ipatests/test_integration/tasks.py:646:    unapply_fixes(host)
> ipatests/test_integration/tasks.py:654:    unapply_fixes(host)
> ipatests/test_integration/tasks.py:997:    unapply_fixes(replica)
> ipatests/test_integration/test_legacy_clients.py:373:           
> tasks.unapply_fixes(cls.legacy_client)
> 
> 
> ipatests/test_integration/tasks.py:91:def apply_common_fixes(host,
> fix_resolv=True):
> ipatests/test_integration/tasks.py:269:    apply_common_fixes(host,
> fix_resolv=False)
> ipatests/test_integration/tasks.py:320:    apply_common_fixes(replica)
> ipatests/test_integration/tasks.py:386:    apply_common_fixes(client)
> ipatests/test_integration/test_caless.py:101:           
> tasks.apply_common_fixes(host)
> ipatests/test_integration/test_legacy_clients.py:361:       
> tasks.apply_common_fixes(cls.legacy_client)


> 
> 2)
> IMO unapply_fixes is called twice for replica uninstall
> 
>      uninstall_master(replica)
> +    unapply_fixes(replica)
> 
> uninstall_master calls unapply_fixes() too

right. Fixed

> 
> 3)
> what is purpose of prepare_host? I saw it used only in
> test_forced_client_reenrollment.py

The purpose is to create a testing folder that contains the host
configuration file env.sh and system file backups

The problem was that unapply_common_fixes removed the testing directory.
If you then wanted to re-install master within the same test execution,
it would fail at the attempt to backup system files (as part of
apply_common_fixes, see backup_file method in tasks.py - line that does
mkdir_recursive)

> Which test was failing?

Test_replica_promotion.py (not merged yet)
And basically any test that includes more than one
installation-uninstallation cycle would be affected

> 
> 4)
> in test_forced_client_reenrollment.py there is direct call of
> prepare_host, but this is also install_client that calls prepare_host
> too (added by your patch)

We can remove prepare_host call from test_client_reenrollment. Do you
think I should do it in the same patch, or in a separate one?

> 
> Martin

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 6965ed7788d0c7382d6036dd87da12aac1a5833d Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Fri, 29 Jan 2016 13:19:24 +0100
Subject: [PATCH] Enabled recreation of test directory in apply_common_fixes
 function

Without it any test comprized of more than one cycle of installing-uninstalling
of ipa would fail due to the fact that test folder on the remote machine gets
deleted during ipa uninstallation.

Also removed duplicate call of apply_common fixes and added unapply_fixes to
uninstall_replica
---
 ipatests/test_integration/tasks.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index a1d6cb88a1bfe2ac8c2fae9d758da726a5536ce2..27d2449ec71e0bf55a576cc495175a5ae41da1d6 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -94,6 +94,7 @@ def apply_common_fixes(host, fix_resolv=True):
     modify_nm_resolv_conf_settings(host)
     if fix_resolv:
         fix_resolv_conf(host)
+    prepare_host(host)
 
 
 def backup_file(host, filename):
@@ -348,7 +349,6 @@ def install_replica(master, replica, setup_ca=True, setup_dns=False,
             '--forwarder', replica.config.dns_forwarder
         ])
     if domainlevel(master) == DOMAIN_LEVEL_0:
-        apply_common_fixes(replica)
         # prepare the replica file on master and put it to replica, AKA "old way"
         replica_prepare(master, replica)
         replica_filename = get_replica_filename(replica)
-- 
2.4.3

-- 
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

Reply via email to