From f845891184b17e2c31a2b02dbc9217978abd3242 Mon Sep 17 00:00:00 2001 From: Duncan McGreggor Date: Mon, 21 Nov 2011 19:59:50 -0800 Subject: [PATCH] First steps towards consolidating testing infrastructure This commit begins to implement blueprint consolidate-testing-infrastructure by adding a 'testing' subpackage and moving some modules into it. Change-Id: I04bf860bc386bd2016e7dbc5a6f6ef7379a855bb --- HACKING => HACKING.rst | 101 +++++++++++------- doc/source/code.rst | 2 +- doc/source/devref/fakes.rst | 4 +- nova/api/ec2/__init__.py | 2 +- nova/auth/ldapdriver.py | 2 +- nova/auth/manager.py | 2 +- nova/rpc/impl_carrot.py | 4 +- nova/test.py | 4 +- nova/testing/README.rst | 44 ++++++++ nova/testing/__init__.py | 0 nova/testing/fake/__init__.py | 2 + .../fake/memcache.py} | 0 .../{fakerabbit.py => testing/fake/rabbit.py} | 2 +- run_tests.py => nova/testing/runner.py | 21 ++-- nova/tests/test_iptables_network.py | 10 +- run_tests.sh | 2 +- 16 files changed, 136 insertions(+), 66 deletions(-) rename HACKING => HACKING.rst (62%) create mode 100644 nova/testing/README.rst create mode 100644 nova/testing/__init__.py create mode 100644 nova/testing/fake/__init__.py rename nova/{fakememcache.py => testing/fake/memcache.py} (100%) rename nova/{fakerabbit.py => testing/fake/rabbit.py} (98%) rename run_tests.py => nova/testing/runner.py (98%) diff --git a/HACKING b/HACKING.rst similarity index 62% rename from HACKING rename to HACKING.rst index 962617e9a511..d9d0f41a69cd 100644 --- a/HACKING +++ b/HACKING.rst @@ -1,9 +1,9 @@ Nova Style Commandments ======================= -Step 1: Read http://www.python.org/dev/peps/pep-0008/ -Step 2: Read http://www.python.org/dev/peps/pep-0008/ again -Step 3: Read on +- Step 1: Read http://www.python.org/dev/peps/pep-0008/ +- Step 2: Read http://www.python.org/dev/peps/pep-0008/ again +- Step 3: Read on General @@ -23,7 +23,8 @@ Imports - Order your imports by the full module path - Organize your imports according to the following template -:: +Example:: + # vim: tabstop=4 shiftwidth=4 softtabstop=4 {{stdlib imports in human alphabetical order}} \n @@ -37,7 +38,8 @@ Imports Human Alphabetical Order Examples --------------------------------- -:: +Example:: + import httplib import logging import random @@ -58,6 +60,8 @@ Human Alphabetical Order Examples Docstrings ---------- +Example:: + """A one line docstring looks like this and ends in a period.""" @@ -87,34 +91,34 @@ Docstrings Dictionaries/Lists ------------------ - If a dictionary (dict) or list object is longer than 80 characters, its - items should be split with newlines. Embedded iterables should have their - items indented. Additionally, the last item in the dictionary should have - a trailing comma. This increases readability and simplifies future diffs. +If a dictionary (dict) or list object is longer than 80 characters, its items +should be split with newlines. Embedded iterables should have their items +indented. Additionally, the last item in the dictionary should have a trailing +comma. This increases readability and simplifies future diffs. - Example: +Example:: + + my_dictionary = { + "image": { + "name": "Just a Snapshot", + "size": 2749573, + "properties": { + "user_id": 12, + "arch": "x86_64", + }, + "things": [ + "thing_one", + "thing_two", + ], + "status": "ACTIVE", + }, + } - my_dictionary = { - "image": { - "name": "Just a Snapshot", - "size": 2749573, - "properties": { - "user_id": 12, - "arch": "x86_64", - }, - "things": [ - "thing_one", - "thing_two", - ], - "status": "ACTIVE", - }, - } - Calling Methods --------------- - Calls to methods 80 characters or longer should format each argument with - newlines. This is not a requirement, but a guideline. +Calls to methods 80 characters or longer should format each argument with +newlines. This is not a requirement, but a guideline:: unnecessarily_long_function_name('string one', 'string two', @@ -122,7 +126,7 @@ Calling Methods kwarg2=['a', 'b', 'c']) - Rather than constructing parameters inline, it is better to break things up: +Rather than constructing parameters inline, it is better to break things up:: list_of_strings = [ 'what_a_long_string', @@ -134,7 +138,7 @@ Calling Methods 'two': 2, 'twenty four': 24, } - + object_one.call_a_method('string three', 'string four', kwarg1=list_of_strings, @@ -143,23 +147,38 @@ Calling Methods Internationalization (i18n) Strings ----------------------------------- - In order to support multiple languages, we have a mechanism to support - automatic translations of exception and log strings. +In order to support multiple languages, we have a mechanism to support +automatic translations of exception and log strings. + +Example:: - Example: msg = _("An error occurred") raise HTTPBadRequest(explanation=msg) - - If you have a variable to place within the string, first internationalize - the template string then do the replacement. - Example: +If you have a variable to place within the string, first internationalize the +template string then do the replacement. + +Example:: + msg = _("Missing parameter: %s") % ("flavor",) LOG.error(msg) - - If you have multiple variables to place in the string, use keyword - parameters. This helps our translators reorder parameters when needed. - Example: +If you have multiple variables to place in the string, use keyword parameters. +This helps our translators reorder parameters when needed. + +Example:: + msg = _("The server with id %(s_id)s has no key %(m_key)s") LOG.error(msg % {"s_id": "1234", "m_key": "imageId"}) + + +Creating Unit Tests +------------------- +For every new feature, unit tests should be created that both test and +(implicitly) document the usage of said feature. If submitting a patch for a +bug that had no unit test, a new passing unit test should be added. If a +submitted bug fix does have a unit test, be sure to add a new one that fails +without the patch and passes with the patch. + +For more information on creating unit tests and utilizing the testing +infrastructure in OpenStack Nova, please read nova/testing/README.rst. diff --git a/doc/source/code.rst b/doc/source/code.rst index 73fc31e1a6e7..06549e1f52a9 100644 --- a/doc/source/code.rst +++ b/doc/source/code.rst @@ -29,7 +29,7 @@ Generating source/api/nova..db.sqlalchemy.api.rst Generating source/api/nova..db.sqlalchemy.models.rst Generating source/api/nova..db.sqlalchemy.session.rst Generating source/api/nova..exception.rst -Generating source/api/nova..fakerabbit.rst +Generating source/api/nova..fake.rabbit.rst Generating source/api/nova..flags.rst Generating source/api/nova..image.service.rst Generating source/api/nova..manager.rst diff --git a/doc/source/devref/fakes.rst b/doc/source/devref/fakes.rst index 6073447f0915..0aa37ce450cf 100644 --- a/doc/source/devref/fakes.rst +++ b/doc/source/devref/fakes.rst @@ -44,10 +44,10 @@ The :mod:`nova.auth.fakeldap` Module :show-inheritance: -The :mod:`nova.fakerabbit` Module +The :mod:`nova.testing.fake.rabbit` Module --------------------------------- -.. automodule:: nova.fakerabbit +.. automodule:: nova.testing.fake.rabbit :noindex: :members: :undoc-members: diff --git a/nova/api/ec2/__init__.py b/nova/api/ec2/__init__.py index 23042dab0062..0433d5152cf2 100644 --- a/nova/api/ec2/__init__.py +++ b/nova/api/ec2/__init__.py @@ -120,7 +120,7 @@ class Lockout(wsgi.Middleware): if FLAGS.memcached_servers: import memcache else: - from nova import fakememcache as memcache + from nova.testing.fake import memcache self.mc = memcache.Client(FLAGS.memcached_servers, debug=0) super(Lockout, self).__init__(application) diff --git a/nova/auth/ldapdriver.py b/nova/auth/ldapdriver.py index bc37d2d87ba9..6b9969dad157 100644 --- a/nova/auth/ldapdriver.py +++ b/nova/auth/ldapdriver.py @@ -72,7 +72,7 @@ LOG = logging.getLogger("nova.ldapdriver") if FLAGS.memcached_servers: import memcache else: - from nova import fakememcache as memcache + from nova.testing.fake import memcache # TODO(vish): make an abstract base class with the same public methods diff --git a/nova/auth/manager.py b/nova/auth/manager.py index 93b4244ad58e..3d984f1bf103 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -82,7 +82,7 @@ LOG = logging.getLogger('nova.auth.manager') if FLAGS.memcached_servers: import memcache else: - from nova import fakememcache as memcache + from nova.testing.fake import memcache class AuthBase(object): diff --git a/nova/rpc/impl_carrot.py b/nova/rpc/impl_carrot.py index 57fd074f0081..eed8cb10d079 100644 --- a/nova/rpc/impl_carrot.py +++ b/nova/rpc/impl_carrot.py @@ -41,9 +41,9 @@ import greenlet from nova import context from nova import exception -from nova import fakerabbit from nova import flags from nova.rpc.common import RemoteError, LOG +from nova.testing import fake # Needed for tests eventlet.monkey_patch() @@ -71,7 +71,7 @@ class Connection(carrot_connection.BrokerConnection): virtual_host=FLAGS.rabbit_virtual_host) if FLAGS.fake_rabbit: - params['backend_cls'] = fakerabbit.Backend + params['backend_cls'] = fake.rabbit.Backend # NOTE(vish): magic is fun! # pylint: disable=W0142 diff --git a/nova/test.py b/nova/test.py index 6c565f53d45c..448af047e6bd 100644 --- a/nova/test.py +++ b/nova/test.py @@ -35,12 +35,12 @@ import nova.image.fake import shutil import stubout -from nova import fakerabbit from nova import flags from nova import log from nova import rpc from nova import utils from nova import service +from nova.testing.fake import rabbit from nova.virt import fake @@ -142,7 +142,7 @@ class TestCase(unittest.TestCase): finally: # Clean out fake_rabbit's queue if we used it if FLAGS.fake_rabbit: - fakerabbit.reset_all() + rabbit.reset_all() if FLAGS.connection_type == 'fake': if hasattr(fake.FakeConnection, '_instance'): diff --git a/nova/testing/README.rst b/nova/testing/README.rst new file mode 100644 index 000000000000..036f1c77d3ac --- /dev/null +++ b/nova/testing/README.rst @@ -0,0 +1,44 @@ +===================================== +OpenStack Nova Testing Infrastructure +===================================== + +A note of clarification is in order, to help those who are new to testing in +OpenStack nova: + +- actual unit tests are created in the "tests" directory; +- the "testing" directory is used to house the infrastructure needed to support + testing in OpenStack Nova. + +This README file attempts to provide current and prospective contributors with +everything they need to know in order to start creating unit tests and +utilizing the convenience code provided in nova.testing. + +Note: the content for the rest of this file will be added as the work items in +the following blueprint are completed: + https://blueprints.launchpad.net/nova/+spec/consolidate-testing-infrastructure + + +Test Types: Unit vs. Functional vs. Integration +----------------------------------------------- + +TBD + +Writing Unit Tests +------------------ + +TBD + +Using Fakes +~~~~~~~~~~~ + +TBD + +Writing Functional Tests +------------------------ + +TBD + +Writing Integration Tests +------------------------- + +TBD diff --git a/nova/testing/__init__.py b/nova/testing/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/nova/testing/fake/__init__.py b/nova/testing/fake/__init__.py new file mode 100644 index 000000000000..d9eebbd23a15 --- /dev/null +++ b/nova/testing/fake/__init__.py @@ -0,0 +1,2 @@ +import memcache +import rabbit diff --git a/nova/fakememcache.py b/nova/testing/fake/memcache.py similarity index 100% rename from nova/fakememcache.py rename to nova/testing/fake/memcache.py diff --git a/nova/fakerabbit.py b/nova/testing/fake/rabbit.py similarity index 98% rename from nova/fakerabbit.py rename to nova/testing/fake/rabbit.py index 2807a76f6446..0cb91bd25c98 100644 --- a/nova/fakerabbit.py +++ b/nova/testing/fake/rabbit.py @@ -26,7 +26,7 @@ from eventlet import greenthread from nova import log as logging -LOG = logging.getLogger("nova.fakerabbit") +LOG = logging.getLogger("nova.testing.fake.rabbit") EXCHANGES = {} diff --git a/run_tests.py b/nova/testing/runner.py similarity index 98% rename from run_tests.py rename to nova/testing/runner.py index 17547b8e021d..9fbbb418d932 100644 --- a/run_tests.py +++ b/nova/testing/runner.py @@ -41,17 +41,18 @@ """Unittest runner for Nova. To run all tests - python run_tests.py - -To run a single test: - python run_tests.py test_compute:ComputeTestCase.test_run_terminate + python nova/testing/runner.py To run a single test module: - python run_tests.py test_compute + python nova/testing/runner.py test_compute or - python run_tests.py api.test_wsgi + python nova/testing/runner.py api.test_wsgi + +To run a single test: + python nova/testing/runner.py \ + test_compute:ComputeTestCase.test_run_terminate """ @@ -336,8 +337,7 @@ class NovaTestRunner(core.TextTestRunner): return result_ -if __name__ == '__main__': - eventlet.monkey_patch() +def run(): logging.setup() # If any argument looks like a test name but doesn't have "nova.tests" in # front of it, automatically add that so we don't have to type as much @@ -363,3 +363,8 @@ if __name__ == '__main__': config=c, show_elapsed=show_elapsed) sys.exit(not core.run(config=c, testRunner=runner, argv=argv)) + + +if __name__ == '__main__': + eventlet.monkey_patch() + run() diff --git a/nova/tests/test_iptables_network.py b/nova/tests/test_iptables_network.py index 9180342695f9..478dfa70d7c7 100644 --- a/nova/tests/test_iptables_network.py +++ b/nova/tests/test_iptables_network.py @@ -84,12 +84,12 @@ class IptablesManagerTestCase(test.TestCase): table = self.manager.ipv4['filter'] table.add_rule('FORWARD', '-s 1.2.3.4/5 -j DROP') new_lines = self.manager._modify_rules(current_lines, table) - self.assertTrue('-A run_tests.py-FORWARD ' + self.assertTrue('-A runner.py-FORWARD ' '-s 1.2.3.4/5 -j DROP' in new_lines) table.remove_rule('FORWARD', '-s 1.2.3.4/5 -j DROP') new_lines = self.manager._modify_rules(current_lines, table) - self.assertTrue('-A run_tests.py-FORWARD ' + self.assertTrue('-A runner.py-FORWARD ' '-s 1.2.3.4/5 -j DROP' not in new_lines) def test_nat_rules(self): @@ -123,7 +123,7 @@ class IptablesManagerTestCase(test.TestCase): "nova-postouting-bottom: %s" % last_postrouting_line) for chain in ['POSTROUTING', 'PREROUTING', 'OUTPUT']: - self.assertTrue('-A %s -j run_tests.py-%s' \ + self.assertTrue('-A %s -j runner.py-%s' \ % (chain, chain) in new_lines, "Built-in chain %s not wrapped" % (chain,)) @@ -155,10 +155,10 @@ class IptablesManagerTestCase(test.TestCase): break self.assertTrue('-A nova-filter-top ' - '-j run_tests.py-local' in new_lines, + '-j runner.py-local' in new_lines, "nova-filter-top does not jump to wrapped local chain") for chain in ['INPUT', 'OUTPUT', 'FORWARD']: - self.assertTrue('-A %s -j run_tests.py-%s' \ + self.assertTrue('-A %s -j runner.py-%s' \ % (chain, chain) in new_lines, "Built-in chain %s not wrapped" % (chain,)) diff --git a/run_tests.sh b/run_tests.sh index 837f9695adf1..a9b9dd1f902d 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -117,7 +117,7 @@ function run_pep8 { --exclude=vcsversion.py ${srcfiles} } -NOSETESTS="python run_tests.py $noseopts $noseargs" +NOSETESTS="python nova/testing/runner.py $noseopts $noseargs" if [ $never_venv -eq 0 ] then