[Freeipa-devel] [PATCHES] 0656-0673 Switch the test suite to pytest
Petr Viktorin
pviktori at redhat.com
Thu Nov 20 09:12:01 UTC 2014
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 at thinkpad7 ipatests]$ git grep setUpClass
>>>> [tbabej at thinkpad7 ipatests]$ git grep tearDownClass
>>>> [tbabej at 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 at 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 having self.driver as a cached property.
>>>
>>> Well, ideally these would be fixtures.
>>>
>>> Thanks for the catch.
>>>
>>>> #### PATCH 0663: test_ipapython: Use functions instead of classes in
>>>> test generators
>>>>
>>>> ACK.
>>>>
>>>> #### PATCH 0664: Configure pytest to run doctests
>>>>
>>>> ACK.
>>>>
>>>> #### PATCH 0665: Declarative tests: Move cleanup to
>>>> setup_class/teardown_class
>>>>
>>>> You should also remove the now redundant cleanup_generate method.
>>>
>>> Right, removed.
>>>
>>>> Also: Do we really want to print out error.NotFound or
>>>> errors.EmptyModList exceptions produced by cleanup commands?
>>>
>>> It's just one line, might help debugging. It should only show up on
>>> failures.
>>>
>>>> #### PATCH 0666: Declarative tests: Switch to pytest
>>>>
>>>> Nitpitck: declarative.py has wrong year in the copyright header.
>>>> Otherwise ACK.
>>>
>>> Fixed.
>>>
>>>> #### PATCH 0667: Integration tests: Port the ordering plugin to pytest
>>>>
>>>> Specfile change belongs to the previous patch, but I doubt these would
>>>> be separated, so not worth spending the time IMHO.
>>>
>>> Also, the previous patch doesn't actually import pytest. But a lot of
>>> these patches break tests by themselves, so they shouldn't be separated.
>>>
>>>> Why does TestIPACommands have ordered decorator applied? It is applied
>>>> on IntegrationTest directly, from which CALessBase inherits from. I
>>>> don't think it's necessary (unless I'm missing something), so we should
>>>> rather remove the occurence of the decorator in the test_caless.py than
>>>> fixing the import.
>>>
>>> Actually, I think only only the classes that actually need it should
>>> have the decorator.
>>> Again, cleanup for a future time.
>>>
>>>> #### PATCH 0668: Switch make-test to pytest
>>>>
>>>> Nice simplification. However, make-test does not work for me:
>>>>
>>>> I'm getting tons of errors (during the collection) along the lines of:
>>>>
>>>> ________________ ERROR collecting ipatests/test_xmlrpc/testcert.py
>>>> _________________
>>>> /usr/lib/python2.7/site-packages/py/_path/local.py:661: in
>>>> pyimport
>>>> raise self.ImportMismatchError(modname, modfile, self)
>>>> E ImportMismatchError: ('ipatests.test_xmlrpc.testcert',
>>>> '/usr/lib/python2.7/site-packages/ipatests/test_xmlrpc/testcert.py',
>>>> local('/ipadev/freeipa/ipatests/test_xmlrpc/testcert.py'))
>>>> _______________ ERROR collecting
>>>> ipatests/test_xmlrpc/xmlrpc_test.py _______________
>>>> /usr/lib/python2.7/site-packages/py/_path/local.py:661: in
>>>> pyimport
>>>> raise self.ImportMismatchError(modname, modfile, self)
>>>> E ImportMismatchError: ('ipatests.test_xmlrpc.xmlrpc_test',
>>>> '/usr/lib/python2.7/site-packages/ipatests/test_xmlrpc/xmlrpc_test.py',
>>>> local('/ipadev/freeipa/ipatests/test_xmlrpc/xmlrpc_test.py'))
>>>> _______________ ERROR collecting
>>>> ipatests/test_xmlrpc/xmlrpc_test.py _______________
>>>> import file mismatch:
>>>> imported module 'ipatests.test_xmlrpc.xmlrpc_test' has this
>>>> __file__ attribute:
>>>> /usr/lib/python2.7/site-packages/ipatests/test_xmlrpc/xmlrpc_test.py
>>>> which is not the same as the test file we want to collect:
>>>> /ipadev/freeipa/ipatests/test_xmlrpc/xmlrpc_test.py
>>>> HINT: remove __pycache__ / .pyc files and/or use a unique basename
>>>> for your test file modules
>>>> ================ 163 passed, 7 skipped, 679 error in 48.14 seconds
>>>> ==============
>>>>
>>>>
>>>> I am running this as ./make-test from the directory containing the
>>>> freeipa sources. The ipa-run-tests command seems to work fine though.
>>>
>>> Hm. Doesn't happen for me. Did you follow the hint (git clean -fxd)?
>>> I've added . to PYTHONPATH, that may help (and it should also help with
>>> plugin loading in some cases).
>>>
>>>> #### PATCH 0669: Add local pytest plugin for --with-xunit and
>>>> --logging-level
>>>>
>>>> ACK.
>>>>
>>>> #### PATCH 0670: Switch ipa-run-tests to pytest
>>>>
>>>> ACK.
>>>>
>>>> #### PATCH 0671: Switch integration testing config to a fixture
>>>>
>>>> ACK.
>>>>
>>>> #### PATCH 0672: Integration tests: Port the BeakerLib plugin and log
>>>> collection to pytest
>>>>
>>>> I didn't see any glitches, but I haven't tested this one out yet
>>>> functionally.
>>>>
>>>> ### General notes
>>>>
>>>> I did two IPA master installs today, one with your patches applied.
>>>> There were significant differences in the number of successfully
>>>> passing
>>>> tests:
>>>>
>>>> nose: FAILED (SKIP=60, errors=46, failures=14)
>>>> pytest: ========= 42 failed, 1830 passed, 422 skipped, 340 error in
>>>> 860.47 seconds =========
>>>>
>>>> Did you not encounter this in your testing? I will look more deeply
>>>> into
>>>> the failures themselves.
>>>
>>> I did, but it was always some stupid mistake :)
>>> Do you have ~/.ipa set up, and $MASTER undefined?
>>>
>>> The integration test did fail for me now when they were not configured.
>>> I added an additional patch to fix that.
>>
>> Ping, could you look at the updated patches?
>>
>>
>
> Sure, but the patchset requires a rebase.
>
Rebased onto master.
--
Petr³
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0656.4-tests-Use-PEP8-compliant-setup-teardown-method-names.patch
Type: text/x-patch
Size: 20304 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0657.4-tests-Add-configuration-for-pytest.patch
Type: text/x-patch
Size: 781 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0658.4-ipatests.util.ClassChecker-Raise-AttributeError-in-g.patch
Type: text/x-patch
Size: 878 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0659.4-test_automount_plugin-Fix-test-ordering.patch
Type: text/x-patch
Size: 1605 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0660.4-Use-setup_class-teardown_class-in-Declarative-tests.patch
Type: text/x-patch
Size: 4907 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0661.4-dogtag-plugin-Don-t-use-doctest-syntax-for-non-docte.patch
Type: text/x-patch
Size: 2523 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0662.4-test_webui-Don-t-use-__init__-for-test-classes.patch
Type: text/x-patch
Size: 6780 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0663.4-test_ipapython-Use-functions-instead-of-classes-in-t.patch
Type: text/x-patch
Size: 3192 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0664.4-Configure-pytest-to-run-doctests.patch
Type: text/x-patch
Size: 3135 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment-0008.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0665.4-Declarative-tests-Move-cleanup-to-setup_class-teardo.patch
Type: text/x-patch
Size: 2829 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment-0009.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0666.4-Declarative-tests-Switch-to-pytest.patch
Type: text/x-patch
Size: 5130 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment-0010.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0667.4-Integration-tests-Port-the-ordering-plugin-to-pytest.patch
Type: text/x-patch
Size: 12864 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment-0011.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0668.4-Switch-make-test-to-pytest.patch
Type: text/x-patch
Size: 2058 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment-0012.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0669.4-Add-local-pytest-plugin-for-with-xunit-and-logging-l.patch
Type: text/x-patch
Size: 4507 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment-0013.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0670.4-Switch-ipa-run-tests-to-pytest.patch
Type: text/x-patch
Size: 3002 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment-0014.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0671.4-Switch-integration-testing-config-to-a-fixture.patch
Type: text/x-patch
Size: 8104 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment-0015.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0672.4-Integration-tests-Port-the-BeakerLib-plugin-and-log-.patch
Type: text/x-patch
Size: 30085 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment-0016.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0673.4-test_integration-Adjust-tests-for-pytest.patch
Type: text/x-patch
Size: 5933 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141120/c669e3ac/attachment-0017.bin>
More information about the Freeipa-devel
mailing list