From 8f10c89c4fdffc761bc36d0320ae3fd73378abac Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Sat, 5 Oct 2019 22:43:43 +0200 Subject: [PATCH 1/3] Allow setting num_tasks* variables directly in jobs - This allows to set them from their `RegressionTest` counterparts just before the run phase, allowing as a result user tests to set these variables late enough. - `RegressionTest`'s `num_tasks` are now set again from the job in case of flexible tests. --- reframe/core/pipeline.py | 18 +++++--- reframe/core/schedulers/__init__.py | 66 +++++++++-------------------- reframe/core/schedulers/local.py | 2 +- reframe/core/schedulers/pbs.py | 6 +-- unittests/test_schedulers.py | 48 ++++++++++----------- 5 files changed, 60 insertions(+), 80 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index f50414a2ed..950292b762 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -935,13 +935,6 @@ def _setup_job(self, **job_opts): name='rfm_%s_job' % self.name, launcher=launcher_type(), workdir=self._stagedir, - num_tasks=self.num_tasks, - num_tasks_per_node=self.num_tasks_per_node, - num_tasks_per_core=self.num_tasks_per_core, - num_tasks_per_socket=self.num_tasks_per_socket, - num_cpus_per_task=self.num_cpus_per_task, - use_smt=self.use_multithreading, - time_limit=self.time_limit, sched_access=self._current_partition.access, sched_exclusive_access=self.exclusive_access, **job_opts) @@ -1102,6 +1095,13 @@ def run(self): if not self.current_system or not self._current_partition: raise PipelineError('no system or system partition is set') + self.job.num_tasks = self.num_tasks + self.job.num_tasks_per_node = self.num_tasks_per_node + self.job.num_tasks_per_core = self.num_tasks_per_core + self.job.num_cpus_per_task = self.num_cpus_per_task + self.job.use_smt = self.use_multithreading + self.job.time_limit = self.time_limit + exec_cmd = [self.job.launcher.run_command(self.job), self.executable, *self.executable_opts] commands = [*self.pre_run, ' '.join(exec_cmd), *self.post_run] @@ -1120,6 +1120,10 @@ def run(self): ('pid' if self.is_local() else 'jobid', self._job.jobid)) self.logger.debug(msg) + # Update num_tasks if test is flexible + if self.job.sched_flex_alloc_tasks: + self.num_tasks = self.job.num_tasks + def poll(self): '''Poll the test's state. diff --git a/reframe/core/schedulers/__init__.py b/reframe/core/schedulers/__init__.py index 6543a2750a..624e81466f 100644 --- a/reframe/core/schedulers/__init__.py +++ b/reframe/core/schedulers/__init__.py @@ -21,6 +21,18 @@ class Job(abc.ABC): Users may not create jobs directly. ''' + num_tasks = fields.TypedField('num_tasks', int) + num_tasks_per_node = fields.TypedField('num_tasks_per_node', + int, type(None)) + num_tasks_per_core = fields.TypedField('num_tasks_per_core', + int, type(None)) + num_tasks_per_socket = fields.TypedField('num_tasks_per_socket', + int, type(None)) + num_cpus_per_tasks = fields.TypedField('num_cpus_per_task', + int, type(None)) + use_smt = fields.TypedField('use_smt', bool, type(None)) + time_limit = fields.TimerField('time_limit', type(None)) + #: Options to be passed to the backend job scheduler. #: #: :type: :class:`List[str]` @@ -65,21 +77,21 @@ def __init__(self, sched_options=[]): # Mutable fields + self.num_tasks = num_tasks + self.num_tasks_per_node = num_tasks_per_node + self.num_tasks_per_core = num_tasks_per_core + self.num_tasks_per_socket = num_tasks_per_socket + self.num_cpus_per_task = num_cpus_per_task + self.use_smt = use_smt + self.time_limit = time_limit self.options = list(sched_options) self.launcher = launcher self._name = name self._workdir = workdir - self._num_tasks = num_tasks - self._num_tasks_per_node = num_tasks_per_node - self._num_tasks_per_core = num_tasks_per_core - self._num_tasks_per_socket = num_tasks_per_socket - self._num_cpus_per_task = num_cpus_per_task - self._use_smt = use_smt self._script_filename = script_filename or '%s.sh' % name self._stdout = stdout or '%s.out' % name self._stderr = stderr or '%s.err' % name - self._time_limit = time_limit self._nodelist = None # Backend scheduler related information @@ -122,18 +134,6 @@ def name(self): def workdir(self): return self._workdir - @property - def num_tasks(self): - '''The number of tasks assigned to this job. - - This attribute is useful in a flexible regression test for determining - the actual number of tasks that ReFrame assigned to the test. - - For more information on flexible task allocation, please refer to the - `tutorial `__. - ''' - return self._num_tasks - @property def script_filename(self): return self._script_filename @@ -146,30 +146,6 @@ def stdout(self): def stderr(self): return self._stderr - @property - def time_limit(self): - return self._time_limit - - @property - def num_cpus_per_task(self): - return self._num_cpus_per_task - - @property - def num_tasks_per_core(self): - return self._num_tasks_per_core - - @property - def num_tasks_per_node(self): - return self._num_tasks_per_node - - @property - def num_tasks_per_socket(self): - return self._num_tasks_per_socket - - @property - def use_smt(self): - return self._use_smt - @property def sched_flex_alloc_tasks(self): return self._sched_flex_alloc_tasks @@ -222,9 +198,9 @@ def prepare(self, commands, environs=None, **gen_opts): 'required %s, found %s' % (nodes_required, nodes_found)) - self._num_tasks = guessed_num_tasks + self.num_tasks = guessed_num_tasks getlogger().debug('flex_alloc_tasks: setting num_tasks to %s' % - self._num_tasks) + self.num_tasks) with shell.generate_script(self.script_filename, **gen_opts) as builder: diff --git a/reframe/core/schedulers/local.py b/reframe/core/schedulers/local.py index 4e40ef1e89..d0e3be399f 100644 --- a/reframe/core/schedulers/local.py +++ b/reframe/core/schedulers/local.py @@ -115,7 +115,7 @@ def cancel(self): # Set the time limit to the grace period and let wait() do the final # killing - self._time_limit = (0, 0, self.cancel_grace_period) + self.time_limit = (0, 0, self.cancel_grace_period) self.wait() def wait(self): diff --git a/reframe/core/schedulers/pbs.py b/reframe/core/schedulers/pbs.py index d5d98b6d2c..8b26fc3da3 100644 --- a/reframe/core/schedulers/pbs.py +++ b/reframe/core/schedulers/pbs.py @@ -34,9 +34,9 @@ def __init__(self, *args, **kwargs): self._pbs_server = None def _emit_lselect_option(self): - num_tasks_per_node = self._num_tasks_per_node or 1 - num_cpus_per_task = self._num_cpus_per_task or 1 - num_nodes = self._num_tasks // num_tasks_per_node + num_tasks_per_node = self.num_tasks_per_node or 1 + num_cpus_per_task = self.num_cpus_per_task or 1 + num_nodes = self.num_tasks // num_tasks_per_node num_cpus_per_node = num_tasks_per_node * num_cpus_per_task select_opt = '-l select=%s:mpiprocs=%s:ncpus=%s' % (num_nodes, num_tasks_per_node, diff --git a/unittests/test_schedulers.py b/unittests/test_schedulers.py index 9fd52a830f..68e247c392 100644 --- a/unittests/test_schedulers.py +++ b/unittests/test_schedulers.py @@ -81,22 +81,22 @@ def assertScriptSanity(self, script_file): def setup_job(self): # Mock up a job submission - self.testjob._time_limit = (0, 5, 0) - self.testjob._num_tasks = 16 - self.testjob._num_tasks_per_node = 2 - self.testjob._num_tasks_per_core = 1 - self.testjob._num_tasks_per_socket = 1 - self.testjob._num_cpus_per_task = 18 - self.testjob._use_smt = True + self.testjob.time_limit = (0, 5, 0) + self.testjob.num_tasks = 16 + self.testjob.num_tasks_per_node = 2 + self.testjob.num_tasks_per_core = 1 + self.testjob.num_tasks_per_socket = 1 + self.testjob.num_cpus_per_task = 18 + self.testjob.use_smt = True + self.testjob.options = ['--gres=gpu:4', + '#DW jobdw capacity=100GB', + '#DW stage_in source=/foo'] self.testjob._sched_nodelist = 'nid000[00-17]' self.testjob._sched_exclude_nodelist = 'nid00016' self.testjob._sched_partition = 'foo' self.testjob._sched_reservation = 'bar' self.testjob._sched_account = 'spam' self.testjob._sched_exclusive_access = True - self.testjob.options = ['--gres=gpu:4', - '#DW jobdw capacity=100GB', - '#DW stage_in source=/foo'] def test_prepare(self): self.testjob.prepare(self.commands, self.environs) @@ -115,7 +115,7 @@ def test_submit(self): def test_submit_timelimit(self, check_elapsed_time=True): self.setup_user() self.parallel_cmd = 'sleep 10' - self.testjob._time_limit = (0, 0, 2) + self.testjob.time_limit = (0, 0, 2) self.testjob.prepare(self.commands, self.environs) t_job = datetime.now() self.testjob.submit() @@ -171,7 +171,7 @@ def test_no_empty_lines_in_preamble(self): self.assertNotEqual(l, '') def test_guess_num_tasks(self): - self.testjob._num_tasks = 0 + self.testjob.num_tasks = 0 with self.assertRaises(NotImplementedError): self.testjob.guess_num_tasks() @@ -224,7 +224,7 @@ def test_cancel_with_grace(self): self.parallel_cmd = 'sleep 5 &' self.pre_run = ['trap -- "" TERM'] self.post_run = ['echo $!', 'wait'] - self.testjob._time_limit = (0, 1, 0) + self.testjob.time_limit = (0, 1, 0) self.testjob.cancel_grace_period = 2 self.testjob.prepare(self.commands, self.environs) @@ -347,21 +347,21 @@ def test_prepare_no_exclusive(self): def test_prepare_no_smt(self): self.setup_job() - self.testjob._use_smt = None + self.testjob.use_smt = None super().test_prepare() with open(self.testjob.script_filename) as fp: self.assertIsNone(re.search(r'--hint', fp.read())) def test_prepare_with_smt(self): self.setup_job() - self.testjob._use_smt = True + self.testjob.use_smt = True super().test_prepare() with open(self.testjob.script_filename) as fp: self.assertIsNotNone(re.search(r'--hint=multithread', fp.read())) def test_prepare_without_smt(self): self.setup_job() - self.testjob._use_smt = False + self.testjob.use_smt = False super().test_prepare() with open(self.testjob.script_filename) as fp: self.assertIsNotNone(re.search(r'--hint=nomultithread', fp.read())) @@ -382,7 +382,7 @@ def test_cancel(self): self.assertEqual(self.testjob.state, 'CANCELLED') def test_guess_num_tasks(self): - self.testjob._num_tasks = 0 + self.testjob.num_tasks = 0 self.testjob._sched_flex_alloc_tasks = 'all' # monkey patch `get_all_nodes()` to simulate extraction of # slurm nodes through the use of `scontrol show` @@ -466,7 +466,7 @@ def test_prepare(self): def test_prepare_no_cpus(self): self.setup_job() - self.testjob._num_cpus_per_task = None + self.testjob.num_cpus_per_task = None self.testjob.options += ['mem=100GB', 'cpu_type=haswell'] super().test_prepare() num_nodes = self.testjob.num_tasks // self.testjob.num_tasks_per_node @@ -614,8 +614,8 @@ def setUp(self): # of the default partition self.testjob._get_default_partition = lambda: 'pdef' self.testjob._sched_flex_alloc_tasks = 'all' - self.testjob._num_tasks_per_node = 4 - self.testjob._num_tasks = 0 + self.testjob.num_tasks_per_node = 4 + self.testjob.num_tasks = 0 def tearDown(self): os_ext.rmtree(self.workdir) @@ -748,26 +748,26 @@ def test_exclude_nodes_opt(self): self.assertEqual(self.testjob.num_tasks, 8) def test_no_num_tasks_per_node(self): - self.testjob._num_tasks_per_node = None + self.testjob.num_tasks_per_node = None self.testjob.options = ['-C f1,f2', '--partition=p1,p2'] self.prepare_job() self.assertEqual(self.testjob.num_tasks, 1) def test_not_enough_idle_nodes(self): self.testjob._sched_flex_alloc_tasks = 'idle' - self.testjob._num_tasks = -12 + self.testjob.num_tasks = -12 with self.assertRaises(JobError): self.prepare_job() def test_not_enough_nodes_constraint_partition(self): self.testjob.options = ['-C f1,f2', '--partition=p1,p2'] - self.testjob._num_tasks = -8 + self.testjob.num_tasks = -8 with self.assertRaises(JobError): self.prepare_job() def test_enough_nodes_constraint_partition(self): self.testjob.options = ['-C f1,f2', '--partition=p1,p2'] - self.testjob._num_tasks = -4 + self.testjob.num_tasks = -4 self.prepare_job() self.assertEqual(self.testjob.num_tasks, 4) From a0abcd0905c7655d27e2d7159a8ab269a1c6ed0f Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Mon, 7 Oct 2019 16:46:36 +0200 Subject: [PATCH 2/3] Change the way environments are loaded - `Environment` class is now really immutable and does not carry any logic about loading the environment. - Environments are loaded by a free function that takes care of the actual load details and the necessary shell instructions needed to load the environment. - Unloading an environment is not explicitly supported. It is supported though through environment snapshots. The `load()` function returns a snapshot of the environment before loading the current one. The users can restore this snapshot to effectively unload the environment. - Framework was adapted to use the new environment API. - An important change is also that the test's user environment (i.e., modules and variables) are re-read every time that they are needed (i.e., at the build time and at the run time). This now allows to have different environments for compilation and running. --- reframe/core/environments.py | 169 +++++------------ reframe/core/pipeline.py | 60 ++---- reframe/core/schedulers/__init__.py | 5 +- reframe/frontend/cli.py | 11 +- reframe/frontend/executors/__init__.py | 12 +- reframe/frontend/executors/policies.py | 4 +- unittests/test_cli.py | 6 +- unittests/test_environments.py | 248 +++++++++---------------- unittests/test_modules.py | 6 +- unittests/test_pipeline.py | 3 - 10 files changed, 166 insertions(+), 358 deletions(-) diff --git a/reframe/core/environments.py b/reframe/core/environments.py index 8e657fb0dd..beb6d6a6b3 100644 --- a/reframe/core/environments.py +++ b/reframe/core/environments.py @@ -13,7 +13,6 @@ class Environment: It is simply a collection of modules to be loaded and environment variables to be set when this environment is loaded by the framework. - Users may not create or modify directly environments. ''' name = fields.TypedField('name', typ.Str[r'(\w|-)+']) modules = fields.TypedField('modules', typ.List[str]) @@ -23,11 +22,6 @@ def __init__(self, name, modules=[], variables=[]): self._name = name self._modules = list(modules) self._variables = collections.OrderedDict(variables) - self._loaded = False - self._saved_variables = {} - self._conflicted = [] - self._preloaded = set() - self._module_ops = [] @property def name(self): @@ -63,100 +57,6 @@ def is_loaded(self): all(os.environ.get(k, None) == os_ext.expandvars(v) for k, v in self._variables.items())) - def load(self): - # conflicted module list must be filled at the time of load - rt = runtime() - for m in self._modules: - if rt.modules_system.is_module_loaded(m): - self._preloaded.add(m) - - conflicted = rt.modules_system.load_module(m, force=True) - for c in conflicted: - self._module_ops.append(('u', c)) - - self._module_ops.append(('l', m)) - self._conflicted += conflicted - - for k, v in self._variables.items(): - if k in os.environ: - self._saved_variables[k] = os.environ[k] - - os.environ[k] = os_ext.expandvars(v) - - self._loaded = True - - def unload(self): - if not self._loaded: - return - - for k, v in self._variables.items(): - if k in self._saved_variables: - os.environ[k] = self._saved_variables[k] - elif k in os.environ: - del os.environ[k] - - # Unload modules in reverse order - for m in reversed(self._modules): - if m not in self._preloaded: - runtime().modules_system.unload_module(m) - - # Reload the conflicted packages, previously removed - for m in self._conflicted: - runtime().modules_system.load_module(m) - - self._loaded = False - - def emit_load_commands(self): - rt = runtime() - emit_fn = { - 'l': rt.modules_system.emit_load_commands, - 'u': rt.modules_system.emit_unload_commands - } - module_ops = self._module_ops or [('l', m) for m in self._modules] - - # Emit module commands - ret = [] - for op, m in module_ops: - ret += emit_fn[op](m) - - # Emit variable set commands - for k, v in self._variables.items(): - ret.append('export %s=%s' % (k, v)) - - return ret - - def emit_unload_commands(self): - rt = runtime() - - # Invert the logic of module operations, since we are unloading the - # environment - emit_fn = { - 'l': rt.modules_system.emit_unload_commands, - 'u': rt.modules_system.emit_load_commands - } - - ret = [] - for var in self._variables.keys(): - ret.append('unset %s' % var) - - if self._module_ops: - module_ops = reversed(self._module_ops) - else: - module_ops = (('l', m) for m in reversed(self._modules)) - - for op, m in module_ops: - ret += emit_fn[op](m) - - return ret - - def __eq__(self, other): - if not isinstance(other, type(self)): - return NotImplemented - - return (self._name == other._name and - set(self._modules) == set(other._modules) and - self._variables == other._variables) - def details(self): '''Return a detailed description of this environment.''' variables = '\n'.join(' '*8 + '- %s=%s' % (k, v) @@ -168,6 +68,14 @@ def details(self): ] return '\n'.join(lines) + def __eq__(self, other): + if not isinstance(other, type(self)): + return NotImplemented + + return (self._name == other._name and + set(self._modules) == set(other._modules) and + self._variables == other._variables) + def __str__(self): return self.name @@ -177,44 +85,51 @@ def __repr__(self): self.modules, self.variables) -def swap_environments(src, dst): - src.unload() - dst.load() - - -class EnvironmentSnapshot(Environment): +class _EnvironmentSnapshot(Environment): def __init__(self, name='env_snapshot'): - self._name = name - self._modules = runtime().modules_system.loaded_modules() - self._variables = dict(os.environ) - self._conflicted = [] + super().__init__(name, + runtime().modules_system.loaded_modules(), + os.environ.items()) - def load(self): + def restore(self): + '''Restore this environment snapshot.''' os.environ.clear() os.environ.update(self._variables) - self._loaded = True - @property - def is_loaded(self): - raise NotImplementedError('is_loaded is not a valid property ' - 'of an environment snapshot') - def unload(self): - raise NotImplementedError('cannot unload an environment snapshot') +def snapshot(): + '''Create an environment snapshot''' + return _EnvironmentSnapshot() + +def load(*environs): + '''Load environments in the current Python context. -class save_environment: - '''A context manager for saving and restoring the current environment.''' + Returns a tuple containing a snapshot of the environment at entry to this + function and a list of shell commands required to load ``environs``. + ''' + env_snapshot = snapshot() + commands = [] + rt = runtime() + for env in environs: + for m in env.modules: + conflicted = rt.modules_system.load_module(m, force=True) + for c in conflicted: + commands += rt.modules_system.emit_unload_commands(c) + + commands += rt.modules_system.emit_load_commands(m) + + for k, v in env.variables.items(): + os.environ[k] = os_ext.expandvars(v) + commands.append('export %s=%s' % (k, v)) - def __init__(self): - self.environ_save = EnvironmentSnapshot() + return env_snapshot, commands - def __enter__(self): - return self.environ_save - def __exit__(self, exc_type, exc_value, traceback): - # Restore the environment and propagate any exception thrown - self.environ_save.load() +def emit_load_commands(*environs): + env_snapshot, commands = load(*environs) + env_snapshot.restore() + return commands class ProgEnvironment(Environment): diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 950292b762..18b6e71a6c 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -13,6 +13,7 @@ import os import shutil +import reframe.core.environments as env import reframe.core.fields as fields import reframe.core.logging as logging import reframe.core.runtime as rt @@ -21,7 +22,6 @@ import reframe.utility.typecheck as typ from reframe.core.buildsystems import BuildSystemField from reframe.core.deferrable import deferrable, _DeferredExpression, evaluate -from reframe.core.environments import Environment, EnvironmentSnapshot from reframe.core.exceptions import (BuildError, DependencyError, PipelineError, SanityError, PerformanceError) @@ -567,11 +567,10 @@ class RegressionTest(metaclass=RegressionTestMeta): _current_partition = fields.TypedField('_current_partition', SystemPartition, type(None)) _current_environ = fields.TypedField('_current_environ', - Environment, type(None)) - _user_environ = fields.TypedField('_user_environ', Environment, type(None)) + env.Environment, type(None)) + _cdt_environ = fields.TypedField('_cdt_environ', env.Environment) _job = fields.TypedField('_job', Job, type(None)) _build_job = fields.TypedField('_build_job', Job, type(None)) - _cdt_environ = fields.TypedField('_cdt_environ', Environment) def __new__(cls, *args, **kwargs): obj = super().__new__(cls) @@ -650,7 +649,6 @@ def _rfm_init(self, name=None, prefix=None): # Runtime information of the test self._current_partition = None self._current_environ = None - self._user_environ = None # Associated job self._job = None @@ -677,7 +675,7 @@ def _rfm_init(self, name=None, prefix=None): self._case = None if rt.runtime().non_default_craype: - self._cdt_environ = Environment( + self._cdt_environ = env.Environment( name='__rfm_cdt_environ', variables={ 'LD_LIBRARY_PATH': '$CRAY_LD_LIBRARY_PATH:$LD_LIBRARY_PATH' @@ -685,7 +683,7 @@ def _rfm_init(self, name=None, prefix=None): ) else: # Just an empty environment - self._cdt_environ = Environment('__rfm_cdt_environ') + self._cdt_environ = env.Environment('__rfm_cdt_environ') # Export read-only views to interesting fields @property @@ -870,30 +868,6 @@ def is_local(self): return self.local or self._current_partition.scheduler.is_local - def _setup_environ(self, environ): - '''Setup the current environment and load it.''' - - self._current_environ = environ - - # Set up user environment - self._user_environ = Environment(type(self).__name__, self.modules, - self.variables.items()) - - # Temporarily load the test's environment to record the actual module - # load/unload sequence - environ_save = EnvironmentSnapshot() - # First load the local environment of the partition - self.logger.debug('loading environment for the current partition') - self._current_partition.local_env.load() - - self.logger.debug("loading current programming environment") - self._current_environ.load() - - self.logger.debug("loading user's environment") - self._user_environ.load() - self._cdt_environ.load() - environ_save.load() - def _setup_paths(self): '''Setup the check's dynamic paths.''' self.logger.debug('setting up paths') @@ -965,7 +939,7 @@ def setup(self, partition, environ, **job_opts): :raises reframe.core.exceptions.ReframeError: In case of errors. ''' self._current_partition = partition - self._setup_environ(environ) + self._current_environ = environ self._setup_paths() self._setup_job(**job_opts) if self.perf_patterns is not None: @@ -1055,8 +1029,10 @@ def compile(self): *self.build_system.emit_build_commands(self._current_environ), *self.postbuild_cmd ] + user_environ = env.Environment(type(self).__name__, + self.modules, self.variables.items()) environs = [self._current_partition.local_env, self._current_environ, - self._user_environ, self._cdt_environ] + user_environ, self._cdt_environ] self._build_job = getscheduler('local')( name='rfm_%s_build' % self.name, @@ -1105,8 +1081,10 @@ def run(self): exec_cmd = [self.job.launcher.run_command(self.job), self.executable, *self.executable_opts] commands = [*self.pre_run, ' '.join(exec_cmd), *self.post_run] + user_environ = env.Environment(type(self).__name__, + self.modules, self.variables.items()) environs = [self._current_partition.local_env, self._current_environ, - self._user_environ, self._cdt_environ] + user_environ, self._cdt_environ] with os_ext.change_dir(self._stagedir): try: @@ -1273,13 +1251,11 @@ def _copy_to_outputdir(self): shutil.copytree(f, os.path.join(self.outputdir, f_orig)) @_run_hooks() - def cleanup(self, remove_files=False, unload_env=True): + def cleanup(self, remove_files=False): '''The cleanup phase of the regression test pipeline. :arg remove_files: If :class:`True`, the stage directory associated with this test will be removed. - :arg unload_env: If :class:`True`, the environment that was used to run - this test will be unloaded. ''' aliased = os.path.samefile(self._stagedir, self._outputdir) if aliased: @@ -1292,14 +1268,8 @@ def cleanup(self, remove_files=False, unload_env=True): self.logger.debug('removing stage directory') os_ext.rmtree(self._stagedir) - if unload_env: - self.logger.debug("unloading test's environment") - self._cdt_environ.unload() - self._user_environ.unload() - self._current_environ.unload() - self._current_partition.local_env.unload() - # Dependency API + def user_deps(self): return util.SequenceView(self._userdeps) @@ -1401,7 +1371,7 @@ def setup(self, partition, environ, **job_opts): ''' # No need to setup the job for compile-only checks self._current_partition = partition - self._setup_environ(environ) + self._current_environ = environ self._setup_paths() @property diff --git a/reframe/core/schedulers/__init__.py b/reframe/core/schedulers/__init__.py index 624e81466f..2d523f622d 100644 --- a/reframe/core/schedulers/__init__.py +++ b/reframe/core/schedulers/__init__.py @@ -5,6 +5,7 @@ import abc import reframe.core.debug as debug +import reframe.core.environments as env import reframe.core.fields as fields import reframe.core.shell as shell import reframe.utility.typecheck as typ @@ -205,9 +206,7 @@ def prepare(self, commands, environs=None, **gen_opts): with shell.generate_script(self.script_filename, **gen_opts) as builder: builder.write_prolog(self.emit_preamble()) - for e in environs: - builder.write(e.emit_load_commands()) - + builder.write(env.emit_load_commands(*environs)) for c in commands: builder.write_body(c) diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index f008400a54..558dbb7b44 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -7,6 +7,7 @@ import reframe import reframe.core.config as config +import reframe.core.environments as env import reframe.core.logging as logging import reframe.core.runtime as runtime import reframe.frontend.argparse as argparse @@ -380,15 +381,15 @@ def main(): if options.show_config_env: envname = options.show_config_env for p in rt.system.partitions: - env = p.environment(envname) - if env: + environ = p.environment(envname) + if environ: break - if env is None: + if environ is None: printer.error('no such environment: ' + envname) sys.exit(1) - printer.info(env.details()) + printer.info(environ.details()) sys.exit(0) if hasattr(settings, 'perf_logging_config'): @@ -516,7 +517,7 @@ def main(): # Load the environment for the current system try: - rt.system.preload_environ.load() + env.load(rt.system.preload_environ) except EnvironError as e: printer.error("failed to load current system's environment; " "please check your configuration") diff --git a/reframe/frontend/executors/__init__.py b/reframe/frontend/executors/__init__.py index ec92e9a64d..f1fbe6b996 100644 --- a/reframe/frontend/executors/__init__.py +++ b/reframe/frontend/executors/__init__.py @@ -4,9 +4,9 @@ import weakref import reframe.core.debug as debug +import reframe.core.environments as env import reframe.core.logging as logging import reframe.core.runtime as runtime -from reframe.core.environments import EnvironmentSnapshot from reframe.core.exceptions import (AbortTaskError, JobNotStartedError, ReframeFatalError, TaskExit) from reframe.frontend.printer import PrettyPrinter @@ -153,7 +153,7 @@ def _safe_call(self, fn, *args, **kwargs): def setup(self, *args, **kwargs): self._safe_call(self.check.setup, *args, **kwargs) - self._environ = EnvironmentSnapshot() + self._environ = env.snapshot() def compile(self): self._safe_call(self.check.compile) @@ -193,7 +193,7 @@ def fail(self, exc_info=None): self._notify_listeners('on_task_failure') def resume(self): - self._environ.load() + self._environ.restore() def abort(self, cause=None): logging.getlogger().debug('aborting: %s' % self.check.info()) @@ -241,7 +241,7 @@ def __init__(self, policy, printer=None, max_retries=0): self._stats = TestStats() self._policy.stats = self._stats self._policy.printer = self._printer - self._environ_snapshot = EnvironmentSnapshot() + self._environ_snapshot = env.snapshot() def __repr__(self): return debug.repr(self) @@ -274,7 +274,7 @@ def runall(self, testcases): (len(testcases), num_checks, num_failures), just='center' ) self._printer.timestamp('Finished on', 'short double line') - self._environ_snapshot.load() + self._environ_snapshot.restore() def _retry_failed(self, cases): rt = runtime.runtime() @@ -309,7 +309,7 @@ def print_separator(check, prefix): print_separator(t.check, 'started processing') last_check = t.check - self._environ_snapshot.load() + self._environ_snapshot.restore() self._policy.runcase(t) # Close the last visual box diff --git a/reframe/frontend/executors/policies.py b/reframe/frontend/executors/policies.py index 02a9443302..417063ea7d 100644 --- a/reframe/frontend/executors/policies.py +++ b/reframe/frontend/executors/policies.py @@ -45,7 +45,7 @@ def runcase(self, case): if not self.skip_performance_check: task.performance() - task.cleanup(not self.keep_stage_files, False) + task.cleanup(not self.keep_stage_files) except TaskExit: return @@ -222,7 +222,7 @@ def _finalize_task(self, task): if not self.skip_performance_check: task.performance() - task.cleanup(not self.keep_stage_files, False) + task.cleanup(not self.keep_stage_files) def _failall(self, cause): '''Mark all tests as failures''' diff --git a/unittests/test_cli.py b/unittests/test_cli.py index d99e37d07b..7ea7ec3349 100644 --- a/unittests/test_cli.py +++ b/unittests/test_cli.py @@ -10,16 +10,16 @@ from io import StringIO import reframe.core.config as config +import reframe.core.environments as env import reframe.core.runtime as rt import reframe.utility.os_ext as os_ext import unittests.fixtures as fixtures -from reframe.core.environments import EnvironmentSnapshot def run_command_inline(argv, funct, *args, **kwargs): # Save current execution context argv_save = sys.argv - environ_save = EnvironmentSnapshot() + environ_save = env.snapshot() sys.argv = argv exitcode = None @@ -35,7 +35,7 @@ def run_command_inline(argv, funct, *args, **kwargs): exitcode = e.code finally: # Restore execution context - environ_save.load() + environ_save.restore() sys.argv = argv_save return (exitcode, diff --git a/unittests/test_environments.py b/unittests/test_environments.py index 13330b0012..14f92aa74d 100644 --- a/unittests/test_environments.py +++ b/unittests/test_environments.py @@ -1,7 +1,8 @@ import os +import pytest import unittest -import reframe.core.environments as renv +import reframe.core.environments as env import reframe.utility.os_ext as os_ext import unittests.fixtures as fixtures from reframe.core.runtime import runtime @@ -11,11 +12,11 @@ class TestEnvironment(unittest.TestCase): def assertModulesLoaded(self, modules): for m in modules: - self.assertTrue(self.modules_system.is_module_loaded(m)) + assert self.modules_system.is_module_loaded(m) def assertModulesNotLoaded(self, modules): for m in modules: - self.assertFalse(self.modules_system.is_module_loaded(m)) + assert not self.modules_system.is_module_loaded(m) def setup_modules_system(self): if not fixtures.has_sane_modules_system(): @@ -34,198 +35,165 @@ def setUp(self): self.modules_system = None os.environ['_var0'] = 'val0' os.environ['_var1'] = 'val1' - self.environ_save = renv.EnvironmentSnapshot() - self.environ = renv.Environment(name='TestEnv1', - modules=['testmod_foo'], - variables=[('_var0', 'val1'), - ('_var2', '$_var0'), - ('_var3', '${_var1}')]) - self.environ_other = renv.Environment(name='TestEnv2', - modules=['testmod_boo'], - variables={'_var4': 'val4'}) + self.environ_save = env.snapshot() + self.environ = env.Environment(name='TestEnv1', + modules=['testmod_foo'], + variables=[('_var0', 'val1'), + ('_var2', '$_var0'), + ('_var3', '${_var1}')]) + self.environ_other = env.Environment(name='TestEnv2', + modules=['testmod_boo'], + variables={'_var4': 'val4'}) def tearDown(self): if self.modules_system is not None: self.modules_system.searchpath_remove(fixtures.TEST_MODULES) - self.environ_save.load() + self.environ_save.restore() def test_setup(self): if fixtures.has_sane_modules_system(): - self.assertEqual(len(self.environ.modules), 1) - self.assertIn('testmod_foo', self.environ.modules) + assert len(self.environ.modules) == 1 + assert 'testmod_foo' in self.environ.modules - self.assertEqual(len(self.environ.variables.keys()), 3) - self.assertEqual(self.environ.variables['_var0'], 'val1') + assert len(self.environ.variables.keys()) == 3 + assert self.environ.variables['_var0'] == 'val1' # No variable expansion, if environment is not loaded - self.assertEqual(self.environ.variables['_var2'], '$_var0') - self.assertEqual(self.environ.variables['_var3'], '${_var1}') + self.environ.variables['_var2'] == '$_var0' + self.environ.variables['_var3'] == '${_var1}' def test_environ_snapshot(self): - self.assertRaises(NotImplementedError, self.environ_save.unload) - self.environ.load() - self.environ_other.load() - self.environ_save.load() - self.assertEqual(self.environ_save, renv.EnvironmentSnapshot()) - self.assertFalse(self.environ.is_loaded) - self.assertFalse(self.environ_other.is_loaded) - with self.assertRaises(NotImplementedError): - _ = self.environ_save.is_loaded - - def test_environ_snapshot_context_mgr(self): - with renv.save_environment() as env: - self.assertIsInstance(env, renv.EnvironmentSnapshot) - del os.environ['_var0'] - os.environ['_var1'] = 'valX' - os.environ['_var2'] = 'var3' - - self.assertEqual('val0', os.environ['_var0']) - self.assertEqual('val1', os.environ['_var1']) - self.assertNotIn('_var2', os.environ) + env.load(self.environ, self.environ_other) + self.environ_save.restore() + assert self.environ_save == env.snapshot() + assert not self.environ.is_loaded + assert not self.environ_other.is_loaded def test_load_restore(self): - self.environ.load() - self.assertEqual(os.environ['_var0'], 'val1') - self.assertEqual(os.environ['_var1'], 'val1') - self.assertEqual(os.environ['_var2'], 'val1') - self.assertEqual(os.environ['_var3'], 'val1') + snapshot, _ = env.load(self.environ) + os.environ['_var0'] == 'val1' + os.environ['_var1'] == 'val1' + os.environ['_var2'] == 'val1' + os.environ['_var3'] == 'val1' if fixtures.has_sane_modules_system(): self.assertModulesLoaded(self.environ.modules) - self.assertTrue(self.environ.is_loaded) - self.environ.unload() - self.assertEqual(self.environ_save, renv.EnvironmentSnapshot()) - self.assertEqual(os.environ['_var0'], 'val0') + assert self.environ.is_loaded + snapshot.restore() + self.environ_save == env.snapshot() + os.environ['_var0'], 'val0' if fixtures.has_sane_modules_system(): - self.assertFalse( - self.modules_system.is_module_loaded('testmod_foo')) + assert not self.modules_system.is_module_loaded('testmod_foo') - self.assertFalse(self.environ.is_loaded) + assert not self.environ.is_loaded @fixtures.switch_to_user_runtime def test_load_already_present(self): self.setup_modules_system() self.modules_system.load_module('testmod_boo') - self.environ.load() - self.environ.unload() - self.assertTrue(self.modules_system.is_module_loaded('testmod_boo')) + snapshot, _ = env.load(self.environ) + snapshot.restore() + assert self.modules_system.is_module_loaded('testmod_boo') def test_load_non_overlapping(self): - e0 = renv.Environment(name='e0', variables=[('a', '1'), ('b', '2')]) - e1 = renv.Environment(name='e1', variables=[('c', '3'), ('d', '4')]) - e0.load() - e1.load() - self.assertTrue(e0.is_loaded) - self.assertTrue(e1.is_loaded) + e0 = env.Environment(name='e0', variables=[('a', '1'), ('b', '2')]) + e1 = env.Environment(name='e1', variables=[('c', '3'), ('d', '4')]) + env.load(e0, e1) + assert e0.is_loaded + assert e1.is_loaded def test_load_overlapping(self): - e0 = renv.Environment(name='e0', variables=[('a', '1'), ('b', '2')]) - e1 = renv.Environment(name='e1', variables=[('b', '3'), ('c', '4')]) - e0.load() - e1.load() - self.assertFalse(e0.is_loaded) - self.assertTrue(e1.is_loaded) + e0 = env.Environment(name='e0', variables=[('a', '1'), ('b', '2')]) + e1 = env.Environment(name='e1', variables=[('b', '3'), ('c', '4')]) + env.load(e0, e1) + assert not e0.is_loaded + assert e1.is_loaded def test_equal(self): - env1 = renv.Environment('env1', modules=['foo', 'bar']) - env2 = renv.Environment('env1', modules=['bar', 'foo']) - self.assertEqual(env1, env2) + env1 = env.Environment('env1', modules=['foo', 'bar']) + env2 = env.Environment('env1', modules=['bar', 'foo']) + assert env1 == env2 def test_not_equal(self): - env1 = renv.Environment('env1', modules=['foo', 'bar']) - env2 = renv.Environment('env2', modules=['foo', 'bar']) - self.assertNotEqual(env1, env2) + env1 = env.Environment('env1', modules=['foo', 'bar']) + env2 = env.Environment('env2', modules=['foo', 'bar']) + assert env1 != env2 # Variables are ordered, because they might depend on each other - env1 = renv.Environment('env1', variables=[('a', 1), ('b', 2)]) - env1 = renv.Environment('env1', variables=[('b', 2), ('a', 1)]) - self.assertNotEqual(env1, env2) + env1 = env.Environment('env1', variables=[('a', 1), ('b', 2)]) + env1 = env.Environment('env1', variables=[('b', 2), ('a', 1)]) + assert env1 != env2 @fixtures.switch_to_user_runtime def test_conflicting_environments(self): self.setup_modules_system() - envfoo = renv.Environment(name='envfoo', - modules=['testmod_foo', 'testmod_boo']) - envbar = renv.Environment(name='envbar', modules=['testmod_bar']) - envfoo.load() - envbar.load() + envfoo = env.Environment(name='envfoo', + modules=['testmod_foo', 'testmod_boo']) + envbar = env.Environment(name='envbar', modules=['testmod_bar']) + env.load(envfoo, envbar) for m in envbar.modules: - self.assertTrue(self.modules_system.is_module_loaded(m)) + assert self.modules_system.is_module_loaded(m) for m in envfoo.modules: - self.assertFalse(self.modules_system.is_module_loaded(m)) + assert not self.modules_system.is_module_loaded(m) @fixtures.switch_to_user_runtime def test_conflict_environ_after_module_load(self): self.setup_modules_system() self.modules_system.load_module('testmod_foo') - envfoo = renv.Environment(name='envfoo', modules=['testmod_foo']) - envfoo.load() - envfoo.unload() - self.assertTrue(self.modules_system.is_module_loaded('testmod_foo')) + envfoo = env.Environment(name='envfoo', modules=['testmod_foo']) + snapshot, _ = env.load(envfoo) + snapshot.restore() + assert self.modules_system.is_module_loaded('testmod_foo') @fixtures.switch_to_user_runtime def test_conflict_environ_after_module_force_load(self): self.setup_modules_system() self.modules_system.load_module('testmod_foo') - envbar = renv.Environment(name='envbar', modules=['testmod_bar']) - envbar.load() - envbar.unload() - self.assertTrue(self.modules_system.is_module_loaded('testmod_foo')) - - def test_swap(self): - from reframe.core.environments import swap_environments - - self.environ.load() - swap_environments(self.environ, self.environ_other) - self.assertFalse(self.environ.is_loaded) - self.assertTrue(self.environ_other.is_loaded) + envbar = env.Environment(name='envbar', modules=['testmod_bar']) + snapshot, _ = env.load(envbar) + snapshot.restore() + assert self.modules_system.is_module_loaded('testmod_foo') def test_immutability(self): # Check emit_load_commands() - commands = self.environ.emit_load_commands() - self.assertIsNot(commands, self.environ.emit_load_commands()) - self.assertEqual(commands, self.environ.emit_load_commands()) + _, commands = env.load(self.environ) # Try to modify the returned list of commands commands.append('foo') - self.assertNotIn('foo', self.environ.emit_load_commands()) + assert 'foo' not in env.load(self.environ)[1] # Test ProgEnvironment - prgenv = renv.ProgEnvironment('foo_prgenv') - self.assertIsInstance(prgenv, renv.Environment) - with self.assertRaises(AttributeError): + prgenv = env.ProgEnvironment('foo_prgenv') + assert isinstance(prgenv, env.Environment) + with pytest.raises(AttributeError): prgenv.cc = 'gcc' - with self.assertRaises(AttributeError): + with pytest.raises(AttributeError): prgenv.cxx = 'g++' - with self.assertRaises(AttributeError): + with pytest.raises(AttributeError): prgenv.ftn = 'gfortran' - with self.assertRaises(AttributeError): + with pytest.raises(AttributeError): prgenv.nvcc = 'clang' - with self.assertRaises(AttributeError): + with pytest.raises(AttributeError): prgenv.cppflags = ['-DFOO'] - with self.assertRaises(AttributeError): + with pytest.raises(AttributeError): prgenv.cflags = ['-O1'] - with self.assertRaises(AttributeError): + with pytest.raises(AttributeError): prgenv.cxxflags = ['-O1'] - with self.assertRaises(AttributeError): + with pytest.raises(AttributeError): prgenv.fflags = ['-O1'] - with self.assertRaises(AttributeError): + with pytest.raises(AttributeError): prgenv.ldflags = ['-lm'] - def test_immutability_after_load(self): - self.environ.load() - self.test_immutability() - @fixtures.switch_to_user_runtime def test_emit_load_commands(self): self.setup_modules_system() @@ -236,7 +204,7 @@ def test_emit_load_commands(self): 'export _var2=$_var0', 'export _var3=${_var1}', ] - self.assertEqual(expected_commands, self.environ.emit_load_commands()) + assert expected_commands == env.emit_load_commands(self.environ) @fixtures.switch_to_user_runtime def test_emit_load_commands_with_confict(self): @@ -244,12 +212,6 @@ def test_emit_load_commands_with_confict(self): # Load a conflicting module self.modules_system.load_module('testmod_bar') - - # When the environment is not loaded, the conflicting module does not - # make a difference - self.test_emit_load_commands() - - self.environ.load() rt = runtime() expected_commands = [ rt.modules_system.emit_unload_commands('testmod_bar')[0], @@ -258,40 +220,4 @@ def test_emit_load_commands_with_confict(self): 'export _var2=$_var0', 'export _var3=${_var1}', ] - self.assertEqual(expected_commands, self.environ.emit_load_commands()) - - @fixtures.switch_to_user_runtime - def test_emit_unload_commands(self): - self.setup_modules_system() - rt = runtime() - expected_commands = [ - 'unset _var0', - 'unset _var2', - 'unset _var3', - rt.modules_system.emit_unload_commands('testmod_foo')[0], - ] - self.assertEqual(expected_commands, - self.environ.emit_unload_commands()) - - @fixtures.switch_to_user_runtime - def test_emit_unload_commands_with_confict(self): - self.setup_modules_system() - - # Load a conflicting module - self.modules_system.load_module('testmod_bar') - - # When the environment is not loaded, the conflicting module does not - # make a difference - self.test_emit_unload_commands() - - self.environ.load() - rt = runtime() - load_cmd = rt.modules_system.emit_load_commands - unload_cmd = rt.modules_system.emit_unload_commands - expected_commands = ['unset _var0', - 'unset _var2', - 'unset _var3'] - expected_commands += unload_cmd('testmod_foo') - expected_commands += load_cmd('testmod_bar') - self.assertEqual(expected_commands, - self.environ.emit_unload_commands()) + assert expected_commands == env.emit_load_commands(self.environ) diff --git a/unittests/test_modules.py b/unittests/test_modules.py index d3a351e777..6c248f1a03 100644 --- a/unittests/test_modules.py +++ b/unittests/test_modules.py @@ -3,8 +3,8 @@ import unittest from tempfile import NamedTemporaryFile +import reframe.core.environments as env import reframe.core.modules as modules -from reframe.core.environments import EnvironmentSnapshot from reframe.core.exceptions import ConfigError, EnvironError from reframe.core.runtime import runtime from unittests.fixtures import TEST_MODULES @@ -12,11 +12,11 @@ class _TestModulesSystem(abc.ABC): def setUp(self): - self.environ_save = EnvironmentSnapshot() + self.environ_save = env.snapshot() self.modules_system.searchpath_add(TEST_MODULES) def tearDown(self): - self.environ_save.load() + self.environ_save.restore() def test_searchpath(self): self.assertIn(TEST_MODULES, self.modules_system.searchpath) diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index 6c7a450519..c545e848e6 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -85,9 +85,6 @@ def test_environ_setup(self): for k in test.variables.keys(): self.assertNotIn(k, os.environ) - # Manually unload the environment - self.prgenv.unload() - def _run_test(self, test, compile_only=False): _run(test, self.partition, self.prgenv) self.assertFalse(os.path.exists(test.stagedir)) From a5a47faa719af46c5bb6e98d6b9f43047934aaf5 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Wed, 9 Oct 2019 00:42:28 +0200 Subject: [PATCH 3/3] Fix Python 3.5 unit test failures - Use custom equality function for environment functions that ignores the order of the variables. - Provide some fixes to `SequenceView` and `MappingView` - Provide an `__str__` and `__repr__` to `SequenceView` and `MappingView` --- reframe/core/environments.py | 18 +++++++++++++++--- reframe/utility/__init__.py | 23 +++++++++++++++++++---- unittests/test_environments.py | 3 ++- unittests/test_utility.py | 6 +++++- 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/reframe/core/environments.py b/reframe/core/environments.py index beb6d6a6b3..fef08e7221 100644 --- a/reframe/core/environments.py +++ b/reframe/core/environments.py @@ -72,9 +72,9 @@ def __eq__(self, other): if not isinstance(other, type(self)): return NotImplemented - return (self._name == other._name and - set(self._modules) == set(other._modules) and - self._variables == other._variables) + return (self.name == other.name and + set(self.modules) == set(other.modules) and + self.variables == other.variables) def __str__(self): return self.name @@ -96,6 +96,18 @@ def restore(self): os.environ.clear() os.environ.update(self._variables) + def __eq__(self, other): + if not isinstance(other, Environment): + return NotImplemented + + # Order of variables is not important when comparing snapshots + for k, v in self.variables.items(): + if other.variables[k] != v: + return False + + return (self.name == other.name and + set(self.modules) == set(other.modules)) + def snapshot(): '''Create an environment snapshot''' diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index 18adb8600c..30acf3f144 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -472,11 +472,17 @@ def __iadd__(self, other): return NotImplemented def __eq__(self, other): - if not isinstance(other, collections.abc.Sequence): - return NotImplemented + if isinstance(other, SequenceView): + return self.__container == other.__container return self.__container == other + def __repr__(self): + return '%s(%r)' % (type(self).__name__, self.__container) + + def __str__(self): + return str(self.__container) + class MappingView(collections.abc.Mapping): '''A read-only view of a mapping.''' @@ -511,8 +517,17 @@ def __iter__(self, *args, **kwargs): def __len__(self, *args, **kwargs): return self.__mapping.__len__(*args, **kwargs) - def __eq__(self, *args, **kwargs): - return self.__mapping.__eq__(*args, **kwargs) + def __eq__(self, other): + if isinstance(other, MappingView): + return self.__mapping == other.__mapping + + return self.__mapping.__eq__(other) def __ne__(self, *args, **kwargs): return self.__mapping.__ne__(*args, **kwargs) + + def __repr__(self): + return '%s(%r)' % (type(self).__name__, self.__mapping) + + def __str__(self): + return str(self.__mapping) diff --git a/unittests/test_environments.py b/unittests/test_environments.py index 14f92aa74d..47741e9d84 100644 --- a/unittests/test_environments.py +++ b/unittests/test_environments.py @@ -114,6 +114,7 @@ def test_equal(self): env1 = env.Environment('env1', modules=['foo', 'bar']) env2 = env.Environment('env1', modules=['bar', 'foo']) assert env1 == env2 + assert env2 == env1 def test_not_equal(self): env1 = env.Environment('env1', modules=['foo', 'bar']) @@ -122,7 +123,7 @@ def test_not_equal(self): # Variables are ordered, because they might depend on each other env1 = env.Environment('env1', variables=[('a', 1), ('b', 2)]) - env1 = env.Environment('env1', variables=[('b', 2), ('a', 1)]) + env2 = env.Environment('env1', variables=[('b', 2), ('a', 1)]) assert env1 != env2 @fixtures.switch_to_user_runtime diff --git a/unittests/test_utility.py b/unittests/test_utility.py index 02570ff947..be11774fa7 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -767,10 +767,12 @@ def test_sequence(self): self.assertEqual(1, l[0]) self.assertEqual(3, len(l)) self.assertIn(2, l) - self.assertEqual(list(l), [1, 2, 2]) + self.assertEqual(l, [1, 2, 2]) + self.assertEqual(l, util.SequenceView([1, 2, 2])) self.assertEqual(list(reversed(l)), [2, 2, 1]) self.assertEqual(1, l.index(2)) self.assertEqual(2, l.count(2)) + self.assertEqual(str(l), str([1, 2, 2])) # Assert immutability m = l + [3, 4] @@ -832,6 +834,8 @@ def test_mapping(self): self.assertEqual(2, d.get('b')) self.assertEqual(3, d.get('c', 3)) self.assertEqual({'a': 1, 'b': 2}, d) + self.assertEqual(d, util.MappingView({'b': 2, 'a': 1})) + self.assertEqual(str(d), str({'a': 1, 'b': 2})) self.assertNotEqual({'a': 1, 'b': 2, 'c': 3}, d) # Assert immutability