From 2ee3cbb721b062c4f3fca0ad95d74bf0207bfb46 Mon Sep 17 00:00:00 2001 From: Tony Breeds Date: Fri, 20 May 2016 11:24:58 +1000 Subject: [PATCH] Check that the argument is in fact a directory If you mistakenly pass a path that isn't a directory, or doens't exist, you get a traceback that ends with: --- File "/Users/tony8129/projects/openstack/openstack/requirements/openstack_requirements/cmds/update.py", line 229, in _copy_requires dest_path, project.merge_setup_cfg(proj['setup.cfg'], output_extras))) KeyError: 'setup.cfg' ERROR: InvocationError: '.../openstack/requirements/.tox/update/bin/update-requirements -H -s ./nova' --- This is a little confusing. This patch checks that the arg is a directory so a more explicit error can be generated. Change-Id: I9f3f43d5c9b90746bbf2d651ee4cb37afade1528 --- openstack_requirements/cmds/update.py | 3 +++ openstack_requirements/tests/test_update.py | 21 +++++++++++++++++++-- test-requirements.txt | 1 + 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/openstack_requirements/cmds/update.py b/openstack_requirements/cmds/update.py index 9017fb48db..118765e79b 100644 --- a/openstack_requirements/cmds/update.py +++ b/openstack_requirements/cmds/update.py @@ -263,6 +263,9 @@ def main(argv=None, stdout=None, _worker=None): if len(args) != 1: print("Must specify directory to update") raise Exception("Must specify one and only one directory to update.") + if not os.path.isdir(args[0]): + print("%s is not a directory." % (args[0])) + raise Exception("%s is not a directory." % (args[0])) if stdout is None: stdout = sys.stdout if _worker is None: diff --git a/openstack_requirements/tests/test_update.py b/openstack_requirements/tests/test_update.py index 266191c2ce..038216d75e 100644 --- a/openstack_requirements/tests/test_update.py +++ b/openstack_requirements/tests/test_update.py @@ -19,6 +19,7 @@ import sys import textwrap import fixtures +import mock import testscenarios import testtools from testtools import matchers @@ -216,7 +217,8 @@ Syncing setup.py class TestMain(testtools.TestCase): - def test_smoke(self): + @mock.patch('os.path.isdir', return_value=True) + def test_smoke(self, mock_isdir): def check_params( root, source, suffix, softupdate, hacking, stdout, verbose, non_std_reqs): @@ -232,14 +234,29 @@ class TestMain(testtools.TestCase): with fixtures.EnvironmentVariable('NON_STANDARD_REQS', '1'): update.main( ['--source', '/dev/null', '/dev/zero'], _worker=check_params) + self.expectThat(mock_isdir.called, matchers.Equals(True)) - def test_suffix(self): + @mock.patch('os.path.isdir', return_value=True) + def test_suffix(self, mock_isdir): def check_params( root, source, suffix, softupdate, hacking, stdout, verbose, non_std_reqs): self.expectThat(suffix, matchers.Equals('global')) update.main(['-o', 'global', '/dev/zero'], _worker=check_params) + self.expectThat(mock_isdir.called, matchers.Equals(True)) + + def test_isdirectory(self): + def never_called( + root, source, suffix, softupdate, hacking, stdout, verbose, + non_std_reqs): + self.expectThat(False, matchers.Equals(True), + message=("update.main() should riase an " + "excpetion before getting here")) + + with testtools.ExpectedException(Exception, + "/dev/zero is not a directory"): + update.main(['/dev/zero'], _worker=never_called) class TestSyncRequirementsFile(testtools.TestCase): diff --git a/test-requirements.txt b/test-requirements.txt index 31b793b229..d01ce8fe70 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -10,6 +10,7 @@ testscenarios>=0.4 # Apache-2.0/BSD testtools>=1.4.0 # MIT virtualenv # MIT setuptools>=16.0 # PSF/ZPL +mock>=1.2 # BSD # this is required for the docs build jobs sphinx!=1.2.0,!=1.3b1,<1.3,>=1.1.2 # BSD