Issue #7989: Pytest4.2+ errors - freeipa

freeipa

#7989 Pytest4.2+ errors

Closed: fixed Feb 12, 2020 by abbra. Opened Jun 21, 2019 by slev.

I run the IPA test suite ( base + xmlrpc ) against the latest pytest (by now 4.6.3).
Build is available here ( https://dev.azure.com/slev0400/slev/_build/results?buildId=161 )
There are many errors ( logs are attached ):

testsuite errors="41" failures="9" name="pytest" skipped="13" tests="2341"

Though the same test suite passes well against pytest 3.9.3).

Version bisecting points me to https://github.com/pytest-dev/pytest/commit/0f918b1a9dd14a2d046d19be6ec239e8e2df4cb2 :

3094: Classic xunit-style functions and methods now obey the scope of autouse fixtures.

Now the xunit-style functions are integrated with the fixture mechanism and obey the
proper scope rules of fixtures involved in the call.

That introduced in pytest 4.2 new hidden (internal only) fixtures:

xunit_setup_module_fixture
xunit_setup_function_fixture
xunit_setup_class_fixture
xunit_setup_method_fixture

which are auto-injected on the corresponding levels.
How does this feature affect the IPA test suite?

Let's take a look at one of the errors.

ipa-run-tests test_xmlrpc/test_sudocmd_plugin.py::TestSudoCmd

==================================== ERRORS ====================================
___________ ERROR at teardown of TestSudoCmd.test_update_and_verify ____________
tp = <class 'AttributeError'>, value = None, tb = None                          
    def reraise(tp, value, tb=None):                                            
        try:                                                                    
            if value is None:                                                   
                value = tp()                                                    
            if value.__traceback__ is not tb:                                   
                raise value.with_traceback(tb)                                  
>           raise value                                                         
root/.local/lib/python3.7/site-packages/six.py:693:                             
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
root/.local/lib/python3.7/site-packages/six.py:693: in reraise                  
    raise value                                                                 
root/.local/lib/python3.7/site-packages/six.py:693: in reraise                  
    raise value                                                                 
usr/lib/python3.7/site-packages/ipatests/test_xmlrpc/tracker/base.py:253: in cleanup
    del_command()                                                               
usr/lib/python3.7/site-packages/ipatests/test_xmlrpc/tracker/base.py:71: in run_command
    result = cmd(*args, **options)                                              
usr/lib/python3.7/site-packages/ipalib/frontend.py:450: in __call__             
    return self.__do_call(*args, **options)                                     
usr/lib/python3.7/site-packages/ipalib/frontend.py:478: in __do_call            
    ret = self.run(*args, **options)                                            
usr/lib/python3.7/site-packages/ipalib/frontend.py:801: in run                  
    return self.forward(*args, **options)                                       
usr/lib/python3.7/site-packages/ipalib/frontend.py:824: in forward              
    *args, **kw)                                                                
usr/lib/python3.7/site-packages/ipalib/rpc.py:1139: in forward                  
    command = getattr(self.conn, name)                                          
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = ipaclient.plugins.rpcclient.rpcclient()
    def __get_conn(self):                                                       
        """                                                                     
        Return thread-local connection.                                         
        """                                                                     
        if not hasattr(context, self.id):                                       
            raise AttributeError(                                               
                "{0} is not connected ({1} in {2})".format(                     
                    self.name,                                                  
                    self.id,                                                    
>                   threading.currentThread().getName()                         
                )                                                               
            )                                                                   
E           AttributeError: rpcclient is not connected (rpcclient_140106773738160 in MainThread)
usr/lib/python3.7/site-packages/ipalib/backend.py:108: AttributeError           
----------------------------- Captured stdout call -----------------------------
Ran command: ipaserver.plugins.sudocmd.sudocmd_mod()('/usr/bin/sudotestcmd1', description='Updated sudo command 1', version='2.232'): OK
Ran command: ipaserver.plugins.sudocmd.sudocmd_show()('/usr/bin/sudotestcmd1', all=False, version='2.232'): OK
--------------------------- Captured stdout teardown ---------------------------
Ran command: ipaserver.plugins.sudocmd.sudocmd_del()('/usr/bin/sudotestcmd1', version='2.232'): OK
Ran command: ipaserver.plugins.sudocmd.sudocmd_del()('/usr/bin/sudoTestCmd1', version='2.232'): AttributeError: rpcclient is not connected (rpcclient_140106773738160 in MainThread)
====================== 5 passed, 1 error in 4.09 seconds ======================

