From 5300ffac7f1e4f77fcb1ba52083d4a7ba7183467 Mon Sep 17 00:00:00 2001 From: alalazo Date: Sun, 12 Jun 2016 15:11:26 +0200 Subject: [PATCH] qa : fixed flak8 checks --- lib/spack/spack/environment.py | 25 +++++++++++++------ lib/spack/spack/test/environment.py | 37 ++++++++++++++++++++++------- 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py index 87759b36e1..623bfa6ed2 100644 --- a/lib/spack/spack/environment.py +++ b/lib/spack/spack/environment.py @@ -24,13 +24,14 @@ ############################################################################## import collections import inspect +import json import os import os.path import subprocess -import shlex -import json + class NameModifier(object): + def __init__(self, name, **kwargs): self.name = name self.args = {'name': name} @@ -38,6 +39,7 @@ def __init__(self, name, **kwargs): class NameValueModifier(object): + def __init__(self, name, value, **kwargs): self.name = name self.value = value @@ -47,23 +49,27 @@ def __init__(self, name, value, **kwargs): class SetEnv(NameValueModifier): + def execute(self): os.environ[self.name] = str(self.value) class UnsetEnv(NameModifier): + def execute(self): # Avoid throwing if the variable was not set os.environ.pop(self.name, None) class SetPath(NameValueModifier): + def execute(self): string_path = concatenate_paths(self.value, separator=self.separator) os.environ[self.name] = string_path class AppendPath(NameValueModifier): + def execute(self): environment_value = os.environ.get(self.name, '') directories = environment_value.split( @@ -73,6 +79,7 @@ def execute(self): class PrependPath(NameValueModifier): + def execute(self): environment_value = os.environ.get(self.name, '') directories = environment_value.split( @@ -82,6 +89,7 @@ def execute(self): class RemovePath(NameValueModifier): + def execute(self): environment_value = os.environ.get(self.name, '') directories = environment_value.split( @@ -270,7 +278,7 @@ def from_sourcing_files(*args, **kwargs): shell = '{shell}'.format(**info) shell_options = '{shell_options}'.format(**info) source_file = '{source_command} {file} {concatenate_on_success}' - dump_environment = 'python -c "import os, json; print json.dumps(dict(os.environ))"' + dump_environment = 'python -c "import os, json; print json.dumps(dict(os.environ))"' # NOQA: ignore=E501 # Construct the command that will be executed command = [source_file.format(file=file, **info) for file in args] command.append(dump_environment) @@ -282,7 +290,8 @@ def from_sourcing_files(*args, **kwargs): ] # Try to source all the files, - proc = subprocess.Popen(command, stdout=subprocess.PIPE, env=os.environ) + proc = subprocess.Popen( + command, stdout=subprocess.PIPE, env=os.environ) proc.wait() if proc.returncode != 0: raise RuntimeError('sourcing files returned a non-zero exit code') @@ -306,10 +315,12 @@ def from_sourcing_files(*args, **kwargs): for x in unset_variables: env.unset(x) # Variables that have been modified - common_variables = set(this_environment).intersection(set(after_source_env)) - modified_variables = [x for x in common_variables if this_environment[x] != after_source_env[x]] + common_variables = set(this_environment).intersection( + set(after_source_env)) + modified_variables = [x for x in common_variables if this_environment[x] != after_source_env[x]] # NOQA: ignore=E501 for x in modified_variables: - # TODO : we may be smarter here, and try to parse if we could compose append_path + # TODO : we may be smarter here, and try to parse + # TODO : if we could compose append_path # TODO : and prepend_path to modify the value env.set(x, after_source_env[x]) return env diff --git a/lib/spack/spack/test/environment.py b/lib/spack/spack/test/environment.py index 34b4485f8e..31a85f9ee3 100644 --- a/lib/spack/spack/test/environment.py +++ b/lib/spack/spack/test/environment.py @@ -31,12 +31,13 @@ class EnvironmentTest(unittest.TestCase): + def setUp(self): os.environ.clear() os.environ['UNSET_ME'] = 'foo' os.environ['EMPTY_PATH_LIST'] = '' os.environ['PATH_LIST'] = '/path/second:/path/third' - os.environ['REMOVE_PATH_LIST'] = '/a/b:/duplicate:/a/c:/remove/this:/a/d:/duplicate/:/f/g' + os.environ['REMOVE_PATH_LIST'] = '/a/b:/duplicate:/a/c:/remove/this:/a/d:/duplicate/:/f/g' # NOQA: ignore=E501 def test_set(self): env = EnvironmentModifications() @@ -77,9 +78,18 @@ def test_path_manipulation(self): env.remove_path('REMOVE_PATH_LIST', '/duplicate/') env.apply_modifications() - self.assertEqual('/path/first:/path/second:/path/third:/path/last', os.environ['PATH_LIST']) - self.assertEqual('/path/first:/path/middle:/path/last', os.environ['EMPTY_PATH_LIST']) - self.assertEqual('/path/first:/path/middle:/path/last', os.environ['NEWLY_CREATED_PATH_LIST']) + self.assertEqual( + '/path/first:/path/second:/path/third:/path/last', + os.environ['PATH_LIST'] + ) + self.assertEqual( + '/path/first:/path/middle:/path/last', + os.environ['EMPTY_PATH_LIST'] + ) + self.assertEqual( + '/path/first:/path/middle:/path/last', + os.environ['NEWLY_CREATED_PATH_LIST'] + ) self.assertEqual('/a/b:/a/c:/a/d:/f/g', os.environ['REMOVE_PATH_LIST']) def test_extra_arguments(self): @@ -100,7 +110,8 @@ def test_extend(self): assert x is y def test_source_files(self): - datadir = join_path(spack_root, 'lib', 'spack', 'spack', 'test', 'data') + datadir = join_path(spack_root, 'lib', 'spack', + 'spack', 'test', 'data') files = [ join_path(datadir, 'sourceme_first.sh'), join_path(datadir, 'sourceme_second.sh') @@ -115,7 +126,8 @@ def test_source_files(self): self.assertEqual(modifications['NEW_VAR'][0].value, 'new') # Unset variables self.assertEqual(len(modifications['EMPTY_PATH_LIST']), 1) - self.assertTrue(isinstance(modifications['EMPTY_PATH_LIST'][0], UnsetEnv)) + self.assertTrue(isinstance( + modifications['EMPTY_PATH_LIST'][0], UnsetEnv)) # Modified variables self.assertEqual(len(modifications['UNSET_ME']), 1) self.assertTrue(isinstance(modifications['UNSET_ME'][0], SetEnv)) @@ -123,12 +135,19 @@ def test_source_files(self): self.assertEqual(len(modifications['PATH_LIST']), 1) self.assertTrue(isinstance(modifications['PATH_LIST'][0], SetEnv)) - self.assertEqual(modifications['PATH_LIST'][0].value, '/path/first:/path/second:/path/third:/path/fourth') + self.assertEqual( + modifications['PATH_LIST'][0].value, + '/path/first:/path/second:/path/third:/path/fourth' + ) # TODO : with reference to the TODO in spack/environment.py # TODO : remove the above and insert # self.assertEqual(len(modifications['PATH_LIST']), 2) - # self.assertTrue(isinstance(modifications['PATH_LIST'][0], PrependPath)) + # self.assertTrue( + # isinstance(modifications['PATH_LIST'][0], PrependPath) + # ) # self.assertEqual(modifications['PATH_LIST'][0].value, '/path/first') - # self.assertTrue(isinstance(modifications['PATH_LIST'][1], AppendPath)) + # self.assertTrue( + # isinstance(modifications['PATH_LIST'][1], AppendPath) + # ) # self.assertEqual(modifications['PATH_LIST'][1].value, '/path/fourth')