From 673e88e8a6c8fa6cbea3534f80fddb56446e2685 Mon Sep 17 00:00:00 2001 From: Robert Collins Date: Wed, 3 Jun 2015 12:23:17 +1200 Subject: [PATCH] Stop using subprocess in tests. There's no need to run the script to test it, so lets not. Change-Id: I5b2115a5519d3a82dc3d35fc0d68f606fea97047 --- tests/test_update.py | 41 +++++++++++--------------- tests/test_update_pbr.py | 6 ++-- tests/test_update_suffix.py | 10 +++---- update.py | 59 +++++++++++++++++++------------------ 4 files changed, 55 insertions(+), 61 deletions(-) diff --git a/tests/test_update.py b/tests/test_update.py index 3a14d1dece..5668916b78 100644 --- a/tests/test_update.py +++ b/tests/test_update.py @@ -17,12 +17,13 @@ from __future__ import print_function import os import os.path import shutil -import subprocess -import sys +import StringIO import fixtures import testtools +import update + def _file_to_list(fname): with open(fname) as f: @@ -72,10 +73,8 @@ class UpdateTest(testtools.TestCase): def _run_update(self): # now go call update and see what happens - subprocess.check_output( - [sys.executable, "update.py", "project"]) - subprocess.check_output([sys.executable, "update.py", - "project_with_oslo"]) + update.main(['project']) + update.main(['project_with_oslo']) def setUp(self): super(UpdateTest, self).setUp() @@ -140,29 +139,23 @@ class UpdateTest(testtools.TestCase): # These are tests which don't need to run the project update in advance def test_requirment_not_in_global(self): - returncode = subprocess.call([sys.executable, "update.py", - "bad_project"]) - self.assertEqual(returncode, 1) + with testtools.ExpectedException(Exception): + update.main(['bad_project']) def test_requirment_not_in_global_non_fatal(self): - env = os.environ.copy() - env['NON_STANDARD_REQS'] = '1' - returncode = subprocess.call( - [sys.executable, "update.py", "bad_project"], - env=env) - self.assertEqual(returncode, 0) + self.useFixture( + fixtures.EnvironmentVariable("NON_STANDARD_REQS", "1")) + update.main(["bad_project"]) def test_requirement_soft_update(self): - returncode = subprocess.call( - [sys.executable, "update.py", "-s", "bad_project"]) - self.assertEqual(returncode, 0) + update.main(["-s", "bad_project"]) reqs = _file_to_list(self.bad_proj_file) self.assertIn("thisisnotarealdepedency", reqs) # testing output def test_non_verbose_output(self): - output = subprocess.check_output( - [sys.executable, "update.py", "project"]) + 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: greenlet>=0.3.1 -> greenlet>=0.3.2 @@ -186,11 +179,11 @@ Updated project/test-requirements.txt: testrepository>=0.0.13 -> testrepository>=0.0.17 testtools>=0.9.27 -> testtools>=0.9.32 """ - self.assertEqual(expected, output) + self.assertEqual(expected, capture.getvalue()) def test_verbose_output(self): - output = subprocess.check_output( - [sys.executable, "update.py", "-v", "project"]) + capture = StringIO.StringIO() + update.main(['-v', 'project'], capture) expected = """Syncing project/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: @@ -217,4 +210,4 @@ Updated project/test-requirements.txt: testtools>=0.9.27 -> testtools>=0.9.32 Syncing setup.py """ - self.assertEqual(expected, output) + self.assertEqual(expected, capture.getvalue()) diff --git a/tests/test_update_pbr.py b/tests/test_update_pbr.py index 3aa2416830..b45dbc2948 100644 --- a/tests/test_update_pbr.py +++ b/tests/test_update_pbr.py @@ -20,12 +20,12 @@ from __future__ import print_function import os import shutil -import subprocess -import sys import tempfile import testtools +import update + def _file_to_list(fname): with open(fname) as f: @@ -59,7 +59,7 @@ class UpdateTestPbr(testtools.TestCase): # now go call update and see what happens self.addCleanup(os.chdir, os.path.abspath(os.curdir)) os.chdir(self.dir) - subprocess.call([sys.executable, "update.py", "project_pbr"]) + update.main(["project_pbr"]) def test_requirements(self): reqs = _file_to_list(self.req_file) diff --git a/tests/test_update_suffix.py b/tests/test_update_suffix.py index f13106838e..526fb805eb 100644 --- a/tests/test_update_suffix.py +++ b/tests/test_update_suffix.py @@ -17,12 +17,12 @@ from __future__ import print_function import os import os.path import shutil -import subprocess -import sys import tempfile import testtools +import update + def _file_to_list(fname): with open(fname) as f: @@ -64,10 +64,8 @@ class UpdateTestWithSuffix(testtools.TestCase): # now go call update and see what happens self.addCleanup(os.chdir, os.path.abspath(os.curdir)) os.chdir(self.dir) - subprocess.call([sys.executable, "update.py", - "-o", "global", "project"]) - subprocess.call([sys.executable, "update.py", - "-o", "global", "project_with_oslo"]) + update.main(['-o', 'global', 'project']) + update.main(['-o', 'global', 'project_with_oslo']) def test_requirements(self): # this is the sanity check test diff --git a/update.py b/update.py index e5c6247a3b..01f1999b57 100644 --- a/update.py +++ b/update.py @@ -77,9 +77,9 @@ _REQS_HEADER = [ ] -def verbose(msg): +def verbose(msg, stdout): if VERBOSE: - print(msg) + stdout.write(msg + "\n") class Change(object): @@ -138,7 +138,7 @@ def _parse_reqs(filename): def _sync_requirements_file( - source_reqs, dest_path, suffix, softupdate, hacking): + source_reqs, dest_path, suffix, softupdate, hacking, stdout): dest_reqs = _readlines(dest_path) changes = [] # this is specifically for global-requirements gate jobs so we don't @@ -146,10 +146,9 @@ def _sync_requirements_file( if suffix: dest_path = "%s.%s" % (dest_path, suffix) - verbose("Syncing %s" % dest_path) + verbose("Syncing %s" % dest_path, stdout) with open(dest_path, 'w') as new_reqs: - # Check the instructions header if dest_reqs[:len(_REQS_HEADER)] != _REQS_HEADER: new_reqs.writelines(_REQS_HEADER) @@ -194,18 +193,20 @@ def _sync_requirements_file( # devstack jobs that might have legitimate reasons to # override. For those we support NON_STANDARD_REQS=1 # environment variable to turn this into a warning only. - print("'%s' is not in global-requirements.txt" % old_pip) + stdout.write( + "'%s' is not in global-requirements.txt\n" % old_pip) if os.getenv('NON_STANDARD_REQS', '0') != '1': - sys.exit(1) + raise Exception("nonstandard requirement present.") # always print out what we did if we did a thing if changes: - print("Version change for: %s" % ", ".join([x.name for x in changes])) - print("Updated %s:" % dest_path) + stdout.write( + "Version change for: %s\n" % ", ".join([x.name for x in changes])) + stdout.write("Updated %s:\n" % dest_path) for change in changes: - print(" %s" % change) + stdout.write(" %s\n" % change) -def _copy_requires(suffix, softupdate, hacking, dest_dir): +def _copy_requires(suffix, softupdate, hacking, dest_dir, stdout): """Copy requirements files.""" source_reqs = _parse_reqs('global-requirements.txt') @@ -222,10 +223,10 @@ def _copy_requires(suffix, softupdate, hacking, dest_dir): dest_path = os.path.join(dest_dir, dest) if os.path.exists(dest_path): _sync_requirements_file( - source_reqs, dest_path, suffix, softupdate, hacking) + source_reqs, dest_path, suffix, softupdate, hacking, stdout) -def _write_setup_py(dest_path): +def _write_setup_py(dest_path, stdout): target_setup_py = os.path.join(dest_path, 'setup.py') setup_cfg = os.path.join(dest_path, 'setup.cfg') # If it doesn't have a setup.py, then we don't want to update it @@ -234,25 +235,14 @@ def _write_setup_py(dest_path): has_pbr = 'pbr' in _read(target_setup_py) if has_pbr: if 'name = pbr' not in _read(setup_cfg): - verbose("Syncing setup.py") + verbose("Syncing setup.py", stdout) # We only want to sync things that are up to date # with pbr mechanics with open(target_setup_py, 'w') as setup_file: setup_file.write(_setup_py_text) -def main(options, args): - if len(args) != 1: - print("Must specify directory to update") - sys.exit(1) - global VERBOSE - VERBOSE = options.verbose - _copy_requires(options.suffix, options.softupdate, options.hacking, - args[0]) - _write_setup_py(args[0]) - - -if __name__ == "__main__": +def main(argv=None, stdout=None): parser = optparse.OptionParser() parser.add_option("-o", "--output-suffix", dest="suffix", default="", help="output suffix for updated files (i.e. .global)") @@ -265,5 +255,18 @@ if __name__ == "__main__": parser.add_option("-v", "--verbose", dest="verbose", action="store_true", help="Add further verbosity to output") - (options, args) = parser.parse_args() - main(options, args) + options, args = parser.parse_args(argv) + if len(args) != 1: + print("Must specify directory to update") + raise Exception("Must specify one and only one directory to update.") + global VERBOSE + VERBOSE = options.verbose + if stdout is None: + stdout = sys.stdout + _copy_requires(options.suffix, options.softupdate, options.hacking, + args[0], stdout=stdout) + _write_setup_py(args[0], stdout=stdout) + + +if __name__ == "__main__": + main()