From d09630234a82d73aaa34861ea0bbebf4489f9446 Mon Sep 17 00:00:00 2001 From: Robert Collins Date: Wed, 3 Jun 2015 19:20:36 +1200 Subject: [PATCH] Consolidating redundant code in the tests This moves things that are pretty much copy-paste into one place, letting us focus on the actual differences amongst the tests. I've also made the local test fixtures explicit, so we can see what each test really needs, which has also eliminated redundant test overhead, the slowest test is now 0.05s vs 0.1 before. This is more than paid for by the reduced duplication in the basic setup, making the patch a net reduction in LOC. It also fixes the tempdir leak that was present in two of the test scripts. Change-Id: I63eee7681657b2b5eb7959738cc89c57c117fa3b --- tests/common.py | 72 +++++++++++++++++++ tests/test_update.py | 139 +++++++++++++----------------------- tests/test_update_pbr.py | 49 +++---------- tests/test_update_suffix.py | 81 ++++++++------------- update.py | 8 ++- 5 files changed, 165 insertions(+), 184 deletions(-) create mode 100644 tests/common.py diff --git a/tests/common.py b/tests/common.py new file mode 100644 index 0000000000..1ab8687f6f --- /dev/null +++ b/tests/common.py @@ -0,0 +1,72 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import os.path +import shutil + +import fixtures + + +def _file_to_list(fname): + with open(fname) as f: + content = list(map(lambda x: x.rstrip(), f.readlines())) + print(content) + return content + + +class Project(fixtures.Fixture): + """A single project we can update.""" + + def __init__( + self, req_path, setup_path, setup_cfg_path, test_req_path=None): + super(Project, self).__init__() + self._req_path = req_path + self._setup_path = setup_path + self._setup_cfg_path = setup_cfg_path + self._test_req_path = test_req_path + + def setUp(self): + super(Project, self).setUp() + self.root = self.useFixture(fixtures.TempDir()).path + self.req_file = os.path.join(self.root, 'requirements.txt') + self.setup_file = os.path.join(self.root, 'setup.py') + self.setup_cfg_file = os.path.join(self.root, 'setup.cfg') + self.test_req_file = os.path.join(self.root, 'test-requirements.txt') + shutil.copy(self._req_path, self.req_file) + shutil.copy(self._setup_path, self.setup_file) + shutil.copy(self._setup_cfg_path, self.setup_cfg_file) + if self._test_req_path: + shutil.copy(self._test_req_path, self.test_req_file) + + +project_fixture = Project( + "tests/files/project.txt", + "tests/files/setup.py", "tests/files/setup.cfg", + "tests/files/test-project.txt") +bad_project_fixture = Project( + "tests/files/project-with-bad-requirement.txt", "tests/files/setup.py", + "tests/files/setup.cfg") +oslo_fixture = Project( + "tests/files/project-with-oslo-tar.txt", "tests/files/old-setup.py", + "tests/files/setup.cfg") +pbr_fixture = Project( + "tests/files/project.txt", "tests/files/setup.py", + "tests/files/pbr_setup.cfg", "tests/files/test-project.txt") + + +class GlobalRequirements(fixtures.Fixture): + + def setUp(self): + super(GlobalRequirements, self).setUp() + self.root = self.useFixture(fixtures.TempDir()).path + self.req_file = os.path.join(self.root, "global-requirements.txt") + shutil.copy("tests/files/gr-base.txt", self.req_file) diff --git a/tests/test_update.py b/tests/test_update.py index 5668916b78..0fb84bb436 100644 --- a/tests/test_update.py +++ b/tests/test_update.py @@ -14,91 +14,36 @@ from __future__ import print_function -import os -import os.path -import shutil import StringIO import fixtures import testtools +from tests import common import update -def _file_to_list(fname): - with open(fname) as f: - content = list(map(lambda x: x.rstrip(), f.readlines())) - print(content) - return content - - class UpdateTest(testtools.TestCase): - def _init_env(self): - self.project_dir = os.path.join(self.dir, "project") - self.bad_project_dir = os.path.join(self.dir, "bad_project") - self.oslo_dir = os.path.join(self.dir, "project_with_oslo") - - self.req_file = os.path.join(self.dir, "global-requirements.txt") - self.proj_file = os.path.join(self.project_dir, "requirements.txt") - self.oslo_file = os.path.join(self.oslo_dir, "requirements.txt") - self.bad_proj_file = os.path.join(self.bad_project_dir, - "requirements.txt") - self.proj_test_file = os.path.join(self.project_dir, - "test-requirements.txt") - self.setup_file = os.path.join(self.project_dir, "setup.py") - self.old_setup_file = os.path.join(self.oslo_dir, "setup.py") - self.bad_setup_file = os.path.join(self.bad_project_dir, "setup.py") - self.setup_cfg_file = os.path.join(self.project_dir, "setup.cfg") - self.bad_setup_cfg_file = os.path.join(self.bad_project_dir, - "setup.cfg") - self.oslo_setup_cfg_file = os.path.join(self.oslo_dir, "setup.cfg") - os.mkdir(self.project_dir) - os.mkdir(self.oslo_dir) - os.mkdir(self.bad_project_dir) - - shutil.copy("tests/files/gr-base.txt", self.req_file) - shutil.copy("tests/files/project-with-oslo-tar.txt", self.oslo_file) - shutil.copy("tests/files/project.txt", self.proj_file) - shutil.copy("tests/files/project-with-bad-requirement.txt", - self.bad_proj_file) - shutil.copy("tests/files/test-project.txt", self.proj_test_file) - shutil.copy("tests/files/setup.py", self.setup_file) - shutil.copy("tests/files/setup.py", self.bad_setup_file) - shutil.copy("tests/files/old-setup.py", self.old_setup_file) - shutil.copy("tests/files/setup.cfg", self.setup_cfg_file) - shutil.copy("tests/files/setup.cfg", self.bad_setup_cfg_file) - shutil.copy("tests/files/setup.cfg", self.oslo_setup_cfg_file) - shutil.copy("update.py", os.path.join(self.dir, "update.py")) - - def _run_update(self): - # now go call update and see what happens - update.main(['project']) - update.main(['project_with_oslo']) - def setUp(self): super(UpdateTest, self).setUp() - self.dir = self.useFixture(fixtures.TempDir()).path - self._init_env() - # for convenience put us in the directory with the update.py - self.addCleanup(os.chdir, os.path.abspath(os.curdir)) - os.chdir(self.dir) + self.global_env = self.useFixture(common.GlobalRequirements()) def test_requirements(self): - self._run_update() - reqs = _file_to_list(self.req_file) + # This is testing our test input data. Perhaps remove? (lifeless) + reqs = common._file_to_list(self.global_env.req_file) self.assertIn("jsonschema>=1.0.0,!=1.4.0,<2", reqs) def test_project(self): - self._run_update() - reqs = _file_to_list(self.proj_file) + self.project = self.useFixture(common.project_fixture) + update.main(['--source', self.global_env.root, self.project.root]) + reqs = common._file_to_list(self.project.req_file) # ensure various updates take self.assertIn("jsonschema>=1.0.0,!=1.4.0,<2", reqs) self.assertIn("python-keystoneclient>=0.4.1", reqs) self.assertIn("SQLAlchemy>=0.7,<=0.7.99", reqs) def test_requirements_header(self): - self._run_update() _REQS_HEADER = [ '# The order of packages is significant, because pip processes ' 'them in the order', @@ -106,58 +51,73 @@ class UpdateTest(testtools.TestCase): 'integration', '# process, which may cause wedges in the gate later.', ] - reqs = _file_to_list(self.proj_file) + self.project = self.useFixture(common.project_fixture) + update.main(['--source', self.global_env.root, self.project.root]) + reqs = common._file_to_list(self.project.req_file) self.assertEqual(_REQS_HEADER, reqs[:3]) def test_project_with_oslo(self): - self._run_update() - reqs = _file_to_list(self.oslo_file) + self.oslo_project = self.useFixture(common.oslo_fixture) + update.main( + ['--source', self.global_env.root, self.oslo_project.root]) + reqs = common._file_to_list(self.oslo_project.req_file) oslo_tar = ("-f http://tarballs.openstack.org/oslo.config/" "oslo.config-1.2.0a3.tar.gz#egg=oslo.config-1.2.0a3") self.assertIn(oslo_tar, reqs) def test_test_project(self): - self._run_update() - reqs = _file_to_list(self.proj_test_file) + self.project = self.useFixture(common.project_fixture) + update.main(['--source', self.global_env.root, self.project.root]) + reqs = common._file_to_list(self.project.test_req_file) self.assertIn("testtools>=0.9.32", reqs) self.assertIn("testrepository>=0.0.17", reqs) # make sure we didn't add something we shouldn't self.assertNotIn("sphinxcontrib-pecanwsme>=0.2", reqs) def test_install_setup(self): - self._run_update() - setup_contents = _file_to_list(self.setup_file) + self.project = self.useFixture(common.project_fixture) + update.main(['--source', self.global_env.root, self.project.root]) + setup_contents = common._file_to_list(self.project.setup_file) self.assertIn("# THIS FILE IS MANAGED BY THE GLOBAL REQUIREMENTS REPO" " - DO NOT EDIT", setup_contents) def test_no_install_setup(self): - self._run_update() - setup_contents = _file_to_list(self.old_setup_file) + self.oslo_project = self.useFixture(common.oslo_fixture) + update.main( + ['--source', self.global_env.root, self.oslo_project.root]) + setup_contents = common._file_to_list(self.oslo_project.setup_file) self.assertNotIn( "# THIS FILE IS MANAGED BY THE GLOBAL REQUIREMENTS REPO" " - DO NOT EDIT", setup_contents) # These are tests which don't need to run the project update in advance - def test_requirment_not_in_global(self): + def test_requirement_not_in_global(self): + self.bad_project = self.useFixture(common.bad_project_fixture) with testtools.ExpectedException(Exception): - update.main(['bad_project']) + update.main( + ['--source', self.global_env.root, self.bad_project.root]) - def test_requirment_not_in_global_non_fatal(self): + def test_requirement_not_in_global_non_fatal(self): self.useFixture( fixtures.EnvironmentVariable("NON_STANDARD_REQS", "1")) - update.main(["bad_project"]) + self.bad_project = self.useFixture(common.bad_project_fixture) + update.main(['--source', self.global_env.root, self.bad_project.root]) def test_requirement_soft_update(self): - update.main(["-s", "bad_project"]) - reqs = _file_to_list(self.bad_proj_file) + self.bad_project = self.useFixture(common.bad_project_fixture) + update.main([ + '-s', '--source', self.global_env.root, self.bad_project.root]) + reqs = common._file_to_list(self.bad_project.req_file) self.assertIn("thisisnotarealdepedency", reqs) # testing output def test_non_verbose_output(self): capture = StringIO.StringIO() - update.main(['project'], capture) - expected = 'Version change for: greenlet, sqlalchemy, eventlet, pastedeploy, routes, webob, wsgiref, boto, kombu, pycrypto, python-swiftclient, lxml, jsonschema, python-keystoneclient\n' # noqa - expected += """Updated project/requirements.txt: + self.project = self.useFixture(common.project_fixture) + update.main( + ['--source', self.global_env.root, self.project.root], capture) + expected = ('Version change for: greenlet, sqlalchemy, eventlet, pastedeploy, routes, webob, wsgiref, boto, kombu, pycrypto, python-swiftclient, lxml, jsonschema, python-keystoneclient\n' # noqa + """Updated %(project)s/requirements.txt: greenlet>=0.3.1 -> greenlet>=0.3.2 SQLAlchemy>=0.7.8,<=0.7.99 -> SQLAlchemy>=0.7,<=0.7.99 eventlet>=0.9.12 -> eventlet>=0.12.0 @@ -173,20 +133,23 @@ class UpdateTest(testtools.TestCase): jsonschema -> jsonschema>=1.0.0,!=1.4.0,<2 python-keystoneclient>=0.2.0 -> python-keystoneclient>=0.4.1 Version change for: mox, mox3, testrepository, testtools -Updated project/test-requirements.txt: +Updated %(project)s/test-requirements.txt: mox==0.5.3 -> mox>=0.5.3 mox3==0.7.3 -> mox3>=0.7.0 testrepository>=0.0.13 -> testrepository>=0.0.17 testtools>=0.9.27 -> testtools>=0.9.32 -""" +""") % dict(project=self.project.root) self.assertEqual(expected, capture.getvalue()) def test_verbose_output(self): capture = StringIO.StringIO() - update.main(['-v', 'project'], capture) - expected = """Syncing project/requirements.txt + self.project = self.useFixture(common.project_fixture) + update.main( + ['--source', self.global_env.root, '-v', self.project.root], + capture) + expected = ("""Syncing %(project)s/requirements.txt Version change for: greenlet, sqlalchemy, eventlet, pastedeploy, routes, webob, wsgiref, boto, kombu, pycrypto, python-swiftclient, lxml, jsonschema, python-keystoneclient\n""" # noqa - expected += """Updated project/requirements.txt: + """Updated %(project)s/requirements.txt: greenlet>=0.3.1 -> greenlet>=0.3.2 SQLAlchemy>=0.7.8,<=0.7.99 -> SQLAlchemy>=0.7,<=0.7.99 eventlet>=0.9.12 -> eventlet>=0.12.0 @@ -201,13 +164,13 @@ Version change for: greenlet, sqlalchemy, eventlet, pastedeploy, routes, webob, lxml -> lxml>=2.3 jsonschema -> jsonschema>=1.0.0,!=1.4.0,<2 python-keystoneclient>=0.2.0 -> python-keystoneclient>=0.4.1 -Syncing project/test-requirements.txt +Syncing %(project)s/test-requirements.txt Version change for: mox, mox3, testrepository, testtools -Updated project/test-requirements.txt: +Updated %(project)s/test-requirements.txt: mox==0.5.3 -> mox>=0.5.3 mox3==0.7.3 -> mox3>=0.7.0 testrepository>=0.0.13 -> testrepository>=0.0.17 testtools>=0.9.27 -> testtools>=0.9.32 Syncing setup.py -""" +""") % dict(project=self.project.root) self.assertEqual(expected, capture.getvalue()) diff --git a/tests/test_update_pbr.py b/tests/test_update_pbr.py index b45dbc2948..d0b7ee40ac 100644 --- a/tests/test_update_pbr.py +++ b/tests/test_update_pbr.py @@ -18,68 +18,37 @@ from __future__ import print_function -import os -import shutil -import tempfile - import testtools +from tests import common import update -def _file_to_list(fname): - with open(fname) as f: - content = list(map(lambda x: x.rstrip(), f.readlines())) - print(content) - return content - - class UpdateTestPbr(testtools.TestCase): def setUp(self): super(UpdateTestPbr, self).setUp() - self.dir = tempfile.mkdtemp() - self.project_dir = os.path.join(self.dir, "project_pbr") - - self.req_file = os.path.join(self.dir, "global-requirements.txt") - self.proj_file = os.path.join(self.project_dir, "requirements.txt") - self.proj_test_file = os.path.join(self.project_dir, - "test-requirements.txt") - self.setup_file = os.path.join(self.project_dir, "setup.py") - self.setup_cfg_file = os.path.join(self.project_dir, "setup.cfg") - os.mkdir(self.project_dir) - - shutil.copy("tests/files/gr-base.txt", self.req_file) - shutil.copy("tests/files/project.txt", self.proj_file) - shutil.copy("tests/files/test-project.txt", self.proj_test_file) - shutil.copy("tests/files/setup.py", self.setup_file) - shutil.copy("tests/files/pbr_setup.cfg", self.setup_cfg_file) - shutil.copy("update.py", os.path.join(self.dir, "update.py")) - - # now go call update and see what happens - self.addCleanup(os.chdir, os.path.abspath(os.curdir)) - os.chdir(self.dir) - update.main(["project_pbr"]) - - def test_requirements(self): - reqs = _file_to_list(self.req_file) - self.assertIn("jsonschema>=1.0.0,!=1.4.0,<2", reqs) + self.global_env = self.useFixture(common.GlobalRequirements()) + self.pbr = self.useFixture(common.pbr_fixture) def test_project(self): - reqs = _file_to_list(self.proj_file) + update.main(['--source', self.global_env.root, self.pbr.root]) + reqs = common._file_to_list(self.pbr.req_file) # ensure various updates take self.assertIn("jsonschema>=1.0.0,!=1.4.0,<2", reqs) self.assertIn("python-keystoneclient>=0.4.1", reqs) self.assertIn("SQLAlchemy>=0.7,<=0.7.99", reqs) def test_test_project(self): - reqs = _file_to_list(self.proj_test_file) + update.main(['--source', self.global_env.root, self.pbr.root]) + reqs = common._file_to_list(self.pbr.test_req_file) self.assertIn("testtools>=0.9.32", reqs) self.assertIn("testrepository>=0.0.17", reqs) # make sure we didn't add something we shouldn't self.assertNotIn("sphinxcontrib-pecanwsme>=0.2", reqs) def test_install_setup(self): - setup_contents = _file_to_list(self.setup_file) + update.main(['--source', self.global_env.root, self.pbr.root]) + setup_contents = common._file_to_list(self.pbr.setup_file) self.assertNotIn("# THIS FILE IS MANAGED BY THE GLOBAL REQUIREMENTS " "REPO - DO NOT EDIT", setup_contents) diff --git a/tests/test_update_suffix.py b/tests/test_update_suffix.py index 526fb805eb..0fe6fc1944 100644 --- a/tests/test_update_suffix.py +++ b/tests/test_update_suffix.py @@ -14,91 +14,66 @@ from __future__ import print_function -import os -import os.path -import shutil -import tempfile - import testtools +from tests import common import update -def _file_to_list(fname): - with open(fname) as f: - content = list(map(lambda x: x.rstrip(), f.readlines())) - print(content) - return content - - class UpdateTestWithSuffix(testtools.TestCase): def setUp(self): super(UpdateTestWithSuffix, self).setUp() - self.dir = tempfile.mkdtemp() - self.project_dir = os.path.join(self.dir, "project") - self.oslo_dir = os.path.join(self.dir, "project_with_oslo") - - self.req_file = os.path.join(self.dir, "global-requirements.txt") - self.proj_file = os.path.join(self.project_dir, "requirements.txt") - self.oslo_file = os.path.join(self.oslo_dir, "requirements.txt") - self.proj_test_file = os.path.join(self.project_dir, - "test-requirements.txt") - self.setup_file = os.path.join(self.project_dir, "setup.py") - self.old_setup_file = os.path.join(self.oslo_dir, "setup.py") - self.setup_cfg_file = os.path.join(self.project_dir, "setup.cfg") - self.oslo_setup_cfg_file = os.path.join(self.oslo_dir, "setup.cfg") - os.mkdir(self.project_dir) - os.mkdir(self.oslo_dir) - - shutil.copy("tests/files/gr-base.txt", self.req_file) - shutil.copy("tests/files/project-with-oslo-tar.txt", self.oslo_file) - shutil.copy("tests/files/project.txt", self.proj_file) - shutil.copy("tests/files/test-project.txt", self.proj_test_file) - shutil.copy("tests/files/setup.py", self.setup_file) - shutil.copy("tests/files/old-setup.py", self.old_setup_file) - shutil.copy("tests/files/setup.cfg", self.setup_cfg_file) - shutil.copy("tests/files/setup.cfg", self.oslo_setup_cfg_file) - shutil.copy("update.py", os.path.join(self.dir, "update.py")) - - # now go call update and see what happens - self.addCleanup(os.chdir, os.path.abspath(os.curdir)) - os.chdir(self.dir) - update.main(['-o', 'global', 'project']) - update.main(['-o', 'global', 'project_with_oslo']) - - def test_requirements(self): - # this is the sanity check test - reqs = _file_to_list(self.req_file) - self.assertIn("jsonschema>=1.0.0,!=1.4.0,<2", reqs) + self.global_env = self.useFixture(common.GlobalRequirements()) def test_project(self): - reqs = _file_to_list("%s.%s" % (self.proj_file, 'global')) + project = self.useFixture(common.project_fixture) + update.main( + ['--source', self.global_env.root, '-o', 'global', + project.root]) + reqs = common._file_to_list("%s.%s" % (project.req_file, 'global')) # ensure various updates take self.assertIn("jsonschema>=1.0.0,!=1.4.0,<2", reqs) self.assertIn("python-keystoneclient>=0.4.1", reqs) self.assertIn("SQLAlchemy>=0.7,<=0.7.99", reqs) def test_project_with_oslo(self): - reqs = _file_to_list("%s.%s" % (self.oslo_file, 'global')) + project = self.useFixture(common.oslo_fixture) + update.main( + ['--source', self.global_env.root, '-o', 'global', + project.root]) + reqs = common._file_to_list("%s.%s" % (project.req_file, 'global')) oslo_tar = ("-f http://tarballs.openstack.org/oslo.config/" "oslo.config-1.2.0a3.tar.gz#egg=oslo.config-1.2.0a3") self.assertIn(oslo_tar, reqs) def test_test_project(self): - reqs = _file_to_list("%s.%s" % (self.proj_test_file, 'global')) + project = self.useFixture(common.project_fixture) + update.main( + ['--source', self.global_env.root, '-o', 'global', + project.root]) + reqs = common._file_to_list( + "%s.%s" % (project.test_req_file, 'global')) self.assertIn("testtools>=0.9.32", reqs) self.assertIn("testrepository>=0.0.17", reqs) # make sure we didn't add something we shouldn't self.assertNotIn("sphinxcontrib-pecanwsme>=0.2", reqs) def test_install_setup(self): - setup_contents = _file_to_list(self.setup_file) + project = self.useFixture(common.project_fixture) + update.main( + ['--source', self.global_env.root, '-o', 'global', + project.root]) + setup_contents = common._file_to_list(project.setup_file) self.assertIn("# THIS FILE IS MANAGED BY THE GLOBAL REQUIREMENTS REPO" " - DO NOT EDIT", setup_contents) def test_no_install_setup(self): - setup_contents = _file_to_list(self.old_setup_file) + project = self.useFixture(common.oslo_fixture) + update.main( + ['--source', self.global_env.root, '-o', 'global', + project.root]) + setup_contents = common._file_to_list(project.setup_file) self.assertNotIn( "# THIS FILE IS MANAGED BY THE GLOBAL REQUIREMENTS REPO" " - DO NOT EDIT", setup_contents) diff --git a/update.py b/update.py index 01f1999b57..17a04ee5c4 100644 --- a/update.py +++ b/update.py @@ -206,10 +206,10 @@ def _sync_requirements_file( stdout.write(" %s\n" % change) -def _copy_requires(suffix, softupdate, hacking, dest_dir, stdout): +def _copy_requires(suffix, softupdate, hacking, dest_dir, stdout, source="."): """Copy requirements files.""" - source_reqs = _parse_reqs('global-requirements.txt') + source_reqs = _parse_reqs(os.path.join(source, 'global-requirements.txt')) target_files = [ 'requirements.txt', 'tools/pip-requires', @@ -255,6 +255,8 @@ def main(argv=None, stdout=None): parser.add_option("-v", "--verbose", dest="verbose", action="store_true", help="Add further verbosity to output") + parser.add_option("--source", dest="source", default=".", + help="Dir where global-requirements.txt is located.") options, args = parser.parse_args(argv) if len(args) != 1: print("Must specify directory to update") @@ -264,7 +266,7 @@ def main(argv=None, stdout=None): if stdout is None: stdout = sys.stdout _copy_requires(options.suffix, options.softupdate, options.hacking, - args[0], stdout=stdout) + args[0], stdout=stdout, source=options.source) _write_setup_py(args[0], stdout=stdout)