Re: [Freeipa-devel] [PATCH 0025] proper client host setup/teardown in forced client reenrollment integration test suite

2015-04-14 Thread Petr Vobornik

On 04/14/2015 03:54 PM, Milan Kubik wrote:



On 04/14/2015 03:20 PM, Milan Kubik wrote:



On 03/31/2015 10:42 AM, Martin Babinsky wrote:

During the investigation of
https://fedorahosted.org/freeipa/ticket/4614 I discovered a bug (?)
in forced client reenrollment integration test.

During test scenario, master and replica are setup correctly at the
beginning of the test, but the client is never setup resulting in a
couple of tracebacks.

After some investigation I realized that the setUp/tearDown methods
are actually never called because they are supposed to be inherited
from unittest.TestCase. However, IntegrationTest no longer inherits
from this class, hence the bug.

I have tried to fix this by adding a fixture which runs client
fixup/teardown and doing some other small modifications. Tests now
work as expected, but I need a review from QE guys or someone
well-versed in pytest framework.

TL;DR: I think I have fixed a bug in integration test but I need
someone to review the fix because I may not know what I'm doing.


Hello,

please fix the pep8 complaints. Otherwise looks good to me.

Thanks,
Milan


Taking back request on pep8, this is not related to the patch introduced
code.

ACK.

Milan



Pushed to master: c8fae594df474669416b96b8033528332daf9b37
--
Petr Vobornik

--
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] [PATCH 0025] proper client host setup/teardown in forced client reenrollment integration test suite

2015-04-14 Thread Milan Kubik



On 04/14/2015 03:20 PM, Milan Kubik wrote:



On 03/31/2015 10:42 AM, Martin Babinsky wrote:
During the investigation of 
https://fedorahosted.org/freeipa/ticket/4614 I discovered a bug (?) 
in forced client reenrollment integration test.


During test scenario, master and replica are setup correctly at the 
beginning of the test, but the client is never setup resulting in a 
couple of tracebacks.


After some investigation I realized that the setUp/tearDown methods 
are actually never called because they are supposed to be inherited 
from unittest.TestCase. However, IntegrationTest no longer inherits 
from this class, hence the bug.


I have tried to fix this by adding a fixture which runs client 
fixup/teardown and doing some other small modifications. Tests now 
work as expected, but I need a review from QE guys or someone 
well-versed in pytest framework.


TL;DR: I think I have fixed a bug in integration test but I need 
someone to review the fix because I may not know what I'm doing.



Hello,

please fix the pep8 complaints. Otherwise looks good to me.

Thanks,
Milan

Taking back request on pep8, this is not related to the patch introduced 
code.


ACK.

Milan

--
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] [PATCH 0025] proper client host setup/teardown in forced client reenrollment integration test suite

2015-04-14 Thread Milan Kubik



On 03/31/2015 10:42 AM, Martin Babinsky wrote:
During the investigation of 
https://fedorahosted.org/freeipa/ticket/4614 I discovered a bug (?) in 
forced client reenrollment integration test.


During test scenario, master and replica are setup correctly at the 
beginning of the test, but the client is never setup resulting in a 
couple of tracebacks.


After some investigation I realized that the setUp/tearDown methods 
are actually never called because they are supposed to be inherited 
from unittest.TestCase. However, IntegrationTest no longer inherits 
from this class, hence the bug.


I have tried to fix this by adding a fixture which runs client 
fixup/teardown and doing some other small modifications. Tests now 
work as expected, but I need a review from QE guys or someone 
well-versed in pytest framework.


TL;DR: I think I have fixed a bug in integration test but I need 
someone to review the fix because I may not know what I'm doing.



Hello,

please fix the pep8 complaints. Otherwise looks good to me.

Thanks,
Milan

--
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] [PATCH 0025] proper client host setup/teardown in forced client reenrollment integration test suite

2015-03-31 Thread Martin Babinsky
During the investigation of https://fedorahosted.org/freeipa/ticket/4614 
I discovered a bug (?) in forced client reenrollment integration test.


During test scenario, master and replica are setup correctly at the 
beginning of the test, but the client is never setup resulting in a 
couple of tracebacks.


After some investigation I realized that the setUp/tearDown methods are 
actually never called because they are supposed to be inherited from 
unittest.TestCase. However, IntegrationTest no longer inherits from this 
class, hence the bug.


I have tried to fix this by adding a fixture which runs client 
fixup/teardown and doing some other small modifications. Tests now work 
as expected, but I need a review from QE guys or someone well-versed in 
pytest framework.


TL;DR: I think I have fixed a bug in integration test but I need someone 
to review the fix because I may not know what I'm doing.


--
Martin^3 Babinsky
From 9cb41407f2db6a353969977016c326fe76169bc9 Mon Sep 17 00:00:00 2001
From: Martin Babinsky mbabi...@redhat.com
Date: Tue, 31 Mar 2015 09:33:53 +0200
Subject: [PATCH] proper client host setup/teardown in forced client
 reenrollment integration test suite

Replace setUp()/tearDown() methods with a pytest.fixture for proper client
setup/teardown during test_forced_client_reenrollment

---
 .../test_forced_client_reenrollment.py | 43 --
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/ipatests/test_integration/test_forced_client_reenrollment.py b/ipatests/test_integration/test_forced_client_reenrollment.py
index ed9800d49f77f6f292f59c22509a9f568041200b..e1edff9b7f2d535a341d1e8ca55917012943818e 100644
--- a/ipatests/test_integration/test_forced_client_reenrollment.py
+++ b/ipatests/test_integration/test_forced_client_reenrollment.py
@@ -19,6 +19,7 @@
 import os
 import subprocess
 from ipaplatform.paths import paths
