From 585605fccd48f319b2c4b2c270e4f950a16ee97d Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 10 Sep 2021 11:38:39 +0200 Subject: [PATCH 1/5] Add unit test for remote auto-detection --- unittests/test_autodetect.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/unittests/test_autodetect.py b/unittests/test_autodetect.py index 0703de24e6..54c6513827 100644 --- a/unittests/test_autodetect.py +++ b/unittests/test_autodetect.py @@ -7,7 +7,7 @@ import os import pytest - +import unittests.utility as test_util from reframe.core.runtime import runtime from reframe.frontend.autodetect import detect_topology from reframe.utility.cpuinfo import cpuinfo @@ -84,3 +84,30 @@ def test_autotect_with_invalid_files(invalid_topo_exec_ctx): part = runtime().system.partitions[0] assert part.processor.info == cpuinfo() assert part.devices == [] + + +# FIXME: We should consider making these framework-wide fixtures + +@pytest.fixture +def user_exec_ctx(make_exec_ctx_g): + if test_util.USER_CONFIG_FILE is None: + pytest.skip('no user configuration file supplied') + + yield from make_exec_ctx_g(test_util.USER_CONFIG_FILE, + test_util.USER_SYSTEM) + + +@pytest.fixture +def remote_exec_ctx(user_exec_ctx): + partition = test_util.partition_by_scheduler() + if not partition: + pytest.skip('job submission not supported') + + return partition, partition.environs[0] + + +def test_remote_autodetect(remote_exec_ctx): + # All we can do with this test is to trigger the remote auto-detection + # path; since we don't know what the remote user system is, we cannot test + # if the topology is right. + detect_topology() From 6d0b502fc7fb3430cf70eb71baf388b18cf6882f Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 10 Sep 2021 13:27:59 +0200 Subject: [PATCH 2/5] Correctly define remote_exec_ctx fixture --- unittests/test_autodetect.py | 61 +++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/unittests/test_autodetect.py b/unittests/test_autodetect.py index 54c6513827..ca234ec24f 100644 --- a/unittests/test_autodetect.py +++ b/unittests/test_autodetect.py @@ -14,7 +14,7 @@ @pytest.fixture -def exec_ctx(make_exec_ctx_g, tmp_path, monkeypatch): +def temp_topo(tmp_path, monkeypatch): # Monkey-patch HOME, since topology is always written there monkeypatch.setenv('HOME', str(tmp_path)) @@ -30,11 +30,9 @@ def exec_ctx(make_exec_ctx_g, tmp_path, monkeypatch): } ], fp) - yield from make_exec_ctx_g() - @pytest.fixture -def invalid_topo_exec_ctx(make_exec_ctx_g, tmp_path, monkeypatch): +def invalid_topo(tmp_path, monkeypatch): # Monkey-patch HOME, since topology is always written there monkeypatch.setenv('HOME', str(tmp_path)) @@ -47,10 +45,41 @@ def invalid_topo_exec_ctx(make_exec_ctx_g, tmp_path, monkeypatch): with open(meta_prefix / 'devices.json', 'w') as fp: fp.write('{') + +@pytest.fixture +def user_exec_ctx(make_exec_ctx_g): + if test_util.USER_CONFIG_FILE is None: + pytest.skip('no user configuration file supplied') + + yield from make_exec_ctx_g(test_util.USER_CONFIG_FILE, + test_util.USER_SYSTEM) + + +@pytest.fixture +def remote_exec_ctx(user_exec_ctx): + partition = test_util.partition_by_scheduler() + if not partition: + pytest.skip('job submission not supported') + + yield partition, partition.environs[0] + + +@pytest.fixture +def default_exec_ctx(make_exec_ctx_g, temp_topo): + yield from make_exec_ctx_g() + + +@pytest.fixture +def remote_topo_exec_ctx(remote_exec_ctx, temp_topo): + yield from remote_exec_ctx() + + +@pytest.fixture +def invalid_topo_exec_ctx(make_exec_ctx_g, invalid_topo): yield from make_exec_ctx_g() -def test_autotect(exec_ctx): +def test_autotect(default_exec_ctx): detect_topology() part = runtime().system.partitions[0] assert part.processor.info == cpuinfo() @@ -86,27 +115,7 @@ def test_autotect_with_invalid_files(invalid_topo_exec_ctx): assert part.devices == [] -# FIXME: We should consider making these framework-wide fixtures - -@pytest.fixture -def user_exec_ctx(make_exec_ctx_g): - if test_util.USER_CONFIG_FILE is None: - pytest.skip('no user configuration file supplied') - - yield from make_exec_ctx_g(test_util.USER_CONFIG_FILE, - test_util.USER_SYSTEM) - - -@pytest.fixture -def remote_exec_ctx(user_exec_ctx): - partition = test_util.partition_by_scheduler() - if not partition: - pytest.skip('job submission not supported') - - return partition, partition.environs[0] - - -def test_remote_autodetect(remote_exec_ctx): +def test_remote_autodetect(remote_topo_exec_ctx): # All we can do with this test is to trigger the remote auto-detection # path; since we don't know what the remote user system is, we cannot test # if the topology is right. From b9f0baf9f2d94d60db72137161f61e158c559ffc Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 10 Sep 2021 15:33:59 +0200 Subject: [PATCH 3/5] Trigger remote auto-detection in unit tests --- unittests/test_autodetect.py | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/unittests/test_autodetect.py b/unittests/test_autodetect.py index ca234ec24f..6d8131ac6a 100644 --- a/unittests/test_autodetect.py +++ b/unittests/test_autodetect.py @@ -46,32 +46,20 @@ def invalid_topo(tmp_path, monkeypatch): fp.write('{') -@pytest.fixture -def user_exec_ctx(make_exec_ctx_g): - if test_util.USER_CONFIG_FILE is None: - pytest.skip('no user configuration file supplied') - - yield from make_exec_ctx_g(test_util.USER_CONFIG_FILE, - test_util.USER_SYSTEM) - - -@pytest.fixture -def remote_exec_ctx(user_exec_ctx): - partition = test_util.partition_by_scheduler() - if not partition: - pytest.skip('job submission not supported') - - yield partition, partition.environs[0] - - @pytest.fixture def default_exec_ctx(make_exec_ctx_g, temp_topo): yield from make_exec_ctx_g() @pytest.fixture -def remote_topo_exec_ctx(remote_exec_ctx, temp_topo): - yield from remote_exec_ctx() +def remote_exec_ctx(make_exec_ctx): + if test_util.USER_CONFIG_FILE is None: + pytest.skip('no user configuration file supplied') + + ctx = make_exec_ctx(test_util.USER_CONFIG_FILE, + test_util.USER_SYSTEM, + {'general/remote_detect': True}) + yield ctx @pytest.fixture @@ -115,7 +103,7 @@ def test_autotect_with_invalid_files(invalid_topo_exec_ctx): assert part.devices == [] -def test_remote_autodetect(remote_topo_exec_ctx): +def test_remote_autodetect(remote_exec_ctx): # All we can do with this test is to trigger the remote auto-detection # path; since we don't know what the remote user system is, we cannot test # if the topology is right. From 73c3aa7d7f4b6f31691e5c2135c9fe631f2efb90 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 10 Sep 2021 18:21:49 +0200 Subject: [PATCH 4/5] Fix remote auto-detection crash --- reframe/frontend/autodetect.py | 26 ++++++++++++++++++++------ unittests/test_autodetect.py | 16 +++++++++++----- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/reframe/frontend/autodetect.py b/reframe/frontend/autodetect.py index be026f55e3..8e5901ba17 100644 --- a/reframe/frontend/autodetect.py +++ b/reframe/frontend/autodetect.py @@ -8,6 +8,7 @@ import os import shutil import tempfile +import traceback import reframe as rfm import reframe.core.runtime as runtime @@ -19,6 +20,10 @@ from reframe.utility.cpuinfo import cpuinfo +# This is meant to be used by the unit tests +_TREAT_WARNINGS_AS_ERRORS = False + + def _contents(filename): '''Return the contents of a file.''' @@ -35,8 +40,8 @@ def _log_contents(filename): class _copy_reframe: def __init__(self, prefix): - self._prefix = prefix - self._prefix = runtime().get_option('general/0/remote_workdir') + rt = runtime.runtime() + self._prefix = rt.get_option('general/0/remote_workdir') self._workdir = None def __enter__(self): @@ -82,6 +87,9 @@ def _load_info(filename, schema=None): with open(filename) as fp: return _validate_info(json.load(fp), schema) except OSError as e: + if _TREAT_WARNINGS_AS_ERRORS: + raise + getlogger().warning( f'could not load file: {filename!r}: {e}' ) @@ -101,9 +109,14 @@ def _save_info(filename, topo_info): with open(filename, 'w') as fp: json.dump(topo_info, fp, indent=2) except OSError as e: + if _TREAT_WARNINGS_AS_ERRORS: + raise + getlogger().warning( f'could not save topology file: {filename!r}: {e}' ) + else: + getlogger().debug(f'> saved topology in {filename!r}') def _is_part_local(part): @@ -143,7 +156,11 @@ def _emit_script(job, env): _log_contents(job.stderr) topo_info = json.loads(_contents('topo.json')) except Exception as e: + if _TREAT_WARNINGS_AS_ERRORS: + raise + getlogger().warning(f'failed to retrieve remote processor info: {e}') + getlogger().debug(traceback.format_exc()) return topo_info @@ -226,14 +243,11 @@ def detect_topology(): _save_info(topo_file, part.processor.info) elif detect_remote_systems: - with runtime.temp_environment(modules=temp_modules, - variables=temp_vars): + with runtime.temp_environment(modules=modules, variables=vars): part._processor = ProcessorInfo(_remote_detect(part)) if part.processor.info: _save_info(topo_file, part.processor.info) - getlogger().debug(f'> saved topology in {topo_file!r}') - if not found_devinfo: getlogger().debug(f'> device auto-detection is not supported') diff --git a/unittests/test_autodetect.py b/unittests/test_autodetect.py index 6d8131ac6a..46c17ffe58 100644 --- a/unittests/test_autodetect.py +++ b/unittests/test_autodetect.py @@ -7,9 +7,9 @@ import os import pytest +import reframe.frontend.autodetect as autodetect import unittests.utility as test_util from reframe.core.runtime import runtime -from reframe.frontend.autodetect import detect_topology from reframe.utility.cpuinfo import cpuinfo @@ -17,6 +17,7 @@ def temp_topo(tmp_path, monkeypatch): # Monkey-patch HOME, since topology is always written there monkeypatch.setenv('HOME', str(tmp_path)) + monkeypatch.setattr(autodetect, '_TREAT_WARNINGS_AS_ERRORS', True) # Create a devices file manually, since it is not auto-generated meta_prefix = tmp_path / '.reframe' / 'topology' / 'generic-default' @@ -35,6 +36,7 @@ def temp_topo(tmp_path, monkeypatch): def invalid_topo(tmp_path, monkeypatch): # Monkey-patch HOME, since topology is always written there monkeypatch.setenv('HOME', str(tmp_path)) + monkeypatch.setattr(autodetect, '_TREAT_WARNINGS_AS_ERRORS', True) # Create invalid processor and devices files meta_prefix = tmp_path / '.reframe' / 'topology' / 'generic-default' @@ -52,7 +54,7 @@ def default_exec_ctx(make_exec_ctx_g, temp_topo): @pytest.fixture -def remote_exec_ctx(make_exec_ctx): +def remote_exec_ctx(make_exec_ctx, temp_topo): if test_util.USER_CONFIG_FILE is None: pytest.skip('no user configuration file supplied') @@ -68,7 +70,7 @@ def invalid_topo_exec_ctx(make_exec_ctx_g, invalid_topo): def test_autotect(default_exec_ctx): - detect_topology() + autodetect.detect_topology() part = runtime().system.partitions[0] assert part.processor.info == cpuinfo() if part.processor.info: @@ -97,7 +99,7 @@ def test_autotect(default_exec_ctx): def test_autotect_with_invalid_files(invalid_topo_exec_ctx): - detect_topology() + autodetect.detect_topology() part = runtime().system.partitions[0] assert part.processor.info == cpuinfo() assert part.devices == [] @@ -107,4 +109,8 @@ def test_remote_autodetect(remote_exec_ctx): # All we can do with this test is to trigger the remote auto-detection # path; since we don't know what the remote user system is, we cannot test # if the topology is right. - detect_topology() + partition = test_util.partition_by_scheduler() + if not partition: + pytest.skip('job submission not supported') + + autodetect.detect_topology() From 069c2f6f2cb8355470f848cf815198003148096b Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Mon, 13 Sep 2021 23:09:35 +0200 Subject: [PATCH 5/5] Fix `_copy_reframe()` implementation --- reframe/frontend/autodetect.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/reframe/frontend/autodetect.py b/reframe/frontend/autodetect.py index 8e5901ba17..50b85a8fc1 100644 --- a/reframe/frontend/autodetect.py +++ b/reframe/frontend/autodetect.py @@ -40,8 +40,7 @@ def _log_contents(filename): class _copy_reframe: def __init__(self, prefix): - rt = runtime.runtime() - self._prefix = rt.get_option('general/0/remote_workdir') + self._prefix = prefix self._workdir = None def __enter__(self):