CONF calls in testtools skip decorators causes import errors without a config file

Bug #1369118 reported by Matthew Treinish
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
tempest
Fix Released
High
Matthew Treinish

Bug Description

Throughout the tempest code the testtools skip decorators, skipIf() and skipUnless() are used to skip tests based on various conditions. However, this causes issues with listing tests without a config file.

For example, running 'testr list-tests' without a config file, while using a modified testtools to get stack traces on import errors (https://github.com/mtreinish/testtools/commit/430cec9321f0a37cca801797ffdb205f503c911f ) yields:

Failed to import test module: tempest.scenario.test_stamp_pattern\nTraceback (most recent call last):\n File "/git/testtools/testtools/run.py", line 481, in _find_tests\n module = self._get_module_from_name(name)\n File "/usr/lib64/python2.7/unittest/loader.py", line 232, in _get_module_from_name\n __import__(name)\n File "tempest/scenario/test_stamp_pattern.py", line 33, in <module>\n class TestStampPattern(manager.OfficialClientTest):\n File "tempest/scenario/test_stamp_pattern.py", line 154, in TestStampPattern\n @testtools.skipUnless(CONF.compute_feature_enabled.snapshot,\n File "tempest/config.py", line 1174, in __getattr__\n self._config = TempestConfigPrivate(config_path=self._path)\n File "tempest/config.py", line 1152, in __init__\n cfg.CONF.log_opt_values(LOG, std_logging.DEBUG)\n File "/git/tempest/.tox/py27/lib/python2.7/site-packages/oslo/config/cfg.py", line 1921, in log_opt_values\n _sanitize(opt, getattr(group_attr, opt_name)))\n File "/git/tempest/.tox/py27/lib/python2.7/site-packages/oslo/config/cfg.py", line 2226, in __getattr__\n return self._conf._get(name, self._group)\n File "/git/tempest/.tox/py27/lib/python2.7/site-packages/oslo/config/cfg.py", line 1964, in _get\n value = self._do_get(name, group, namespace)\n File "/git/tempest/.tox/py27/lib/python2.7/site-packages/oslo/config/cfg.py", line 2000, in _do_get\n return convert(opt._get_from_namespace(namespace, group_name))\n File "/git/tempest/.tox/py27/lib/python2.7/site-packages/oslo/config/cfg.py", line 1995, in convert\n return self._convert_value(self._substitute(value, namespace), opt)\n File "/git/tempest/.tox/py27/lib/python2.7/site-packages/oslo/config/cfg.py", line 2044, in _substitute\n self.StrSubWrapper(self, namespace=namespace))\n File "/usr/lib64/python2.7/string.py", line 205, in safe_substitute\n return self.pattern.sub(convert, self.template)\n File "/usr/lib64/python2.7/string.py", line 190, in convert\n return \'%s\' % (mapping[named],)\n File "/git/tempest/.tox/py27/lib/python2.7/site-packages/oslo/config/cfg.py", line 2301, in __getitem__\n value = self.conf._get(key, namespace=self.namespace)\n File "/git/tempest/.tox/py27/lib/python2.7/site-packages/oslo/config/cfg.py", line 1964, in _get\n value = self._do_get(name, group, namespace)\n File "/git/tempest/.tox/py27/lib/python2.7/site-packages/oslo/config/cfg.py", line 1982, in _do_get\n info = self._get_opt_info(name, group)\n File "/git/tempest/.tox/py27/lib/python2.7/site-packages/oslo/config/cfg.py", line 2106, in _get_opt_info\n raise NoSuchOptError(opt_name, group)\nNoSuchOptError: no such option: PUBLIC_NETWORK_ID

While I'm too lazy to clean up the formatting that basically shows the cause of the error at import time, the logic is due to the skip decorator performing a getattr on the CONF object during import. This is supported by the testtools code:

def skipIf(condition, reason):
    """A decorator to skip a test if the condition is true."""
    if condition:
        return skip(reason)
    def _id(obj):
        return obj
    return _id

and

def skipUnless(condition, reason):
    """A decorator to skip a test unless the condition is true."""
    if not condition:
        return skip(reason)
    def _id(obj):
        return obj
    return _id

To avoid this issue in tempest and remove an import time dependence on the config file we need to stop using these decorators in the short term and use ones that do the evaluation logic at runtime not import.

description: updated
summary: - CONF calls in testtools skip decorators
+ CONF calls in testtools skip decorators causes import errors without a
+ config file
Revision history for this message
Matthew Treinish (treinish) wrote :

It looks like this issue is more widespread than just the testtools decorators. It looks like direct any parameters that get passed into a decorator will be eval-ed on import. The only way to avoid this is to avoid doing any getattrs in a param for a decorator.

Instead we probably should add a new pair of decorators for skipIfConfig() and skipUnlessConfig() which will take the group and option as string args and do the get attr inside of a functools.wrap to avoid the import config dependency.

Revision history for this message
David Kranz (david-kranz) wrote :

We seem to be jumping through a lot of hoops to avoid just saying that you should be able to create the conf object even if there is no tempest.conf file. Is there a problem with treating the lack of a tempest.conf file the same as if there were one but it was empty?

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to tempest (master)

Fix proposed to branch: master
Review: https://review.openstack.org/125735

Changed in tempest:
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to tempest (master)

Reviewed: https://review.openstack.org/125735
Committed: https://git.openstack.org/cgit/openstack/tempest/commit/?id=5440a40d45c59929f70b1dbab1633d2d7071af21
Submitter: Jenkins
Branch: master

commit 5440a40d45c59929f70b1dbab1633d2d7071af21
Author: Matthew Treinish <email address hidden>
Date: Thu Oct 2 14:36:16 2014 -0400

    Don't parse config file if it doesn't exist

    This commit adds a check before we parse the config file to see if the
    config file actually exists. If it doesn't we shouldn't try to parse
    it. There is still an issue with one scenario test that uses
    testscenarios but it doesn't result in an error code being raised by a
    test-list so that can be tracked down separately.

    Change-Id: Iac31f2edcfbccf8c06d73c6ca01e30c8319c0485
    Closes-Bug: #1369118

Changed in tempest:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.