Re: [Freeipa-devel] [PATCHES] 0656-0673 Switch the test suite to pytest
On 11/21/2014 10:13 PM, Rob Crittenden wrote: Tomas Babej wrote: On 11/20/2014 10:12 AM, Petr Viktorin wrote: On 11/19/2014 01:11 PM, Tomas Babej wrote: On 11/14/2014 09:55 AM, Petr Viktorin wrote: On 10/29/2014 04:52 PM, Petr Viktorin wrote: On 10/29/2014 01:22 PM, Tomas Babej wrote: On 10/27/2014 04:38 PM, Petr Viktorin wrote: On 10/15/2014 02:58 PM, Petr Viktorin wrote: This almost completes the switch to pytest. There are two missing things: - the details of test results (--with-xunit) are not read correctly by Jenkins. I have a theory I'm investigating here. - the beakerlib integration is still not ready I'll not be available for the rest of the week so I'm sending this early, in case someone wants to take a look. I've updated (and rearranged) the patches after some more testing. Both points above are fixed. Individual plugins are broken out; some would be nice to even release independently of IPA. (There is some demand for the BeakerLib plugin; for that I'd only need to break the dependency on ipa_log_manager.) These depend on my patches 0656-0660. Thanks for this effort! Patch 0656: tests: Use PEP8-compliant setup/teardown method names There are some references to the old names in the test_ipapython and test_install: [tbabej@thinkpad7 ipatests]$ git grep setUpClass [tbabej@thinkpad7 ipatests]$ git grep tearDownClass [tbabej@thinkpad7 ipatests]$ git grep setUp test_install/test_updates.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): [tbabej@thinkpad7 ipatests]$ git grep tearDown test_install/test_updates.py:def tearDown(self): These are in unittest.testCase. It would be nice to convert those as well, but that's for a larger cleanup. Patch 0657: tests: Add configuration for pytest Shouldn't we rather change the class names? Ideally yes, but I don't think renaming most of our test classes would be worth the benefit. Patch 0658: ipatests.util.ClassChecker: Raise AttributeError in get_subcls ACK. Patch 0659: test_automount_plugin: Fix test ordering ACK. PATCH 0660: Use setup_class/teardown_class in Declarative tests --- a/ipatests/test_ipaserver/test_changepw.py +++ b/ipatests/test_ipaserver/test_changepw.py @@ -33,8 +33,7 @@ class test_changepw(XMLRPC_test, Unauthorized_HTTP_test): app_uri = '/ipa/session/change_password' -def setup(self): -super(test_changepw, self).setup() +def setup(cls): try: api.Command['user_add'](uid=testuser, givenname=u'Test', sn=u'User') api.Command['passwd'](testuser, password=u'old_password') @@ -43,12 +42,11 @@ def setup(self): 'Cannot set up test user: %s' % e ) -def teardown(self): +def teardown(cls): try: api.Command['user_del']([testuser]) except errors.NotFound: pass -super(test_changepw, self).teardown() The setup/teardown methods are not classmethods, so the name of the first argument should remain self. Oops, thanks for the catch! PATCH 0661: dogtag plugin: Don't use doctest syntax for non-doctest examples ACK. PATCH 0662: test_webui: Don't use __init__ for test classes I don't think the following change will work: -def __init__(self, driver=None, config=None): +def setup(self, driver=None, config=None): self.request_timeout = 30 self.driver = driver self.config = config if not config: self.load_config() +self.get_driver().maximize_window() + +def teardown(self): +self.driver.quit() def load_config(self): @@ -161,20 +165,6 @@ def load_config(self): if 'type' not in c: c['type'] = DEFAULT_TYPE -def setup(self): - -Test setup - -if not self.driver: -self.driver = self.get_driver() -self.driver.maximize_window() - -def teardown(self): - -Test clean up - -self.driver.quit() The setup(self) method used to set the self.driver attribute on the class instance, now nothing sets it. The setup method should do: def
Re: [Freeipa-devel] [PATCHES] 0656-0673 Switch the test suite to pytest
On 11/20/2014 10:12 AM, Petr Viktorin wrote: On 11/19/2014 01:11 PM, Tomas Babej wrote: On 11/14/2014 09:55 AM, Petr Viktorin wrote: On 10/29/2014 04:52 PM, Petr Viktorin wrote: On 10/29/2014 01:22 PM, Tomas Babej wrote: On 10/27/2014 04:38 PM, Petr Viktorin wrote: On 10/15/2014 02:58 PM, Petr Viktorin wrote: This almost completes the switch to pytest. There are two missing things: - the details of test results (--with-xunit) are not read correctly by Jenkins. I have a theory I'm investigating here. - the beakerlib integration is still not ready I'll not be available for the rest of the week so I'm sending this early, in case someone wants to take a look. I've updated (and rearranged) the patches after some more testing. Both points above are fixed. Individual plugins are broken out; some would be nice to even release independently of IPA. (There is some demand for the BeakerLib plugin; for that I'd only need to break the dependency on ipa_log_manager.) These depend on my patches 0656-0660. Thanks for this effort! Patch 0656: tests: Use PEP8-compliant setup/teardown method names There are some references to the old names in the test_ipapython and test_install: [tbabej@thinkpad7 ipatests]$ git grep setUpClass [tbabej@thinkpad7 ipatests]$ git grep tearDownClass [tbabej@thinkpad7 ipatests]$ git grep setUp test_install/test_updates.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): [tbabej@thinkpad7 ipatests]$ git grep tearDown test_install/test_updates.py:def tearDown(self): These are in unittest.testCase. It would be nice to convert those as well, but that's for a larger cleanup. Patch 0657: tests: Add configuration for pytest Shouldn't we rather change the class names? Ideally yes, but I don't think renaming most of our test classes would be worth the benefit. Patch 0658: ipatests.util.ClassChecker: Raise AttributeError in get_subcls ACK. Patch 0659: test_automount_plugin: Fix test ordering ACK. PATCH 0660: Use setup_class/teardown_class in Declarative tests --- a/ipatests/test_ipaserver/test_changepw.py +++ b/ipatests/test_ipaserver/test_changepw.py @@ -33,8 +33,7 @@ class test_changepw(XMLRPC_test, Unauthorized_HTTP_test): app_uri = '/ipa/session/change_password' -def setup(self): -super(test_changepw, self).setup() +def setup(cls): try: api.Command['user_add'](uid=testuser, givenname=u'Test', sn=u'User') api.Command['passwd'](testuser, password=u'old_password') @@ -43,12 +42,11 @@ def setup(self): 'Cannot set up test user: %s' % e ) -def teardown(self): +def teardown(cls): try: api.Command['user_del']([testuser]) except errors.NotFound: pass -super(test_changepw, self).teardown() The setup/teardown methods are not classmethods, so the name of the first argument should remain self. Oops, thanks for the catch! PATCH 0661: dogtag plugin: Don't use doctest syntax for non-doctest examples ACK. PATCH 0662: test_webui: Don't use __init__ for test classes I don't think the following change will work: -def __init__(self, driver=None, config=None): +def setup(self, driver=None, config=None): self.request_timeout = 30 self.driver = driver self.config = config if not config: self.load_config() +self.get_driver().maximize_window() + +def teardown(self): +self.driver.quit() def load_config(self): @@ -161,20 +165,6 @@ def load_config(self): if 'type' not in c: c['type'] = DEFAULT_TYPE -def setup(self): - -Test setup - -if not self.driver: -self.driver = self.get_driver() -self.driver.maximize_window() - -def teardown(self): - -Test clean up - -self.driver.quit() The setup(self) method used to set the self.driver attribute on the class instance, now nothing sets it. The setup method should do: def setup(self, driver=None, config=None):
Re: [Freeipa-devel] [PATCHES] 0656-0673 Switch the test suite to pytest
Tomas Babej wrote: On 11/20/2014 10:12 AM, Petr Viktorin wrote: On 11/19/2014 01:11 PM, Tomas Babej wrote: On 11/14/2014 09:55 AM, Petr Viktorin wrote: On 10/29/2014 04:52 PM, Petr Viktorin wrote: On 10/29/2014 01:22 PM, Tomas Babej wrote: On 10/27/2014 04:38 PM, Petr Viktorin wrote: On 10/15/2014 02:58 PM, Petr Viktorin wrote: This almost completes the switch to pytest. There are two missing things: - the details of test results (--with-xunit) are not read correctly by Jenkins. I have a theory I'm investigating here. - the beakerlib integration is still not ready I'll not be available for the rest of the week so I'm sending this early, in case someone wants to take a look. I've updated (and rearranged) the patches after some more testing. Both points above are fixed. Individual plugins are broken out; some would be nice to even release independently of IPA. (There is some demand for the BeakerLib plugin; for that I'd only need to break the dependency on ipa_log_manager.) These depend on my patches 0656-0660. Thanks for this effort! Patch 0656: tests: Use PEP8-compliant setup/teardown method names There are some references to the old names in the test_ipapython and test_install: [tbabej@thinkpad7 ipatests]$ git grep setUpClass [tbabej@thinkpad7 ipatests]$ git grep tearDownClass [tbabej@thinkpad7 ipatests]$ git grep setUp test_install/test_updates.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): [tbabej@thinkpad7 ipatests]$ git grep tearDown test_install/test_updates.py:def tearDown(self): These are in unittest.testCase. It would be nice to convert those as well, but that's for a larger cleanup. Patch 0657: tests: Add configuration for pytest Shouldn't we rather change the class names? Ideally yes, but I don't think renaming most of our test classes would be worth the benefit. Patch 0658: ipatests.util.ClassChecker: Raise AttributeError in get_subcls ACK. Patch 0659: test_automount_plugin: Fix test ordering ACK. PATCH 0660: Use setup_class/teardown_class in Declarative tests --- a/ipatests/test_ipaserver/test_changepw.py +++ b/ipatests/test_ipaserver/test_changepw.py @@ -33,8 +33,7 @@ class test_changepw(XMLRPC_test, Unauthorized_HTTP_test): app_uri = '/ipa/session/change_password' -def setup(self): -super(test_changepw, self).setup() +def setup(cls): try: api.Command['user_add'](uid=testuser, givenname=u'Test', sn=u'User') api.Command['passwd'](testuser, password=u'old_password') @@ -43,12 +42,11 @@ def setup(self): 'Cannot set up test user: %s' % e ) -def teardown(self): +def teardown(cls): try: api.Command['user_del']([testuser]) except errors.NotFound: pass -super(test_changepw, self).teardown() The setup/teardown methods are not classmethods, so the name of the first argument should remain self. Oops, thanks for the catch! PATCH 0661: dogtag plugin: Don't use doctest syntax for non-doctest examples ACK. PATCH 0662: test_webui: Don't use __init__ for test classes I don't think the following change will work: -def __init__(self, driver=None, config=None): +def setup(self, driver=None, config=None): self.request_timeout = 30 self.driver = driver self.config = config if not config: self.load_config() +self.get_driver().maximize_window() + +def teardown(self): +self.driver.quit() def load_config(self): @@ -161,20 +165,6 @@ def load_config(self): if 'type' not in c: c['type'] = DEFAULT_TYPE -def setup(self): - -Test setup - -if not self.driver: -self.driver = self.get_driver() -self.driver.maximize_window() - -def teardown(self): - -Test clean up - -self.driver.quit() The setup(self) method used to set the self.driver attribute on the class instance, now nothing sets it. The setup method should do: def setup(self,
Re: [Freeipa-devel] [PATCHES] 0656-0673 Switch the test suite to pytest
On 11/14/2014 09:55 AM, Petr Viktorin wrote: On 10/29/2014 04:52 PM, Petr Viktorin wrote: On 10/29/2014 01:22 PM, Tomas Babej wrote: On 10/27/2014 04:38 PM, Petr Viktorin wrote: On 10/15/2014 02:58 PM, Petr Viktorin wrote: This almost completes the switch to pytest. There are two missing things: - the details of test results (--with-xunit) are not read correctly by Jenkins. I have a theory I'm investigating here. - the beakerlib integration is still not ready I'll not be available for the rest of the week so I'm sending this early, in case someone wants to take a look. I've updated (and rearranged) the patches after some more testing. Both points above are fixed. Individual plugins are broken out; some would be nice to even release independently of IPA. (There is some demand for the BeakerLib plugin; for that I'd only need to break the dependency on ipa_log_manager.) These depend on my patches 0656-0660. Thanks for this effort! Patch 0656: tests: Use PEP8-compliant setup/teardown method names There are some references to the old names in the test_ipapython and test_install: [tbabej@thinkpad7 ipatests]$ git grep setUpClass [tbabej@thinkpad7 ipatests]$ git grep tearDownClass [tbabej@thinkpad7 ipatests]$ git grep setUp test_install/test_updates.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): [tbabej@thinkpad7 ipatests]$ git grep tearDown test_install/test_updates.py:def tearDown(self): These are in unittest.testCase. It would be nice to convert those as well, but that's for a larger cleanup. Patch 0657: tests: Add configuration for pytest Shouldn't we rather change the class names? Ideally yes, but I don't think renaming most of our test classes would be worth the benefit. Patch 0658: ipatests.util.ClassChecker: Raise AttributeError in get_subcls ACK. Patch 0659: test_automount_plugin: Fix test ordering ACK. PATCH 0660: Use setup_class/teardown_class in Declarative tests --- a/ipatests/test_ipaserver/test_changepw.py +++ b/ipatests/test_ipaserver/test_changepw.py @@ -33,8 +33,7 @@ class test_changepw(XMLRPC_test, Unauthorized_HTTP_test): app_uri = '/ipa/session/change_password' -def setup(self): -super(test_changepw, self).setup() +def setup(cls): try: api.Command['user_add'](uid=testuser, givenname=u'Test', sn=u'User') api.Command['passwd'](testuser, password=u'old_password') @@ -43,12 +42,11 @@ def setup(self): 'Cannot set up test user: %s' % e ) -def teardown(self): +def teardown(cls): try: api.Command['user_del']([testuser]) except errors.NotFound: pass -super(test_changepw, self).teardown() The setup/teardown methods are not classmethods, so the name of the first argument should remain self. Oops, thanks for the catch! PATCH 0661: dogtag plugin: Don't use doctest syntax for non-doctest examples ACK. PATCH 0662: test_webui: Don't use __init__ for test classes I don't think the following change will work: -def __init__(self, driver=None, config=None): +def setup(self, driver=None, config=None): self.request_timeout = 30 self.driver = driver self.config = config if not config: self.load_config() +self.get_driver().maximize_window() + +def teardown(self): +self.driver.quit() def load_config(self): @@ -161,20 +165,6 @@ def load_config(self): if 'type' not in c: c['type'] = DEFAULT_TYPE -def setup(self): - -Test setup - -if not self.driver: -self.driver = self.get_driver() -self.driver.maximize_window() - -def teardown(self): - -Test clean up - -self.driver.quit() The setup(self) method used to set the self.driver attribute on the class instance, now nothing sets it. The setup method should do: def setup(self, driver=None, config=None): self.request_timeout = 30 self.driver = driver self.config = config if not config: self.load_config()
Re: [Freeipa-devel] [PATCHES] 0656-0673 Switch the test suite to pytest
On 10/29/2014 04:52 PM, Petr Viktorin wrote: On 10/29/2014 01:22 PM, Tomas Babej wrote: On 10/27/2014 04:38 PM, Petr Viktorin wrote: On 10/15/2014 02:58 PM, Petr Viktorin wrote: This almost completes the switch to pytest. There are two missing things: - the details of test results (--with-xunit) are not read correctly by Jenkins. I have a theory I'm investigating here. - the beakerlib integration is still not ready I'll not be available for the rest of the week so I'm sending this early, in case someone wants to take a look. I've updated (and rearranged) the patches after some more testing. Both points above are fixed. Individual plugins are broken out; some would be nice to even release independently of IPA. (There is some demand for the BeakerLib plugin; for that I'd only need to break the dependency on ipa_log_manager.) These depend on my patches 0656-0660. Thanks for this effort! Patch 0656: tests: Use PEP8-compliant setup/teardown method names There are some references to the old names in the test_ipapython and test_install: [tbabej@thinkpad7 ipatests]$ git grep setUpClass [tbabej@thinkpad7 ipatests]$ git grep tearDownClass [tbabej@thinkpad7 ipatests]$ git grep setUp test_install/test_updates.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): [tbabej@thinkpad7 ipatests]$ git grep tearDown test_install/test_updates.py:def tearDown(self): These are in unittest.testCase. It would be nice to convert those as well, but that's for a larger cleanup. Patch 0657: tests: Add configuration for pytest Shouldn't we rather change the class names? Ideally yes, but I don't think renaming most of our test classes would be worth the benefit. Patch 0658: ipatests.util.ClassChecker: Raise AttributeError in get_subcls ACK. Patch 0659: test_automount_plugin: Fix test ordering ACK. PATCH 0660: Use setup_class/teardown_class in Declarative tests --- a/ipatests/test_ipaserver/test_changepw.py +++ b/ipatests/test_ipaserver/test_changepw.py @@ -33,8 +33,7 @@ class test_changepw(XMLRPC_test, Unauthorized_HTTP_test): app_uri = '/ipa/session/change_password' -def setup(self): -super(test_changepw, self).setup() +def setup(cls): try: api.Command['user_add'](uid=testuser, givenname=u'Test', sn=u'User') api.Command['passwd'](testuser, password=u'old_password') @@ -43,12 +42,11 @@ def setup(self): 'Cannot set up test user: %s' % e ) -def teardown(self): +def teardown(cls): try: api.Command['user_del']([testuser]) except errors.NotFound: pass -super(test_changepw, self).teardown() The setup/teardown methods are not classmethods, so the name of the first argument should remain self. Oops, thanks for the catch! PATCH 0661: dogtag plugin: Don't use doctest syntax for non-doctest examples ACK. PATCH 0662: test_webui: Don't use __init__ for test classes I don't think the following change will work: -def __init__(self, driver=None, config=None): +def setup(self, driver=None, config=None): self.request_timeout = 30 self.driver = driver self.config = config if not config: self.load_config() +self.get_driver().maximize_window() + +def teardown(self): +self.driver.quit() def load_config(self): @@ -161,20 +165,6 @@ def load_config(self): if 'type' not in c: c['type'] = DEFAULT_TYPE -def setup(self): - -Test setup - -if not self.driver: -self.driver = self.get_driver() -self.driver.maximize_window() - -def teardown(self): - -Test clean up - -self.driver.quit() The setup(self) method used to set the self.driver attribute on the class instance, now nothing sets it. The setup method should do: def setup(self, driver=None, config=None): self.request_timeout = 30 self.driver = driver self.config = config if not config: self.load_config() if not self.driver: self.driver = self.get_driver() self.driver.maximize_window() However, I would prefer
