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

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.

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

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

--
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org



--
PetrĀ³

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to