From 3b6e0bdca29b86ffddd12397c994a9769c9a063b Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Fri, 27 Sep 2024 01:55:23 +1000 Subject: [PATCH 1/4] Support new and old style of PSyclone command line (no more nemo api etc) --- source/fab/tools/psyclone.py | 129 +++++++++++++- .../psyclone/test_psyclone_system_test.py | 6 +- tests/unit_tests/tools/test_psyclone.py | 163 +++++++++++------- 3 files changed, 229 insertions(+), 69 deletions(-) diff --git a/source/fab/tools/psyclone.py b/source/fab/tools/psyclone.py index 1a2b3b40..1d7fa255 100644 --- a/source/fab/tools/psyclone.py +++ b/source/fab/tools/psyclone.py @@ -8,7 +8,9 @@ """ from pathlib import Path +import re from typing import Callable, List, Optional, TYPE_CHECKING, Union +import warnings from fab.tools.category import Category from fab.tools.tool import Tool @@ -24,15 +26,75 @@ class Psyclone(Tool): '''This is the base class for `PSyclone`. ''' - def __init__(self, api: Optional[str] = None): + def __init__(self): super().__init__("psyclone", "psyclone", Category.PSYCLONE) - self._api = api + self._version = None + + def check_available(self) -> bool: + '''This function determines if PSyclone is available. Additionally, + it established the version, since command line option changes + significantly from python 2.5.0 to the next release. + ''' + + # First get the version (and confirm that PSyclone is installed): + try: + # Older versions of PSyclone (2.3.1 and earlier) expect a filename + # even when --version is used, and won't produce version info + # without this. So provide a dummy file (which does not need to + # exist), and check the error for details to see if PSyclone does + # not exist, or if the error is because of the non-existing file + version_output = self.run(["--version", "does_not_exist"], + capture_output=True) + except RuntimeError as err: + # If the command is not found, the error contains the following: + if "could not be executed" in str(err): + return False + # Otherwise, psyclone likely complained about the not existing + # file. Continue and try to find version information in the output: + version_output = str(err) + + # Search for the version info: + exp = r"PSyclone version: (\d[\d.]+\d)" + print("VERSION [", version_output, "]") + matches = re.search(exp, version_output) + if not matches: + warnings.warn(f"Unexpected version information for PSyclone: " + f"'{version_output}'.") + # If we don't recognise the version number, something is wrong + return False + + # Now convert the version info to integer. The regular expression + # match guarantees that we have integer numbers now: + version = tuple(int(x) for x in matches.groups()[0].split('.')) + + if version == (2, 5, 0): + # The behaviour of PSyclone changes from 2.5.0 to the next + # release. But since head-of-trunk still reports 2.5.0, we + # need to run additional tests to see if we have the official + # 2.5.0 release, or current trunk (which already has the new + # command line options). PSyclone needs an existing file + # in order to work, so use __file__ to present this file. + # PSyclone will obviously abort since this is not a Fortran + # file, but we only need to check the error message to + # see if the domain name is incorrect (--> current trunk) + # or not (2.5.0 release) + try: + self.run(["-api", "nemo", __file__], capture_output=True) + except RuntimeError as err: + if "Unsupported PSyKAL DSL / API 'nemo' specified" in str(err): + # It is current development. Just give it a version number + # greater than 2.5.0 + version = (2, 5, 0, 1) + + self._version = version + return True def process(self, config: "BuildConfig", x90_file: Path, - psy_file: Path, - alg_file: Union[Path, str], + psy_file: Optional[Path] = None, + alg_file: Optional[Union[Path, str]] = None, + transformed_file: Optional[Path] = None, transformation_script: Optional[Callable[[Path, "BuildConfig"], Path]] = None, additional_parameters: Optional[List[str]] = None, @@ -40,29 +102,78 @@ def process(self, api: Optional[str] = None, ): # pylint: disable=too-many-arguments - '''Run PSyclone with the specified parameters. + '''Run PSyclone with the specified parameters. If PSyclone is used to + transform existing Fortran files, `api` must be None, and the output + file name is `transformed_file`. If PSyclone is using its DSL + features, api must be a valid PSyclone API, and the two output + filenames are `psy_file` and `alg_file`. :param api: the PSyclone API. :param x90_file: the input file for PSyclone :param psy_file: the output PSy-layer file. :param alg_file: the output modified algorithm file. + :param transformed_file: the output filename if PSyclone is called + as transformation tool. :param transformation_script: an optional transformation script :param additional_parameters: optional additional parameters for PSyclone :param kernel_roots: optional directories with kernels. ''' + if not self.is_available: + raise RuntimeError("PSyclone is not available.") + + if api: + # API specified, we need both psy- and alg-file, but not + # transformed file. + if not psy_file: + raise RuntimeError(f"PSyclone called with api '{api}', but " + f"no psy_file is specified.") + if not alg_file: + raise RuntimeError(f"PSyclone called with api '{api}', but " + f"no alg_file is specified.") + if transformed_file: + raise RuntimeError(f"PSyclone called with api '{api}' and " + f"transformed_file.") + else: + if psy_file: + raise RuntimeError("PSyclone called without api, but " + "psy_file is specified.") + if alg_file: + raise RuntimeError("PSyclone called without api, but " + "alg_file is specified.") + if not transformed_file: + raise RuntimeError("PSyclone called without api, but " + "transformed_file it not specified.") + parameters: List[Union[str, Path]] = [] # If an api is defined in this call (or in the constructor) add it # as parameter. No API is required if PSyclone works as # transformation tool only, so calling PSyclone without api is # actually valid. + print("API", api, self._version) if api: - parameters.extend(["-api", api]) - elif self._api: - parameters.extend(["-api", self._api]) + if self._version > (2, 5, 0): + api_param = "--psykal-dsl" + # Mapping from old names to new names: + mapping = {"dynamo0.3": "lfric", + "gocean1.0": "gocean"} + else: + api_param = "-api" + # Mapping from new names to old names: + mapping = {"lfric": "dynamo0.3", + "gocean": "gocean1.0"} - parameters.extend(["-l", "all", "-opsy", psy_file, "-oalg", alg_file]) + parameters.extend([api_param, mapping.get(api, api), + "-opsy", psy_file, "-oalg", alg_file]) + else: # no api + if self._version > (2, 5, 0): + # New version: no API, parameter, but -o for output name: + parameters.extend(["-o", transformed_file]) + else: + # 2.5.0 or earlier: needs api nemo, output name is -oalg + parameters.extend(["-api", "nemo", "-opsy", transformed_file]) + parameters.extend(["-l", "all"]) if transformation_script: transformation_script_return_path = \ diff --git a/tests/system_tests/psyclone/test_psyclone_system_test.py b/tests/system_tests/psyclone/test_psyclone_system_test.py index 14b265d8..df299470 100644 --- a/tests/system_tests/psyclone/test_psyclone_system_test.py +++ b/tests/system_tests/psyclone/test_psyclone_system_test.py @@ -202,6 +202,9 @@ class TestTransformationScript: """ def test_transformation_script(self, psyclone_lfric_api): psyclone_tool = Psyclone() + psyclone_tool._version = (2, 4, 0) + psyclone_tool._is_available = True + mock_transformation_script = mock.Mock(return_value=__file__) with mock.patch('fab.tools.psyclone.Psyclone.run') as mock_run_command: mock_transformation_script.return_value = Path(__file__) @@ -219,8 +222,9 @@ def test_transformation_script(self, psyclone_lfric_api): mock_transformation_script.assert_called_once_with(Path(__file__), None) # check transformation_script is passed to psyclone command with '-s' mock_run_command.assert_called_with( - additional_parameters=['-api', psyclone_lfric_api, '-l', 'all', + additional_parameters=['-api', psyclone_lfric_api, '-opsy', Path(__file__), '-oalg', Path(__file__), + '-l', 'all', '-s', Path(__file__), __file__]) diff --git a/tests/unit_tests/tools/test_psyclone.py b/tests/unit_tests/tools/test_psyclone.py index 7efc60ec..619c0260 100644 --- a/tests/unit_tests/tools/test_psyclone.py +++ b/tests/unit_tests/tools/test_psyclone.py @@ -10,9 +10,26 @@ from importlib import reload from unittest import mock +import pytest + from fab.tools import (Category, Psyclone) +def get_mock_result(version_info: str) -> mock.Mock: + '''Returns a mock PSyclone object that will return + the specified str as version info. + + :param version_info: the simulated output of psyclone --version + The leading "PSyclone version: " will be added automatically. + ''' + # The return of subprocess run has an attribute 'stdout', + # that returns the stdout when its `decode` method is called. + # So we mock stdout, then put this mock_stdout into the mock result: + mock_stdout = mock.Mock(decode=lambda: f"PSyclone version: {version_info}") + mock_result = mock.Mock(stdout=mock_stdout, returncode=0) + return mock_result + + def test_psyclone_constructor(): '''Test the PSyclone constructor.''' psyclone = Psyclone() @@ -20,45 +37,102 @@ def test_psyclone_constructor(): assert psyclone.name == "psyclone" assert psyclone.exec_name == "psyclone" assert psyclone.flags == [] - assert psyclone._api is None - psyclone = Psyclone(api="gocean1.0") - assert psyclone.category == Category.PSYCLONE - assert psyclone.name == "psyclone" - assert psyclone.exec_name == "psyclone" - assert psyclone.flags == [] - assert psyclone._api == "gocean1.0" - -def test_psyclone_check_available(): - '''Tests the is_available functionality.''' +def test_psyclone_check_available_2_4_0(): + '''Tests the is_available functionality with version 2.4.0. + We get only one call. + ''' psyclone = Psyclone() - mock_result = mock.Mock(returncode=0) + + mock_result = get_mock_result("2.4.0") with mock.patch('fab.tools.tool.subprocess.run', return_value=mock_result) as tool_run: assert psyclone.check_available() tool_run.assert_called_once_with( - ["psyclone", "--version"], capture_output=True, env=None, - cwd=None, check=False) + ["psyclone", "--version", mock.ANY], capture_output=True, + env=None, cwd=None, check=False) + + +def test_psyclone_check_available_2_5_0(): + '''Tests the is_available functionality with PSyclone 2.5.0. + We get two calls. First version, then check if nemo API exists + ''' + psyclone = Psyclone() + + mock_result = get_mock_result("2.5.0") + with mock.patch('fab.tools.tool.subprocess.run', + return_value=mock_result) as tool_run: + assert psyclone.check_available() + tool_run.assert_any_call( + ["psyclone", "--version", mock.ANY], capture_output=True, + env=None, cwd=None, check=False) + tool_run.assert_any_call( + ["psyclone", "-api", "nemo", mock.ANY], capture_output=True, + env=None, cwd=None, check=False) # Test behaviour if a runtime error happens: with mock.patch("fab.tools.tool.Tool.run", side_effect=RuntimeError("")) as tool_run: + with pytest.warns(UserWarning, + match="Unexpected version information " + "for PSyclone: ''."): + assert not psyclone.check_available() + + +def test_psyclone_check_available_after_2_5_0(): + '''Tests the is_available functionality with releases after 2.5.0. + We get two calls. First version, then check if nemo API exists + ''' + psyclone = Psyclone() + + # We detect the dummy version '2.5.0.1' if psyclone reports 2.5.0 + # but the command line option "-api nemo" is not accepted. + # So we need to return two results from our mock objects: first + # success for version 2.5.0, then a failure with an appropriate + # error message: + mock_result1 = get_mock_result("2.5.0") + mock_result2 = get_mock_result("Unsupported PSyKAL DSL / " + "API 'nemo' specified") + mock_result2.returncode = 1 + + # "Unsupported PSyKAL DSL / API 'nemo' specified" + with mock.patch('fab.tools.tool.subprocess.run', + return_value=mock_result1) as tool_run: + tool_run.side_effect = [mock_result1, mock_result2] + assert psyclone.check_available() + assert psyclone._version == (2, 5, 0, 1) + + +def test_psyclone_check_available_errors(): + '''Test various errors that can happen in check_available. + ''' + psyclone = Psyclone() + with mock.patch('fab.tools.tool.subprocess.run', + side_effect=FileNotFoundError("ERR")): assert not psyclone.check_available() + psyclone = Psyclone() + mock_result = get_mock_result("NOT_A_NUMBER.4.0") + with mock.patch('fab.tools.tool.subprocess.run', + return_value=mock_result): + with pytest.warns(UserWarning, + match="Unexpected version information for PSyclone: " + "'PSyclone version: NOT_A_NUMBER.4.0'"): + assert not psyclone.check_available() -def test_psyclone_process(psyclone_lfric_api): + +@pytest.mark.parametrize("api", ["dynamo0.3", "lfric"]) +def test_psyclone_process_api_2_4_0(api): '''Test running PSyclone.''' psyclone = Psyclone() - mock_result = mock.Mock(returncode=0) - # Create a mock function that returns a 'transformation script' - # called `script_called`: + mock_result = get_mock_result("2.4.0") transformation_function = mock.Mock(return_value="script_called") config = mock.Mock() with mock.patch('fab.tools.tool.subprocess.run', return_value=mock_result) as tool_run: psyclone.process(config=config, - api=psyclone_lfric_api, + api=api, x90_file="x90_file", psy_file="psy_file", alg_file="alg_file", @@ -66,60 +140,31 @@ def test_psyclone_process(psyclone_lfric_api): kernel_roots=["root1", "root2"], additional_parameters=["-c", "psyclone.cfg"]) tool_run.assert_called_with( - ['psyclone', '-api', psyclone_lfric_api, '-l', 'all', '-opsy', - 'psy_file', '-oalg', 'alg_file', '-s', 'script_called', '-c', + ['psyclone', '-api', 'dynamo0.3', '-opsy', 'psy_file', + '-oalg', 'alg_file', '-l', 'all', '-s', 'script_called', '-c', 'psyclone.cfg', '-d', 'root1', '-d', 'root2', 'x90_file'], capture_output=True, env=None, cwd=None, check=False) - # Don't specify an API: - with mock.patch('fab.tools.tool.subprocess.run', - return_value=mock_result) as tool_run: - psyclone.process(config=config, - x90_file="x90_file", - psy_file="psy_file", - alg_file="alg_file", - transformation_script=transformation_function, - kernel_roots=["root1", "root2"], - additional_parameters=["-c", "psyclone.cfg"]) - tool_run.assert_called_with( - ['psyclone', '-l', 'all', '-opsy', 'psy_file', '-oalg', 'alg_file', - '-s', 'script_called', '-c', - 'psyclone.cfg', '-d', 'root1', '-d', 'root2', 'x90_file'], - capture_output=True, env=None, cwd=None, check=False) - # Don't specify an API, but define an API on the PSyclone tool: - psyclone = Psyclone(api="gocean1.0") - with mock.patch('fab.tools.tool.subprocess.run', - return_value=mock_result) as tool_run: - psyclone.process(config=config, - x90_file="x90_file", - psy_file="psy_file", - alg_file="alg_file", - transformation_script=transformation_function, - kernel_roots=["root1", "root2"], - additional_parameters=["-c", "psyclone.cfg"]) - tool_run.assert_called_with( - ['psyclone', '-api', 'gocean1.0', '-l', 'all', '-opsy', 'psy_file', - '-oalg', 'alg_file', '-s', 'script_called', '-c', - 'psyclone.cfg', '-d', 'root1', '-d', 'root2', 'x90_file'], - capture_output=True, env=None, cwd=None, check=False) +def test_psyclone_process_no_api_2_4_0(): + '''Test running PSyclone.''' + psyclone = Psyclone() + mock_result = get_mock_result("2.4.0") + transformation_function = mock.Mock(return_value="script_called") + config = mock.Mock() - # Have both a default and a command line option - the latter - # must take precedence: - psyclone = Psyclone(api="gocean1.0") with mock.patch('fab.tools.tool.subprocess.run', return_value=mock_result) as tool_run: psyclone.process(config=config, + api="", x90_file="x90_file", - psy_file="psy_file", - alg_file="alg_file", - api=psyclone_lfric_api, + transformed_file="psy_file", transformation_script=transformation_function, kernel_roots=["root1", "root2"], additional_parameters=["-c", "psyclone.cfg"]) tool_run.assert_called_with( - ['psyclone', '-api', psyclone_lfric_api, '-l', 'all', '-opsy', - 'psy_file', '-oalg', 'alg_file', '-s', 'script_called', '-c', + ['psyclone', '-api', 'nemo', '-opsy', 'psy_file', '-l', 'all', + '-s', 'script_called', '-c', 'psyclone.cfg', '-d', 'root1', '-d', 'root2', 'x90_file'], capture_output=True, env=None, cwd=None, check=False) From 16d3ff5d59a6c73d0378da9345a9e8b3e89b1696 Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Fri, 27 Sep 2024 02:00:29 +1000 Subject: [PATCH 2/4] Fix mypy errors. --- source/fab/tools/psyclone.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/source/fab/tools/psyclone.py b/source/fab/tools/psyclone.py index 1d7fa255..424b3907 100644 --- a/source/fab/tools/psyclone.py +++ b/source/fab/tools/psyclone.py @@ -55,7 +55,6 @@ def check_available(self) -> bool: # Search for the version info: exp = r"PSyclone version: (\d[\d.]+\d)" - print("VERSION [", version_output, "]") matches = re.search(exp, version_output) if not matches: warnings.warn(f"Unexpected version information for PSyclone: " @@ -151,7 +150,6 @@ def process(self, # as parameter. No API is required if PSyclone works as # transformation tool only, so calling PSyclone without api is # actually valid. - print("API", api, self._version) if api: if self._version > (2, 5, 0): api_param = "--psykal-dsl" @@ -163,10 +161,16 @@ def process(self, # Mapping from new names to old names: mapping = {"lfric": "dynamo0.3", "gocean": "gocean1.0"} - + # Make mypy happy - we tested above that these variables + # are defined + assert psy_file + assert alg_file parameters.extend([api_param, mapping.get(api, api), "-opsy", psy_file, "-oalg", alg_file]) else: # no api + # Make mypy happy - we tested above that transformed_file is + # specified when no api is specified. + assert transformed_file if self._version > (2, 5, 0): # New version: no API, parameter, but -o for output name: parameters.extend(["-o", transformed_file]) From 71fd1aed179fb0e2b3655db9fb0704c1b76329b2 Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Mon, 30 Sep 2024 12:02:07 +1000 Subject: [PATCH 3/4] Added missing tests for calling psyclone, and converting old style to new stle arguments and vice versa. --- source/fab/tools/psyclone.py | 8 +- tests/unit_tests/tools/test_psyclone.py | 198 +++++++++++++++++++++++- 2 files changed, 196 insertions(+), 10 deletions(-) diff --git a/source/fab/tools/psyclone.py b/source/fab/tools/psyclone.py index 424b3907..fa508d7a 100644 --- a/source/fab/tools/psyclone.py +++ b/source/fab/tools/psyclone.py @@ -100,7 +100,7 @@ def process(self, kernel_roots: Optional[List[Union[str, Path]]] = None, api: Optional[str] = None, ): - # pylint: disable=too-many-arguments + # pylint: disable=too-many-arguments, too-many-branches '''Run PSyclone with the specified parameters. If PSyclone is used to transform existing Fortran files, `api` must be None, and the output file name is `transformed_file`. If PSyclone is using its DSL @@ -122,6 +122,10 @@ def process(self, if not self.is_available: raise RuntimeError("PSyclone is not available.") + # Convert the old style API nemo to be empty + if api and api.lower() == "nemo": + api = "" + if api: # API specified, we need both psy- and alg-file, but not # transformed file. @@ -143,7 +147,7 @@ def process(self, "alg_file is specified.") if not transformed_file: raise RuntimeError("PSyclone called without api, but " - "transformed_file it not specified.") + "transformed_file is not specified.") parameters: List[Union[str, Path]] = [] # If an api is defined in this call (or in the constructor) add it diff --git a/tests/unit_tests/tools/test_psyclone.py b/tests/unit_tests/tools/test_psyclone.py index 619c0260..5586c485 100644 --- a/tests/unit_tests/tools/test_psyclone.py +++ b/tests/unit_tests/tools/test_psyclone.py @@ -36,6 +36,7 @@ def test_psyclone_constructor(): assert psyclone.category == Category.PSYCLONE assert psyclone.name == "psyclone" assert psyclone.exec_name == "psyclone" + # pylint: disable=use-implicit-booleaness-not-comparison assert psyclone.flags == [] @@ -120,19 +121,82 @@ def test_psyclone_check_available_errors(): match="Unexpected version information for PSyclone: " "'PSyclone version: NOT_A_NUMBER.4.0'"): assert not psyclone.check_available() + # Also check that we can't call process if PSyclone is not available. + psyclone._is_available = False + config = mock.Mock() + with pytest.raises(RuntimeError) as err: + psyclone.process(config, "x90file") + assert "PSyclone is not available" in str(err.value) + + +def test_psyclone_processing_errors_without_api(): + '''Test all processing errors in PSyclone if no API is specified.''' + + psyclone = Psyclone() + psyclone._is_available = True + config = mock.Mock() + + # No API --> we need transformed file, but not psy or alg: + with pytest.raises(RuntimeError) as err: + psyclone.process(config, "x90file", api=None, psy_file="psy_file") + assert ("PSyclone called without api, but psy_file is specified" + in str(err.value)) + with pytest.raises(RuntimeError) as err: + psyclone.process(config, "x90file", api=None, alg_file="alg_file") + assert ("PSyclone called without api, but alg_file is specified" + in str(err.value)) + with pytest.raises(RuntimeError) as err: + psyclone.process(config, "x90file", api=None) + assert ("PSyclone called without api, but transformed_file is not " + "specified" in str(err.value)) @pytest.mark.parametrize("api", ["dynamo0.3", "lfric"]) -def test_psyclone_process_api_2_4_0(api): - '''Test running PSyclone.''' +def test_psyclone_processing_errors_with_api(api): + '''Test all processing errors in PSyclone if an API is specified.''' + psyclone = Psyclone() - mock_result = get_mock_result("2.4.0") + psyclone._is_available = True + config = mock.Mock() + + # No API --> we need transformed file, but not psy or alg: + with pytest.raises(RuntimeError) as err: + psyclone.process(config, "x90file", api=api, psy_file="psy_file") + assert (f"PSyclone called with api '{api}', but no alg_file is specified" + in str(err.value)) + with pytest.raises(RuntimeError) as err: + psyclone.process(config, "x90file", api=api, alg_file="alg_file") + assert (f"PSyclone called with api '{api}', but no psy_file is specified" + in str(err.value)) + with pytest.raises(RuntimeError) as err: + psyclone.process(config, "x90file", api=api, + psy_file="psy_file", alg_file="alg_file", + transformed_file="transformed_file") + assert (f"PSyclone called with api '{api}' and transformed_file" + in str(err.value)) + + +@pytest.mark.parametrize("version", ["2.4.0", "2.5.0"]) +@pytest.mark.parametrize("api", [("dynamo0.3", "dynamo0.3"), + ("lfric", "dynamo0.3"), + ("gocean1.0", "gocean1.0"), + ("gocean", "gocean1.0") + ]) +def test_psyclone_process_api_old_psyclone(api, version): + '''Test running 'old style' PSyclone (2.5.0 and earlier) with the old API + names (dynamo0.3 and gocean1.0). Also check that the new API names will + be accepted, but are mapped to the old style names. The 'api' parameter + contains the input api, and expected output API. + ''' + api_in, api_out = api + psyclone = Psyclone() + mock_result = get_mock_result(version) transformation_function = mock.Mock(return_value="script_called") config = mock.Mock() with mock.patch('fab.tools.tool.subprocess.run', return_value=mock_result) as tool_run: psyclone.process(config=config, - api=api, + api=api_in, x90_file="x90_file", psy_file="psy_file", alg_file="alg_file", @@ -140,16 +204,20 @@ def test_psyclone_process_api_2_4_0(api): kernel_roots=["root1", "root2"], additional_parameters=["-c", "psyclone.cfg"]) tool_run.assert_called_with( - ['psyclone', '-api', 'dynamo0.3', '-opsy', 'psy_file', + ['psyclone', '-api', api_out, '-opsy', 'psy_file', '-oalg', 'alg_file', '-l', 'all', '-s', 'script_called', '-c', 'psyclone.cfg', '-d', 'root1', '-d', 'root2', 'x90_file'], capture_output=True, env=None, cwd=None, check=False) -def test_psyclone_process_no_api_2_4_0(): - '''Test running PSyclone.''' +@pytest.mark.parametrize("version", ["2.4.0", "2.5.0"]) +def test_psyclone_process_no_api_old_psyclone(version): + '''Test running old-style PSyclone (2.5.0 and earlier) when requesting + to transform existing files by not specifying an API. We need to add + the flags `-api nemo` in this case for older PSyclone versions. + ''' psyclone = Psyclone() - mock_result = get_mock_result("2.4.0") + mock_result = get_mock_result(version) transformation_function = mock.Mock(return_value="script_called") config = mock.Mock() @@ -169,6 +237,119 @@ def test_psyclone_process_no_api_2_4_0(): capture_output=True, env=None, cwd=None, check=False) +@pytest.mark.parametrize("version", ["2.4.0", "2.5.0"]) +def test_psyclone_process_nemo_api_old_psyclone(version): + '''Test running old-style PSyclone (2.5.0 and earlier) when requesting + to transform existing files by specifying the nemo api. + ''' + + psyclone = Psyclone() + mock_result = get_mock_result(version) + transformation_function = mock.Mock(return_value="script_called") + config = mock.Mock() + + with mock.patch('fab.tools.tool.subprocess.run', + return_value=mock_result) as tool_run: + psyclone.process(config=config, + api="nemo", + x90_file="x90_file", + transformed_file="psy_file", + transformation_script=transformation_function, + kernel_roots=["root1", "root2"], + additional_parameters=["-c", "psyclone.cfg"]) + tool_run.assert_called_with( + ['psyclone', '-api', 'nemo', '-opsy', 'psy_file', '-l', 'all', + '-s', 'script_called', '-c', + 'psyclone.cfg', '-d', 'root1', '-d', 'root2', 'x90_file'], + capture_output=True, env=None, cwd=None, check=False) + + +@pytest.mark.parametrize("api", [("dynamo0.3", "lfric"), + ("lfric", "lfric"), + ("gocean1.0", "gocean"), + ("gocean", "gocean") + ]) +def test_psyclone_process_api_new__psyclone(api): + '''Test running the new PSyclone version. Since this version is not + yet released, we use the Fab internal version number 2.5.0.1 for + now. It uses new API names, and we need to check that the old style + names are converted to the new names. + ''' + api_in, api_out = api + psyclone = Psyclone() + mock_result = get_mock_result("2.5.0.1") + transformation_function = mock.Mock(return_value="script_called") + config = mock.Mock() + with mock.patch('fab.tools.tool.subprocess.run', + return_value=mock_result) as tool_run: + psyclone.process(config=config, + api=api_in, + x90_file="x90_file", + psy_file="psy_file", + alg_file="alg_file", + transformation_script=transformation_function, + kernel_roots=["root1", "root2"], + additional_parameters=["-c", "psyclone.cfg"]) + tool_run.assert_called_with( + ['psyclone', '--psykal-dsl', api_out, '-opsy', 'psy_file', + '-oalg', 'alg_file', '-l', 'all', '-s', 'script_called', '-c', + 'psyclone.cfg', '-d', 'root1', '-d', 'root2', 'x90_file'], + capture_output=True, env=None, cwd=None, check=False) + + +def test_psyclone_process_no_api_new_psyclone(): + '''Test running the new PSyclone version without an API. Since this + version is not yet released, we use the Fab internal version number + 2.5.0.1 for now. + ''' + psyclone = Psyclone() + mock_result = get_mock_result("2.5.0.1") + transformation_function = mock.Mock(return_value="script_called") + config = mock.Mock() + + with mock.patch('fab.tools.tool.subprocess.run', + return_value=mock_result) as tool_run: + psyclone.process(config=config, + api="", + x90_file="x90_file", + transformed_file="psy_file", + transformation_script=transformation_function, + kernel_roots=["root1", "root2"], + additional_parameters=["-c", "psyclone.cfg"]) + tool_run.assert_called_with( + ['psyclone', '-o', 'psy_file', '-l', 'all', + '-s', 'script_called', '-c', + 'psyclone.cfg', '-d', 'root1', '-d', 'root2', 'x90_file'], + capture_output=True, env=None, cwd=None, check=False) + + +def test_psyclone_process_nemo_api_new_psyclone(): + '''Test running PSyclone. Since this version is not yet released, we use + the Fab internal version number 2.5.0.1 for now. This tests that + backwards compatibility of using the nemo api works, i.e. '-api nemo' is + just removed. + ''' + psyclone = Psyclone() + mock_result = get_mock_result("2.5.0.1") + transformation_function = mock.Mock(return_value="script_called") + config = mock.Mock() + + with mock.patch('fab.tools.tool.subprocess.run', + return_value=mock_result) as tool_run: + psyclone.process(config=config, + api="nemo", + x90_file="x90_file", + transformed_file="psy_file", + transformation_script=transformation_function, + kernel_roots=["root1", "root2"], + additional_parameters=["-c", "psyclone.cfg"]) + tool_run.assert_called_with( + ['psyclone', '-o', 'psy_file', '-l', 'all', + '-s', 'script_called', '-c', + 'psyclone.cfg', '-d', 'root1', '-d', 'root2', 'x90_file'], + capture_output=True, env=None, cwd=None, check=False) + + def test_type_checking_import(): '''PSyclone contains an import of TYPE_CHECKING to break a circular dependency. In order to reach 100% coverage of PSyclone, we set @@ -178,5 +359,6 @@ def test_type_checking_import(): with mock.patch('typing.TYPE_CHECKING', True): # This import will not actually re-import, since the module # is already imported. But we need this in order to call reload: + # pylint: disable=import-outside-toplevel import fab.tools.psyclone reload(fab.tools.psyclone) From ec4c0f6cd01dbb01f6ea42792a37e3991189137b Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Mon, 30 Sep 2024 13:52:05 +1000 Subject: [PATCH 4/4] Updated comment. --- source/fab/tools/psyclone.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source/fab/tools/psyclone.py b/source/fab/tools/psyclone.py index fa508d7a..cbf12a9f 100644 --- a/source/fab/tools/psyclone.py +++ b/source/fab/tools/psyclone.py @@ -82,7 +82,8 @@ def check_available(self) -> bool: except RuntimeError as err: if "Unsupported PSyKAL DSL / API 'nemo' specified" in str(err): # It is current development. Just give it a version number - # greater than 2.5.0 + # greater than 2.5.0 for now, till the official release + # is done. version = (2, 5, 0, 1) self._version = version