Skip to content

Commit

Permalink
Fix unit tests failing when OS_ env vars are set
Browse files Browse the repository at this point in the history
tests/unit/test_shell.py:TestParsing tests can fail if
there are OS_* variables set in the environment. There
is already code in the test setUp to remove ST_* variables,
so we should do the same for OS_* variables.

This patch also changes the mechanism used to remove and
then restore any unwanted variables found in os.environ.

The existing setUp() takes a copy of os.environ and then
deletes any ST_* variables in the original. In tearDown() it
sets os.environ as the copy. However, the environ imported
into shell.py remains pointing to the original os.environ
object. So after the first call to tearDown, subsequent
mocking of os.environ has no effect on shell.environ.
This renders some of the tests ineffective e.g.
test_insufficient_env_vars_v3 is not actually setting any
vars in shell.environ.

The issue can be provoked by repeating a test:
nosetests -w tests/unit/ test_shell.py:TestParsing.test_args_v3 \
 test_shell.py:TestParsing.test_args_v3

The test will pass first time and fail second time.

Change-Id: I5d100f81115e74878d510326acb5777e6a3626c8
  • Loading branch information
Alistair Coles committed Sep 25, 2014
1 parent 8f1b394 commit b13c84b
Showing 1 changed file with 5 additions and 4 deletions.
9 changes: 5 additions & 4 deletions tests/unit/test_shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -399,14 +399,15 @@ class TestParsing(unittest.TestCase):

def setUp(self):
super(TestParsing, self).setUp()
self._orig_environ = os.environ.copy()
self._environ_vars = {}
keys = os.environ.keys()
for k in keys:
if k in ('ST_KEY', 'ST_USER', 'ST_AUTH'):
del os.environ[k]
if (k in ('ST_KEY', 'ST_USER', 'ST_AUTH')
or k.startswith('OS_')):
self._environ_vars[k] = os.environ.pop(k)

def tearDown(self):
os.environ = self._orig_environ
os.environ.update(self._environ_vars)

def _make_fake_command(self, result):
def fake_command(parser, args, thread_manager):
Expand Down

0 comments on commit b13c84b

Please sign in to comment.