From 6646c798ebce7189180a3f1e2ccbb995a9e8511d Mon Sep 17 00:00:00 2001 From: jgphpc Date: Fri, 18 Sep 2020 18:20:04 +0200 Subject: [PATCH 1/5] Add ignore_exit_code configuration parameter --- docs/config_reference.rst | 8 ++++++++ docs/manpage.rst | 12 ++++++++++++ reframe/core/pipeline.py | 7 +++++++ reframe/schemas/config.json | 2 ++ 4 files changed, 29 insertions(+) diff --git a/docs/config_reference.rst b/docs/config_reference.rst index 9b46426d0f..bb8efae19f 100644 --- a/docs/config_reference.rst +++ b/docs/config_reference.rst @@ -1067,6 +1067,14 @@ General Configuration Ignore test name conflicts when loading tests. +.. js:attribute:: .general[].ignore_exit_code + + :required: No + :default: ``true`` + + Ignore job exit code + + .. js:attribute:: .general[].keep_stage_files :required: No diff --git a/docs/manpage.rst b/docs/manpage.rst index 9e3a62dfac..ae1198e182 100644 --- a/docs/manpage.rst +++ b/docs/manpage.rst @@ -694,6 +694,18 @@ Here is an alphabetical list of the environment variables recognized by ReFrame: ================================== ================== +.. envvar:: RFM_IGNORE_EXIT_CODE + + Ignore job exit code + + .. table:: + :align: left + + ================================== ================== + Associated configuration parameter :js:attr:`ignore_exit_code` general configuration parameter + ================================== ================== + + .. envvar:: RFM_IGNORE_REQNODENOTAVAIL Do not treat specially jobs in pending state with the reason ``ReqNodeNotAvail`` (Slurm only). diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index ee94751ea7..14e2a68eb8 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -1407,6 +1407,13 @@ def check_sanity(self): if self.sanity_patterns is None: raise SanityError('sanity_patterns not set') + if not rt.runtime().get_option('general/0/ignore_exit_code'): + self.sanity_patterns = sn.all([ + sn.assert_eq(self.job.exitcode, 0, + msg='job exited with exit code {0}'), + self.sanity_patterns + ]) + with os_ext.change_dir(self._stagedir): success = sn.evaluate(self.sanity_patterns) if not success: diff --git a/reframe/schemas/config.json b/reframe/schemas/config.json index bd35d64c1b..429c9eefee 100644 --- a/reframe/schemas/config.json +++ b/reframe/schemas/config.json @@ -356,6 +356,7 @@ "clean_stagedir": {"type": "boolean"}, "colorize": {"type": "boolean"}, "ignore_check_conflicts": {"type": "boolean"}, + "ignore_exit_code": {"type": "boolean"}, "keep_stage_files": {"type": "boolean"}, "module_map_file": {"type": "string"}, "module_mappings": { @@ -402,6 +403,7 @@ "general/clean_stagedir": true, "general/colorize": true, "general/ignore_check_conflicts": false, + "general/ignore_exit_code": true, "general/keep_stage_files": false, "general/module_map_file": "", "general/module_mappings": [], From a37ea5b5c2498d8bd2de61b6f8622612e6293a3b Mon Sep 17 00:00:00 2001 From: jgphpc Date: Wed, 23 Sep 2020 10:58:58 +0200 Subject: [PATCH 2/5] changes for review --- docs/config_reference.rst | 6 +++--- docs/manpage.rst | 4 ++-- reframe/core/pipeline.py | 2 +- reframe/schemas/config.json | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/config_reference.rst b/docs/config_reference.rst index 8a27823c5a..5312081831 100644 --- a/docs/config_reference.rst +++ b/docs/config_reference.rst @@ -1067,12 +1067,12 @@ General Configuration Ignore test name conflicts when loading tests. -.. js:attribute:: .general[].ignore_exit_code +.. js:attribute:: .general[].trap_job_errors :required: No - :default: ``true`` + :default: ``false`` - Ignore job exit code + Trap any failing command from the jobscript. .. js:attribute:: .general[].keep_stage_files diff --git a/docs/manpage.rst b/docs/manpage.rst index ae1198e182..9230848cea 100644 --- a/docs/manpage.rst +++ b/docs/manpage.rst @@ -694,7 +694,7 @@ Here is an alphabetical list of the environment variables recognized by ReFrame: ================================== ================== -.. envvar:: RFM_IGNORE_EXIT_CODE +.. envvar:: RFM_TRAP_JOB_ERRORS Ignore job exit code @@ -702,7 +702,7 @@ Here is an alphabetical list of the environment variables recognized by ReFrame: :align: left ================================== ================== - Associated configuration parameter :js:attr:`ignore_exit_code` general configuration parameter + Associated configuration parameter :js:attr:`trap_job_errors` general configuration parameter ================================== ================== diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 14e2a68eb8..9964a67a5f 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -1407,7 +1407,7 @@ def check_sanity(self): if self.sanity_patterns is None: raise SanityError('sanity_patterns not set') - if not rt.runtime().get_option('general/0/ignore_exit_code'): + if rt.runtime().get_option('general/0/trap_job_errors'): self.sanity_patterns = sn.all([ sn.assert_eq(self.job.exitcode, 0, msg='job exited with exit code {0}'), diff --git a/reframe/schemas/config.json b/reframe/schemas/config.json index 10d0dcaa2d..2e352b4517 100644 --- a/reframe/schemas/config.json +++ b/reframe/schemas/config.json @@ -356,7 +356,7 @@ "clean_stagedir": {"type": "boolean"}, "colorize": {"type": "boolean"}, "ignore_check_conflicts": {"type": "boolean"}, - "ignore_exit_code": {"type": "boolean"}, + "trap_job_errors": {"type": "boolean"}, "keep_stage_files": {"type": "boolean"}, "module_map_file": {"type": "string"}, "module_mappings": { @@ -403,7 +403,7 @@ "general/clean_stagedir": true, "general/colorize": true, "general/ignore_check_conflicts": false, - "general/ignore_exit_code": true, + "general/trap_job_errors": false, "general/keep_stage_files": false, "general/module_map_file": "", "general/module_mappings": [], From 0d890cf0ed652c2afd492746fbd5c37d3c84f675 Mon Sep 17 00:00:00 2001 From: jgphpc Date: Thu, 24 Sep 2020 11:56:38 +0200 Subject: [PATCH 3/5] add trap_errors to job script --- reframe/core/pipeline.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 9964a67a5f..94493d26af 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -1317,9 +1317,10 @@ def run(self): self._job.prepare( commands, environs, login=rt.runtime().get_option('general/0/use_login_shell'), + trap_errors=True ) except OSError as e: - raise PipelineError('failed to prepare job') from e + raise PipelineError('failed to prepare run job') from e self._job.submit() From f3595506420f42010167a0b42bf44c9a320d9564 Mon Sep 17 00:00:00 2001 From: jgphpc Date: Sat, 26 Sep 2020 07:37:22 +0200 Subject: [PATCH 4/5] fix for review --- docs/config_reference.rst | 2 +- reframe/core/pipeline.py | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/config_reference.rst b/docs/config_reference.rst index 5312081831..3c2a3b2fe1 100644 --- a/docs/config_reference.rst +++ b/docs/config_reference.rst @@ -1072,7 +1072,7 @@ General Configuration :required: No :default: ``false`` - Trap any failing command from the jobscript. + Trap command errors and exit in the generated job scripts. .. js:attribute:: .general[].keep_stage_files diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 94493d26af..dfc83bcc53 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -46,7 +46,7 @@ #: dependencies will be explicitly specified by the user. #: #: This constant is directly available under the :mod:`reframe` module. -DEPEND_EXACT = 1 +DEPEND_EXACT = 1 #: Constant to be passed as the ``how`` argument of the #: :func:`RegressionTest.depends_on` method. It denotes that the test cases of @@ -61,7 +61,7 @@ #: this test depends on all the test cases of the target test. #: #: This constant is directly available under the :mod:`reframe` module. -DEPEND_FULLY = 3 +DEPEND_FULLY = 3 def _run_hooks(name=None): @@ -1147,7 +1147,7 @@ def compile(self): # Verify the sourcepath and determine the sourcepath in the stagedir if (os.path.isabs(self.sourcepath) or - os.path.normpath(self.sourcepath).startswith('..')): + os.path.normpath(self.sourcepath).startswith('..')): raise PipelineError( 'self.sourcepath is an absolute path or does not point to a ' 'subfolder or a file contained in self.sourcesdir: ' + @@ -1317,7 +1317,8 @@ def run(self): self._job.prepare( commands, environs, login=rt.runtime().get_option('general/0/use_login_shell'), - trap_errors=True + trap_errors=rt.runtime().get_option( + 'general/0/trap_job_errors') ) except OSError as e: raise PipelineError('failed to prepare run job') from e @@ -1601,7 +1602,7 @@ def depends_on(self, target, how=DEPEND_BY_ENV, subdeps=None): raise TypeError("how argument must be of type: `int'") if (subdeps is not None and - not isinstance(subdeps, typ.Dict[str, typ.List[str]])): + not isinstance(subdeps, typ.Dict[str, typ.List[str]])): raise TypeError("subdeps argument must be of type " "`Dict[str, List[str]]' or `None'") From a252634c32cdeb33324e5a5b6147cc0e0f77ea30 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 2 Oct 2020 17:17:23 +0200 Subject: [PATCH 5/5] Add unit tests and fix review comments --- docs/config_reference.rst | 2 +- reframe/core/pipeline.py | 19 +++++++++------- unittests/test_pipeline.py | 45 +++++++++++++++++++++++++------------- 3 files changed, 42 insertions(+), 24 deletions(-) diff --git a/docs/config_reference.rst b/docs/config_reference.rst index 3c2a3b2fe1..3ae6036542 100644 --- a/docs/config_reference.rst +++ b/docs/config_reference.rst @@ -1072,7 +1072,7 @@ General Configuration :required: No :default: ``false`` - Trap command errors and exit in the generated job scripts. + Trap command errors in the generated job scripts and let them exit immediately. .. js:attribute:: .general[].keep_stage_files diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index dfc83bcc53..4ba3b53cac 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -1318,7 +1318,8 @@ def run(self): commands, environs, login=rt.runtime().get_option('general/0/use_login_shell'), trap_errors=rt.runtime().get_option( - 'general/0/trap_job_errors') + 'general/0/trap_job_errors' + ) ) except OSError as e: raise PipelineError('failed to prepare run job') from e @@ -1406,15 +1407,17 @@ def check_sanity(self): more details. ''' - if self.sanity_patterns is None: - raise SanityError('sanity_patterns not set') - if rt.runtime().get_option('general/0/trap_job_errors'): - self.sanity_patterns = sn.all([ + sanity_patterns = [ sn.assert_eq(self.job.exitcode, 0, - msg='job exited with exit code {0}'), - self.sanity_patterns - ]) + msg='job exited with exit code {0}') + ] + if self.sanity_patterns is not None: + sanity_patterns.append(self.sanity_patterns) + + self.sanity_patterns = sn.all(sanity_patterns) + elif self.sanity_patterns is None: + raise SanityError('sanity_patterns not set') with os_ext.change_dir(self._stagedir): success = sn.evaluate(self.sanity_patterns) diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index 9424b6e0e0..f550a84370 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -107,21 +107,6 @@ def remote_exec_ctx(user_system): yield partition, environ -@pytest.fixture -def remote_exec_ctx(user_system): - partition = fixtures.partition_by_scheduler() - if partition is None: - pytest.skip('job submission not supported') - - try: - environ = partition.environs[0] - except IndexError: - pytest.skip('no environments configured for partition: %s' % - partition.fullname) - - yield partition, environ - - @pytest.fixture def container_remote_exec_ctx(remote_exec_ctx): def _container_exec_ctx(platform): @@ -789,6 +774,36 @@ def test_registration_of_tests(): mod.MyBaseTest(10, 20)] == checks +def test_trap_job_errors_without_sanity_patterns(local_exec_ctx): + rt.runtime().site_config.add_sticky_option('general/trap_job_errors', True) + + @fixtures.custom_prefix('unittests/resources/checks') + class MyTest(rfm.RunOnlyRegressionTest): + def __init__(self): + self.valid_prog_environs = ['*'] + self.valid_systems = ['*'] + self.executable = 'exit 10' + + with pytest.raises(SanityError, match='job exited with exit code 10'): + _run(MyTest(), *local_exec_ctx) + + +def test_trap_job_errors_with_sanity_patterns(local_exec_ctx): + rt.runtime().site_config.add_sticky_option('general/trap_job_errors', True) + + @fixtures.custom_prefix('unittests/resources/checks') + class MyTest(rfm.RunOnlyRegressionTest): + def __init__(self): + self.valid_prog_environs = ['*'] + self.valid_systems = ['*'] + self.prerun_cmds = ['echo hello'] + self.executable = 'true' + self.sanity_patterns = sn.assert_not_found(r'hello', self.stdout) + + with pytest.raises(SanityError): + _run(MyTest(), *local_exec_ctx) + + def _run_sanity(test, *exec_ctx, skip_perf=False): test.setup(*exec_ctx) test.check_sanity()