So, an error happens in teardown code. Let's look at setup plan:

ipa-run-tests test_xmlrpc/test_sudocmd_plugin.py::TestSudoCmd --setup-only
usr/lib/python3.7/site-packages/ipatests/test_xmlrpc/test_sudocmd_plugin.py 
      SETUP    C _Class__pytest_setup_class
      SETUP    C sudocmd1
      SETUP    C sudocmd2
        usr/lib/python3.7/site-packages/ipatests/test_xmlrpc/test_sudocmd_plugin.py::TestSudoCmd::test_create (fixtures used: _Class__pytest_setup_class, sudocmd1, sudocmd2)
        usr/lib/python3.7/site-packages/ipatests/test_xmlrpc/test_sudocmd_plugin.py::TestSudoCmd::test_create_duplicates (fixtures used: _Class__pytest_setup_class, sudocmd1, sudocmd2)
        usr/lib/python3.7/site-packages/ipatests/test_xmlrpc/test_sudocmd_plugin.py::TestSudoCmd::test_retrieve (fixtures used: _Class__pytest_setup_class, sudocmd1)
        usr/lib/python3.7/site-packages/ipatests/test_xmlrpc/test_sudocmd_plugin.py::TestSudoCmd::test_search (fixtures used: _Class__pytest_setup_class, sudocmd1, sudocmd2)
        usr/lib/python3.7/site-packages/ipatests/test_xmlrpc/test_sudocmd_plugin.py::TestSudoCmd::test_update_and_verify (fixtures used: _Class__pytest_setup_class, sudocmd1)
      TEARDOWN C sudocmd1
      TEARDOWN C _Class__pytest_setup_class
      TEARDOWN C sudocmd2E
=========================================== ERRORS ============================================
___________________ ERROR at teardown of TestSudoCmd.test_update_and_verify ___________________
...

SETUPs are OK, but TEARDOWN C _Class__pytest_setup_class should follow TEARDOWN C sudocmd2.
The cause is that teardown_ functions are now usual fixtures and follow corresponding rules:

Within a function request for features, fixture of higher-scopes (such as session) are
instantiated first than lower-scoped fixtures (such as function or class). The relative order
of fixtures of same scope follows the declared order in the test function and honours
dependencies between fixtures.

Thus, there is only one legal way to arrange the fixtures within one scope (level) in a strict order - the specification for fixtures interdependencies. This means that all the class fixtures should depend on class setup/teardown ones. But the latter is hidden from usage.
Hence, all the setup_class/teardown_class methods should be replaced with the corresponding auto fixtures and should be linked from the other class fixtures.

The overall problem is mixing of idioms of different test frameworks (nose, unittest, pytest) in the test suite. Yes, pytest supports all of them, but doesn't provide the full power:

https://docs.pytest.org/en/latest/nose.html
https://docs.pytest.org/en/latest/unittest.html

My proposal is to have only one style for test writing - pytest. Other ones should be refactored into the latter. The implementation plan might be as follows:

Plain classes
Replace

def setup_class(cls)
def setup(self)

with

@pytest.fixture(autouse=True, scope="class")
@pytest.fixture(autouse=True)
def somefixture_setup(self, request)

Replace

def teardown_class(cls)
def teardown(self)
def teardown_method(self, method)

with

def fin():
...
request.addfinalizer(fin)

Add corresponding dependencies of base class fixtures

This part is ready to preview:
https://github.com/stanislavlevin/freeipa/tree/pytest4_errors_xmlrpc_run_pytest4
https://dev.azure.com/slev0400/slev/_build/results?buildId=163

unittest.TestCase classes
Replace

def setUp(self)
def setUpClass(cls)

with