+import pytest
 
 from ipatests.test_integration.base import IntegrationTest
 from ipatests.test_integration import tasks
@@ -44,15 +45,7 @@ class TestForcedClientReenrollment(IntegrationTest):
 'krb5.keytab'
 )
 
-def setUp(self):
-tasks.prepare_host(self.clients[0])
-tasks.install_client(self.master, self.clients[0])
-
-def tearDown(self):
-tasks.uninstall_client(self.clients[0])
-self.delete_client_host_entry()
-
-def test_reenroll_with_force_join(self):
+def test_reenroll_with_force_join(self, client):
 
 Client re-enrollment using admin credentials (--force-join)
 
@@ -63,7 +56,7 @@ class TestForcedClientReenrollment(IntegrationTest):
 sshfp_record_post = self.get_sshfp_record()
 assert sshfp_record_pre == sshfp_record_post
 
-def test_reenroll_with_keytab(self):
+def test_reenroll_with_keytab(self, client):
 
 Client re-enrollment using keytab
 
@@ -76,7 +69,7 @@ class TestForcedClientReenrollment(IntegrationTest):
 sshfp_record_post = self.get_sshfp_record()
 assert sshfp_record_pre == sshfp_record_post
 
-def test_reenroll_with_both_force_join_and_keytab(self):
+def test_reenroll_with_both_force_join_and_keytab(self, client):
 
 Client re-enrollment using both --force-join and --keytab options
 
@@ -89,7 +82,7 @@ class TestForcedClientReenrollment(IntegrationTest):
 sshfp_record_post = self.get_sshfp_record()
 assert sshfp_record_pre == sshfp_record_post
 
-def test_reenroll_to_replica(self):
+def test_reenroll_to_replica(self, client):
 
 Client re-enrollment using keytab, to a replica
 
@@ -102,7 +95,7 @@ class TestForcedClientReenrollment(IntegrationTest):
 sshfp_record_post = self.get_sshfp_record()
 assert sshfp_record_pre == sshfp_record_post
 
-def test_try_to_reenroll_with_disabled_host(self):
+def test_try_to_reenroll_with_disabled_host(self, client):
 
 Client re-enrollment using keytab, with disabled host
 
@@ -113,7 +106,7 @@ class TestForcedClientReenrollment(IntegrationTest):
 self.restore_keytab()
 self.reenroll_client(keytab=self.BACKUP_KEYTAB, expect_fail=True)
 
-def test_try_to_reenroll_with_uninstalled_host(self):
+def test_try_to_reenroll_with_uninstalled_host(self, client):
 
 Client re-enrollment using keytab, with uninstalled host
 
@@ -124,7 +117,7 @@ class TestForcedClientReenrollment(IntegrationTest):
 self.restore_keytab()
 self.reenroll_client(keytab=self.BACKUP_KEYTAB, expect_fail=True)
 
-def test_try_to_reenroll_with_deleted_host(self):
+def test_try_to_reenroll_with_deleted_host(self, client):
 
 Client re-enrollment using keytab, with deleted host
 
@@ -135,7 +128,7 @@ class TestForcedClientReenrollment(IntegrationTest):
 self.restore_keytab()
 self.reenroll_client(keytab=self.BACKUP_KEYTAB, 

Re: [Freeipa-devel] [PATCH 0025] proper client host setup/teardown in forced client reenrollment integration test suite

2015-03-31 Thread Petr Viktorin

On 03/31/2015 10:42 AM, Martin Babinsky wrote:

During the investigation of https://fedorahosted.org/freeipa/ticket/4614
I discovered a bug (?) in forced client reenrollment integration test.

During test scenario, master and replica are setup correctly at the
beginning of the test, but the client is never setup resulting in a
couple of tracebacks.

After some investigation I realized that the setUp/tearDown methods are
actually never called because they are supposed to be inherited from
unittest.TestCase. However, IntegrationTest no longer inherits from this
class, hence the bug.

I have tried to fix this by adding a fixture which runs client
fixup/teardown and doing some other small modifications. Tests now work
as expected, but I need a review from QE guys or someone well-versed in
pytest framework.


LGTM, from a quick glance.


--
Petr Viktorin

--
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] [PATCH 0025] proper client host setup/teardown in forced client reenrollment integration test suite

2015-03-31 Thread Martin Babinsky

On 03/31/2015 12:06 PM, Petr Viktorin wrote:

On 03/31/2015 10:42 AM, Martin Babinsky wrote:

During the investigation of https://fedorahosted.org/freeipa/ticket/4614
I discovered a bug (?) in forced client reenrollment integration test.

During test scenario, master and replica are setup correctly at the
beginning of the test, but the client is never setup resulting in a
couple of tracebacks.

After some investigation I realized that the setUp/tearDown methods are
actually never called because they are supposed to be inherited from
unittest.TestCase. However, IntegrationTest no longer inherits from this
class, hence the bug.

I have tried to fix this by adding a fixture which runs client
fixup/teardown and doing some other small modifications. Tests now work
as expected, but I need a review from QE guys or someone well-versed in
pytest framework.


LGTM, from a quick glance.



Thank Petr, anyone else has some opinion on this?

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