From 41b178547b9bc9e4d6d71e4b8b7c47c66a69c8bc Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Tue, 7 Feb 2023 14:30:54 +0100 Subject: [PATCH 1/5] Fix conflict when inheriting from hpctestlib classes --- reframe/frontend/loader.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/reframe/frontend/loader.py b/reframe/frontend/loader.py index a286587345..cac8424844 100644 --- a/reframe/frontend/loader.py +++ b/reframe/frontend/loader.py @@ -177,17 +177,23 @@ def load_from_module(self, module): if not self._validate_check(c): continue - testfile = module.__file__ + # Get the original filename in case of a different module name + testfile = ( + module.__file__ if module.__name__ == c.__module__ else + inspect.getfile(c.__class__) + ) + try: conflicted = self._loaded[c.unique_name] except KeyError: self._loaded[c.unique_name] = testfile final_tests.append(c) else: - raise NameConflictError( - f'test {c.unique_name!r} from {testfile!r} ' - f'is already defined in {conflicted!r}' - ) + if testfile != conflicted: + raise NameConflictError( + f'test {c.unique_name!r} from {testfile!r} ' + f'is already defined in {conflicted!r}' + ) getlogger().debug(f' > Loaded {len(final_tests)} test(s)') return final_tests From 3abe4eab33efca2eb35d0252a4ee5d7b7243bc9c Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Tue, 7 Feb 2023 15:54:58 +0100 Subject: [PATCH 2/5] Add unittest --- .../testlib_inheritance_bar.py | 18 +++++++++ .../testlib_inheritance_foo.py | 18 +++++++++ unittests/resources/testlib/simple.py | 39 +++++++++++++++++++ unittests/resources/testlib/src/Makefile | 2 + unittests/resources/testlib/src/hello.c | 6 +++ unittests/test_cli.py | 14 +++++++ 6 files changed, 97 insertions(+) create mode 100644 unittests/resources/checks_unlisted/testlib_inheritance_bar.py create mode 100644 unittests/resources/checks_unlisted/testlib_inheritance_foo.py create mode 100644 unittests/resources/testlib/simple.py create mode 100644 unittests/resources/testlib/src/Makefile create mode 100644 unittests/resources/testlib/src/hello.c diff --git a/unittests/resources/checks_unlisted/testlib_inheritance_bar.py b/unittests/resources/checks_unlisted/testlib_inheritance_bar.py new file mode 100644 index 0000000000..52ba357dff --- /dev/null +++ b/unittests/resources/checks_unlisted/testlib_inheritance_bar.py @@ -0,0 +1,18 @@ +# Copyright 2016-2023 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# ReFrame Project Developers. See the top-level LICENSE file for details. +# +# SPDX-License-Identifier: BSD-3-Clause + +import reframe as rfm +import reframe.utility.sanity as sn + +from testlib.simple import simple_check + + +@rfm.simple_test +class HelloBar(simple_check): + executable_opts = ['Bar'] + + @sanity_function + def assert_output(self): + return sn.assert_found(r'Hello Bar', self.stdout) diff --git a/unittests/resources/checks_unlisted/testlib_inheritance_foo.py b/unittests/resources/checks_unlisted/testlib_inheritance_foo.py new file mode 100644 index 0000000000..14d0aafc44 --- /dev/null +++ b/unittests/resources/checks_unlisted/testlib_inheritance_foo.py @@ -0,0 +1,18 @@ +# Copyright 2016-2023 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# ReFrame Project Developers. See the top-level LICENSE file for details. +# +# SPDX-License-Identifier: BSD-3-Clause + +import reframe as rfm +import reframe.utility.sanity as sn + +from testlib.simple import simple_check + + +@rfm.simple_test +class HelloFoo(simple_check): + executable_opts = ['Foo'] + + @sanity_function + def assert_output(self): + return sn.assert_found(r'Hello Foo', self.stdout) diff --git a/unittests/resources/testlib/simple.py b/unittests/resources/testlib/simple.py new file mode 100644 index 0000000000..bf31c818aa --- /dev/null +++ b/unittests/resources/testlib/simple.py @@ -0,0 +1,39 @@ +# Copyright 2016-2023 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# ReFrame Project Developers. See the top-level LICENSE file for details. +# +# SPDX-License-Identifier: BSD-3-Clause + +import os + +import reframe as rfm +import reframe.utility.sanity as sn + + +class simple_make_build(rfm.CompileOnlyRegressionTest, pin_prefix=True): + descr = 'Simple Make build fixture' + sourcesdir = 'src' + build_system = 'Make' + + @sanity_function + def assert_success(self): + return sn.assert_not_found(r'\S+', self.stderr) + + +@rfm.simple_test +class simple_check(rfm.RunOnlyRegressionTest): + descr = 'Simple test' + valid_systems = ['*'] + valid_prog_environs = ['builtin'] + executable = 'hello.x' + executable_opts = ['World'] + + hello_binaries = fixture(simple_make_build, scope='environment') + + @run_before('run') + def add_exec_prefix(self): + self.executable = os.path.join(self.hello_binaries.stagedir, + self.executable) + + @sanity_function + def assert_sanity(self): + return sn.assert_found(r'Hello World', self.stdout) diff --git a/unittests/resources/testlib/src/Makefile b/unittests/resources/testlib/src/Makefile new file mode 100644 index 0000000000..515f0566ac --- /dev/null +++ b/unittests/resources/testlib/src/Makefile @@ -0,0 +1,2 @@ +hello.x: hello.c + $(CC) -o $@ $< diff --git a/unittests/resources/testlib/src/hello.c b/unittests/resources/testlib/src/hello.c new file mode 100644 index 0000000000..2f4a161eaf --- /dev/null +++ b/unittests/resources/testlib/src/hello.c @@ -0,0 +1,6 @@ +#include + +int main(int argc, char** argv) { + printf("Hello %s\n", argv[1]); + return 0; +} diff --git a/unittests/test_cli.py b/unittests/test_cli.py index 878b6237fc..4f51817505 100644 --- a/unittests/test_cli.py +++ b/unittests/test_cli.py @@ -1045,3 +1045,17 @@ def test_dynamic_tests_filtering(run_reframe, tmp_path): assert returncode == 0 assert 'Ran 7/7 test case(s)' in stdout assert 'FAILED' not in stdout + + +def test_testlib_inherit_in_different_files(run_reframe, monkeypatch): + monkeypatch.syspath_prepend('unittests/resources') + returncode, stdout, _ = run_reframe( + checkpath=[ + 'unittests/resources/checks_unlisted/testlib_inheritance_foo.py', + 'unittests/resources/checks_unlisted/testlib_inheritance_bar.py' + ], + action='run', + ) + assert returncode == 0 + assert 'Ran 3/3 test case(s)' in stdout + assert 'FAILED' not in stdout From fc7428917704e7f8e7a0bfeb025601688fea32ea Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Tue, 14 Feb 2023 13:56:43 +0100 Subject: [PATCH 3/5] Address PR comments Signed-off-by: Theofilos Manitaras --- reframe/frontend/loader.py | 2 +- .../checks_unlisted/parameter_simple.py | 16 +++++++++++ .../testlib_inheritance_bar.py | 11 ++------ .../testlib_inheritance_foo.py | 11 ++------ unittests/resources/testlib/simple.py | 28 +++++++++---------- unittests/resources/testlib/src/Makefile | 2 -- unittests/resources/testlib/src/hello.c | 6 ---- unittests/test_cli.py | 2 +- 8 files changed, 38 insertions(+), 40 deletions(-) create mode 100644 unittests/resources/checks_unlisted/parameter_simple.py delete mode 100644 unittests/resources/testlib/src/Makefile delete mode 100644 unittests/resources/testlib/src/hello.c diff --git a/reframe/frontend/loader.py b/reframe/frontend/loader.py index cac8424844..2da8ca4301 100644 --- a/reframe/frontend/loader.py +++ b/reframe/frontend/loader.py @@ -189,7 +189,7 @@ def load_from_module(self, module): self._loaded[c.unique_name] = testfile final_tests.append(c) else: - if testfile != conflicted: + if not c.is_fixture(): raise NameConflictError( f'test {c.unique_name!r} from {testfile!r} ' f'is already defined in {conflicted!r}' diff --git a/unittests/resources/checks_unlisted/parameter_simple.py b/unittests/resources/checks_unlisted/parameter_simple.py new file mode 100644 index 0000000000..0f71609757 --- /dev/null +++ b/unittests/resources/checks_unlisted/parameter_simple.py @@ -0,0 +1,16 @@ +# Copyright 2016-2022 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# ReFrame Project Developers. See the top-level LICENSE file for details. +# +# SPDX-License-Identifier: BSD-3-Clause + +import reframe as rfm +import reframe.utility.sanity as sn + + +@rfm.simple_test +class SimpleParameter(rfm.RunOnlyRegressionTest): + message = parameter(['foo', 'bar']) + valid_systems = ['*'] + valid_prog_environs = ['*'] + executable = 'echo' + sanity_patterns = sn.assert_true(1) diff --git a/unittests/resources/checks_unlisted/testlib_inheritance_bar.py b/unittests/resources/checks_unlisted/testlib_inheritance_bar.py index 52ba357dff..874587335c 100644 --- a/unittests/resources/checks_unlisted/testlib_inheritance_bar.py +++ b/unittests/resources/checks_unlisted/testlib_inheritance_bar.py @@ -4,15 +4,10 @@ # SPDX-License-Identifier: BSD-3-Clause import reframe as rfm -import reframe.utility.sanity as sn -from testlib.simple import simple_check +from testlib.simple import simple_echo_check @rfm.simple_test -class HelloBar(simple_check): - executable_opts = ['Bar'] - - @sanity_function - def assert_output(self): - return sn.assert_found(r'Hello Bar', self.stdout) +class HelloBar(simple_echo_check): + message = 'Bar' diff --git a/unittests/resources/checks_unlisted/testlib_inheritance_foo.py b/unittests/resources/checks_unlisted/testlib_inheritance_foo.py index 14d0aafc44..9efd1b281f 100644 --- a/unittests/resources/checks_unlisted/testlib_inheritance_foo.py +++ b/unittests/resources/checks_unlisted/testlib_inheritance_foo.py @@ -4,15 +4,10 @@ # SPDX-License-Identifier: BSD-3-Clause import reframe as rfm -import reframe.utility.sanity as sn -from testlib.simple import simple_check +from testlib.simple import simple_echo_check @rfm.simple_test -class HelloFoo(simple_check): - executable_opts = ['Foo'] - - @sanity_function - def assert_output(self): - return sn.assert_found(r'Hello Foo', self.stdout) +class HelloFoo(simple_echo_check): + message = 'Foo' diff --git a/unittests/resources/testlib/simple.py b/unittests/resources/testlib/simple.py index bf31c818aa..4c12b81962 100644 --- a/unittests/resources/testlib/simple.py +++ b/unittests/resources/testlib/simple.py @@ -9,31 +9,31 @@ import reframe.utility.sanity as sn -class simple_make_build(rfm.CompileOnlyRegressionTest, pin_prefix=True): - descr = 'Simple Make build fixture' - sourcesdir = 'src' - build_system = 'Make' +class simple_echo(rfm.RunOnlyRegressionTest, pin_prefix=True): + descr = 'Simple Echo build fixture' + executable = 'echo' + executable_opts = ['Hello'] @sanity_function def assert_success(self): - return sn.assert_not_found(r'\S+', self.stderr) + return sn.assert_found(r'Hello', self.stdout) @rfm.simple_test -class simple_check(rfm.RunOnlyRegressionTest): - descr = 'Simple test' +class simple_echo_check(rfm.RunOnlyRegressionTest): + descr = 'Simple Echo Test' valid_systems = ['*'] valid_prog_environs = ['builtin'] - executable = 'hello.x' - executable_opts = ['World'] - - hello_binaries = fixture(simple_make_build, scope='environment') + executable = 'echo' + message = variable(str, value='World') + hello_output = fixture(simple_echo, scope='environment') @run_before('run') def add_exec_prefix(self): - self.executable = os.path.join(self.hello_binaries.stagedir, - self.executable) + fixture_output = os.path.join(self.hello_output.stagedir, + str(self.hello_output.stdout)) + self.executable_opts = [f'$(cat {fixture_output})', self.message] @sanity_function def assert_sanity(self): - return sn.assert_found(r'Hello World', self.stdout) + return sn.assert_found(rf'Hello {self.message}', self.stdout) diff --git a/unittests/resources/testlib/src/Makefile b/unittests/resources/testlib/src/Makefile deleted file mode 100644 index 515f0566ac..0000000000 --- a/unittests/resources/testlib/src/Makefile +++ /dev/null @@ -1,2 +0,0 @@ -hello.x: hello.c - $(CC) -o $@ $< diff --git a/unittests/resources/testlib/src/hello.c b/unittests/resources/testlib/src/hello.c deleted file mode 100644 index 2f4a161eaf..0000000000 --- a/unittests/resources/testlib/src/hello.c +++ /dev/null @@ -1,6 +0,0 @@ -#include - -int main(int argc, char** argv) { - printf("Hello %s\n", argv[1]); - return 0; -} diff --git a/unittests/test_cli.py b/unittests/test_cli.py index 4f51817505..4eca908e31 100644 --- a/unittests/test_cli.py +++ b/unittests/test_cli.py @@ -1047,7 +1047,7 @@ def test_dynamic_tests_filtering(run_reframe, tmp_path): assert 'FAILED' not in stdout -def test_testlib_inherit_in_different_files(run_reframe, monkeypatch): +def test_testlib_inherit_fixture_in_different_files(run_reframe, monkeypatch): monkeypatch.syspath_prepend('unittests/resources') returncode, stdout, _ = run_reframe( checkpath=[ From 97288589c40b515839c6b327872a204fd00e5f26 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Wed, 15 Feb 2023 10:06:31 +0100 Subject: [PATCH 4/5] Address PR comments Signed-off-by: Theofilos Manitaras --- reframe/frontend/loader.py | 8 ++++---- .../checks_unlisted/parameter_simple.py | 16 ---------------- unittests/resources/testlib/simple.py | 18 ++++++------------ 3 files changed, 10 insertions(+), 32 deletions(-) delete mode 100644 unittests/resources/checks_unlisted/parameter_simple.py diff --git a/reframe/frontend/loader.py b/reframe/frontend/loader.py index 2da8ca4301..34d4645abb 100644 --- a/reframe/frontend/loader.py +++ b/reframe/frontend/loader.py @@ -178,10 +178,10 @@ def load_from_module(self, module): continue # Get the original filename in case of a different module name - testfile = ( - module.__file__ if module.__name__ == c.__module__ else - inspect.getfile(c.__class__) - ) + if module.__name__ == c.__module__: + testfile = module.__file__ + else: + testfile = inspect.getfile(c.__class__) try: conflicted = self._loaded[c.unique_name] diff --git a/unittests/resources/checks_unlisted/parameter_simple.py b/unittests/resources/checks_unlisted/parameter_simple.py deleted file mode 100644 index 0f71609757..0000000000 --- a/unittests/resources/checks_unlisted/parameter_simple.py +++ /dev/null @@ -1,16 +0,0 @@ -# Copyright 2016-2022 Swiss National Supercomputing Centre (CSCS/ETH Zurich) -# ReFrame Project Developers. See the top-level LICENSE file for details. -# -# SPDX-License-Identifier: BSD-3-Clause - -import reframe as rfm -import reframe.utility.sanity as sn - - -@rfm.simple_test -class SimpleParameter(rfm.RunOnlyRegressionTest): - message = parameter(['foo', 'bar']) - valid_systems = ['*'] - valid_prog_environs = ['*'] - executable = 'echo' - sanity_patterns = sn.assert_true(1) diff --git a/unittests/resources/testlib/simple.py b/unittests/resources/testlib/simple.py index 4c12b81962..ccc4933ca9 100644 --- a/unittests/resources/testlib/simple.py +++ b/unittests/resources/testlib/simple.py @@ -9,14 +9,9 @@ import reframe.utility.sanity as sn -class simple_echo(rfm.RunOnlyRegressionTest, pin_prefix=True): - descr = 'Simple Echo build fixture' +class dummy_fixture(rfm.RunOnlyRegressionTest, pin_prefix=True): executable = 'echo' - executable_opts = ['Hello'] - - @sanity_function - def assert_success(self): - return sn.assert_found(r'Hello', self.stdout) + sanity_patterns = sn.assert_true(1) @rfm.simple_test @@ -25,14 +20,13 @@ class simple_echo_check(rfm.RunOnlyRegressionTest): valid_systems = ['*'] valid_prog_environs = ['builtin'] executable = 'echo' + executable_opts = ['Hello'] message = variable(str, value='World') - hello_output = fixture(simple_echo, scope='environment') + dummy = fixture(dummy_fixture, scope='environment') @run_before('run') - def add_exec_prefix(self): - fixture_output = os.path.join(self.hello_output.stagedir, - str(self.hello_output.stdout)) - self.executable_opts = [f'$(cat {fixture_output})', self.message] + def set_executable_opts(self): + self.executable_opts += [self.message] @sanity_function def assert_sanity(self): From 8b3c8d31256e658af3ea8b7078614da472e1f2b2 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Wed, 15 Feb 2023 10:14:15 +0100 Subject: [PATCH 5/5] Remove unused import Signed-off-by: Theofilos Manitaras --- unittests/resources/testlib/simple.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/unittests/resources/testlib/simple.py b/unittests/resources/testlib/simple.py index ccc4933ca9..a8be65e55f 100644 --- a/unittests/resources/testlib/simple.py +++ b/unittests/resources/testlib/simple.py @@ -3,8 +3,6 @@ # # SPDX-License-Identifier: BSD-3-Clause -import os - import reframe as rfm import reframe.utility.sanity as sn