@pytest.fixture(autouse=True)
@pytest.fixture(autouse=True, scope="class")
def somefixture_setup(self, request)

Replace

def tearDown(self)
def tearDownClass(cls)

with

def fin():
...
request.addfinalizer(fin)

Add corresponding dependencies of base class fixtures
Replace unittest asserts with analogues

So, my questions are:
Does anybody need it?
Does anybody need pytest4 compatibility?
Thanks in advance!

slev commented Jun 21, 2019

Metadata Update from @abbra:
- Issue set to the milestone: FreeIPA 4.8.1

Jul 16, 2019

abbra commented Jul 16, 2019

@slev the proposal sounds good to me.

slev commented Jul 17, 2019

@abbra, thanks for the feedback.
I'm going to resume work on it after vacation (next two weeks).

dmoluguw commented Oct 10, 2019

Hello @slev , the PKI team runs the IPA test suite in our nightly test [1] as well as per commit tests. Your proposal will help fix our CI environment. Thank you

[1] https://travis-ci.org/SilleBille/pki-nightly-test/jobs/596143907

slev commented Oct 10, 2019

Hi @dmoluguw, I'm sorry I broke that promise.
I would like to get a second chance.

Metadata Update from @pcech:
- Issue tagged with: Raven

Oct 14, 2019

slev commented Oct 16, 2019

https://github.com/freeipa/freeipa/pull/3783

abbra commented Feb 12, 2020

master:

292d686c0b7935dd219559b10de3a393b712a990 pytest: Migrate xunit-style setups to Pytest fixtures
fec66942d469188fcd1ccfbd6e14f4e8334056b4 pytest: Migrate unittest/nose to Pytest fixtures
8c7447fd42548d28df6200e354afdc03355b94b6 pytest: Warn about unittest/nose/xunit tests
43a97082bbdf5992dcd355fdcb28ff3bfe66ab59 Update Azure Pipelines to use Fedora 31
19462788f162491e0aedaaf42528cae7104b8068 ipatests: Properly kill gpg-agent
92b440a0baf590fcb3082dfe2ca673813e1e5b7c pylint: Teach Pylint how to handle request.context
e128e7d691d0102460a54359dc8855fb7fcd7f35 pylint: Synchronize pylint plugin to ipatests code
a309de6c08987b1cf9645111414d1c52b4525190 pylint: Clean up comment
ba12165eaf76e6fee30b05b0fe939e13854d70dc lint: Make Pylint-2.4 happy again

Metadata Update from @abbra:
- Issue close_status updated to: fixed
- Issue status updated to: Closed (was: Open)

Feb 12, 2020

abiagion commented Feb 13, 2020

ipa-4-8:

87bc31464b6133af9befd412af54403665c22628 pytest: Migrate xunit-style setups to Pytest fixtures
356f907fc255ab3a9f93ff2808646b92a6652aec pytest: Migrate unittest/nose to Pytest fixtures
3659b46d6aeea06b4875860ec69a9215afcbdd91 pytest: Warn about unittest/nose/xunit tests
f4e2acd1333f0f3d88da81d3fda80e85c9c418c2 Update Azure Pipelines to use Fedora 31
294694ad69fa909e2f699cb2dad0f36b966a246f ipatests: Properly kill gpg-agent
5939c90752db9da1adaf8c0bfe6bec3d6c1e2ad6 pylint: Teach Pylint how to handle request.context
3460db4ee7c7ce6c9a639a644a39c4df09ce31ac pylint: Synchronize pylint plugin to ipatests code
6f48848562f4e9ab9584154fd85e6ad1ac331ecd pylint: Clean up comment
44a59ff39a3f481e90043e546c892c9108231d67 lint: Make Pylint-2.4 happy again

Metadata

Assignee

None

Tags

Blocking

None

Depending on

None

Priority

None

freeipa

#7989 Pytest4.2+ errors Closed: fixed Feb 12, 2020 by abbra. Opened Jun 21, 2019 by slev.

Plain classes

unittest.TestCase classes

Metadata

Raven

#7989 Pytest4.2+ errors

Closed: fixed Feb 12, 2020 by abbra. Opened Jun 21, 2019 by slev.