From 546ee9e6b65ea9437b08974e193fb737cb285b96 Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Fri, 9 Aug 2024 17:41:59 +1000 Subject: [PATCH] Generalisation (#330) * Removed absolute path patch, since this breaks a test. * Fixed failing tests. * Fix styling problems reported by Flake8 * Fix "_metric_send_conn not set, cannot send metrics" warning not issued, as reported by CI pipeline on Github * Remove cosmetic change done to follow other coding standards. * Remove more cosmetic changes (which pylint required). * More removal of cosmetic changes, and some useless code. * Add --links option to rsync. * Extend Fortran analyser to detect use statements with an OpenMP sentinel. * Use existing variable. * Remove unused glob import. * Removed unused glob import. * Fixed typo picked up in review. * Fixed failing tests. * Replaced _artefact_store with getter as much as possible. * #3 Introduce Tool, Compiler, and ToolRepository. * #3 Made ToolRepository a singleton. * #3 Added ToolBox. * #3 Added dedicated enum for categories. * #3 Add the category to a tool. * #3 Use tool category when adding a tool to the tool box. * #3 Added preprocessor, and dictionary-like accesses to ToolBox. * #3 Added tool_box as mandatory parameter for a BuildConfig. * #3 Start to use new compiler objects for Fortran compilation. * #3 Added tests for Compiler class. * #3 Add specific C- and Fortran-compiler classes. Updated tests. * #3 Fixed typing. * #3 Moved function to remove into Flags object, added test file for Flags. * # Fixed tests to work with new Fortran compiler handling. * #3 Use for compiler variables if defined. * #3 Add is_available flag to tools, added types. * #3 Added cpp as Fortran preprocessor. * #3 Removed unnecessary code. * #3 Properly ignore mypy warnings. * Support ToolBox for preprocessing. * #3 Add a specific 'preprocess' method to Preprocessor. * #3 Changed order in which compiler flags are used. * #3 Use toolbox in compile_c step. * #3 Added get_hash function to compiler. * #3 Added conftest.py file (with a fixture to create a C-compiler). * #3 Introduce fixtures for Fortran compiler and tool_box. * #3 Remove explicit compiler information from MpCommonArgs (since it's already part of the config toolbox). * #3 Added linker as tool. * #3 Added test for linking shared libraries. * #3 Pass compiler flags to the linker if a compiler was specified. * #3 Remove unused function. * #3 Removed more unused code. * #3 Automatically add a linker for each compiler. * #3 Fixed typo. * #3 Support vendor for compiler and linker. * #3 Make linker having a vendor, too. * #3 Add set_default_vendor method to tool repository. * Ignore build directory for git. * Updated test. * # Fix some mypy errors and warnings. * Avoid using get() for singleton, instead use __new__ which makes mypy happier. * Changed the transformation_script parameter of function psyclone to accept a function that can return file-specific transformation scripts * Make mypy happy by using patch.object. * Remove more comments and confusion about mypy :) * Try to make mypy happy on older python versions. * Make flake8 happy. * Sort imported name alphabetically. * Try to fix failing hash test (and add some additional improvements in the test). * Removed fpath= for input transformation_script function to pass mypy test for Python 3.7; Moved transformation_script_hash test to unit test from system test * Fix mypy typing check errors for psyclone unit test * Fix config typing issue with mypy in psyclone unit test * Fix flake8 issues; Revert Config mypy typing fix * Add comment to ignore typing check for fpath parameter of input transformation_script function * Fix assert check after transformation_script function is changed from being called twice to once * Filter out 'no transformation script' warning for psyclone system test * Replace 'ignore' typing of fpath of transformation_script with removing keyword argument * #3 Support proper tests to check if tools are available. * 1. Updated transformation_script description; 2. Modified mock_transformation_script; 3.Removed redundant _analysis_for_prebuilds * Updated lfric/atm.py and lfric/gungho.py examples to pass in transformation_script functions * Added description for the psyclone step to instructions on writing a config * #3 Added git as a tool. * #3 Fix incorrect | usage in typing. * #3 Added unit tests for git. * #3 Renamed git.py to versioning.py, to avoid name clash with the corresponding test_git.py tests. * #3 Converted svn and fcm to tools. * #3 Fixed missing whitespace. * Modified the documentation for writing a config with PSyclone * Add config as a parameter for run_psyclone for the transformation_script to use;Updated the related functions and tests; Changed the logic of the transformation_script examples * #3 Replaced ar with tool object. * #3 Added tests for ar.py. * #3 Removed debug output. * #3 Converted PSyclone to be a tool. * #3 Removed debug print, fixed python 3.7 typing information. * #3 Updated comments. * Modified the get_optimisation_script function examples and updated the doc formatting * #3 Add Rsync tool. * #3 Removed now unused function. * #3 Added test for rsync. * #3 Fixed all mypy warnings about functions not checked. * #3 Replace all mock-tests to use subprocess so the name of the executable is tested as well. * #3 Remove duplicated flags. * #3 Fixed changed order of linking. * #3 Removed run_command function. * #3 Fixed 3.8 typing error. * #3 Fixed unused imports. * #3 Move flags checksum into Flags, and remove now unused tools.py file. * #3 Renamed newtools to tools. * #3 Made custom function for all git functions called (instead of just calling run). * #3 Updated and fixed comments. * #3 Fixed errors in comments. * Fixed minor errors in documentation. * #3 Make it easier to create wrapper around standard compiler. * #3 Added documentation for all tool related classes and their usage. * #3 Added MISC category. * Addressed reviewer's comments. * Updated cli to properly use ToolBox etc, removing hard-coded gnu command linker option. * Fixed mypy failures, including changes to import statement to avoid cyclic imports :(. * #3 Fix circular import. * Added #TODO so that this can be removed once fparser supports sentinels. * Fix typing problems by ignoring fparser. * Replaced more string names for artefacts with enums. * Removed EXECUTABLES from constants. * Moved Artefact class out of ArtefactStore and renamed it to ArtefactSet. * Moved OBJECT_FILES from constants into ArtefactSet. * Moved OBJECT_ARCHIVES from constants to ArtefactSet. * Moved PRAGMAD_C from constants to ArtefactSet. * Turned 'all_source' into an enum. * Allow integer as revision. * Fixed flake8 error. * Removed specific functions to add/get fortran source files etc. * Removed non-existing and unneccessary collections. * Try to fix all run_configs. * Fixed rebase issues. * Added replace functionality to ArtefactStore, updated test_artefacts to cover all lines in that file. * Started to replace artefacts when files are pre-processed. * Removed linker argument from linking step in all examples. * Try to get jules to link. * Fixed build_jules. * Fixed other issues raised in reviews. * Try to get jules to link. * Fixed other issues raised in reviews. * Simplify handling of X90 files by replacing the X90 with x90, meaning only one artefact set is involved when running PSyclone. * Make OBJECT_ARCHIVES also a dict, migrate more code to replace/add files to the default build artefact collections. * Fixed some examples. * Fix flake8 error. * Fixed failing tests. * Support empty comments. * Fix preprocessor to not unnecessary remove and add files that are already in the output directory. * Allow find_soure_files to be called more than once by adding files (not replacing artefact). * Updated lfric_common so that files created by configurator are written in build (not source). * Use c_build_files instead of pragmad_c. * Removed unnecessary str. * Documented the new artefact set handling. * Fixed typo. * Make the PSyclone API configurable. * Fixed formatting of documentation, properly used ArtefactSet names. * Support .f and .F Fortran files. * Removed setter for tool.is_available, which was only used for testing. * #3 Fix documentation and coding style issues from review. * Renamed Categories into Category. * Minor coding style cleanup. * Removed more unnecessary (). * Re-added (invalid) grab_pre_build call. * Fixed typo. * Renamed set_default_vendor to set_default_compiler_suite. * Renamed VendorTool to CompilerSuiteTool. * Also accept a Path as exec_name specification for a tool. * Move the check_available function into the base class. * Fixed some types and documentation. * Fix typing error. * Added explanation for meta-compiler. * Improved error handling and documentation. * Replace mpiifort with mpifort to be a tiny bit more portable. * Use classes to group tests for git/svn/fcm together. * Fixed issue in get_transformation script, and moved script into lfric_common to remove code duplication. * Code improvement as suggested by review. * Fixed run config * Added reference to ticket. * Updated type information. * More typing fixes. * Fixed typing warnings. * As requested by reviewer removed is_working_copy functionality. * Issue a warning (which can be silenced) when a tool in a toolbox is replaced. * Fixed flake8. * Fixed flake8. * Fixed failing test. * Addressed issues raised in review. * Removed now unnecessary operations. * Updated some type information. * Fixed all references to APIs to be consistent with PSyclone 2.5. * Added api to the checksum computation. * Fixed type information. * Added test to verify that changing the api changes the checksum. * Provide the name of the PSyclone API for LFRic as a conftest (to make it easier to update when the name changes with the next PSyclone release. * Define API constant in lfric_common to have the correct PSyclone API. * Fixed flake8 error. --------- Co-authored-by: Junwei Lyu Co-authored-by: Junwei Lyu Co-authored-by: jasonjunweilyu <161689601+jasonjunweilyu@users.noreply.github.com> Co-authored-by: Junwei Lyu Co-authored-by: Matthew Hambley Co-authored-by: Junwei Lyu Co-authored-by: Junwei Lyu Co-authored-by: Junwei Lyu --- run_configs/lfric/atm.py | 3 +- run_configs/lfric/gungho.py | 3 +- run_configs/lfric/lfric_common.py | 2 + run_configs/lfric/mesh_tools.py | 3 +- source/fab/steps/analyse.py | 2 +- source/fab/steps/compile_fortran.py | 2 +- source/fab/steps/find_source_files.py | 2 +- source/fab/steps/psyclone.py | 19 ++++-- source/fab/tools/psyclone.py | 22 +++++-- tests/conftest.py | 7 ++ .../psyclone/test_psyclone_system_test.py | 20 +++--- .../steps/test_psyclone_unit_test.py | 26 ++++++-- tests/unit_tests/tools/test_psyclone.py | 66 ++++++++++++++++++- 13 files changed, 141 insertions(+), 36 deletions(-) diff --git a/run_configs/lfric/atm.py b/run_configs/lfric/atm.py index 7a085733..f1c31017 100755 --- a/run_configs/lfric/atm.py +++ b/run_configs/lfric/atm.py @@ -20,7 +20,7 @@ from fab.tools import ToolBox from grab_lfric import lfric_source_config, gpl_utils_source_config -from lfric_common import (configurator, fparser_workaround_stop_concatenation, +from lfric_common import (API, configurator, fparser_workaround_stop_concatenation, get_transformation_script) logger = logging.getLogger('fab') @@ -247,6 +247,7 @@ def file_filtering(config): kernel_roots=[state.build_output / 'lfric' / 'kernel'], transformation_script=get_transformation_script, cli_args=[], + api=API, ) # todo: do we need this one in here? diff --git a/run_configs/lfric/gungho.py b/run_configs/lfric/gungho.py index 2f90a41a..caf59216 100755 --- a/run_configs/lfric/gungho.py +++ b/run_configs/lfric/gungho.py @@ -22,7 +22,7 @@ from fab.tools import ToolBox from grab_lfric import lfric_source_config, gpl_utils_source_config -from lfric_common import (configurator, fparser_workaround_stop_concatenation, +from lfric_common import (API, configurator, fparser_workaround_stop_concatenation, get_transformation_script) logger = logging.getLogger('fab') @@ -72,6 +72,7 @@ kernel_roots=[state.build_output], transformation_script=get_transformation_script, cli_args=[], + api=API, ) fparser_workaround_stop_concatenation(state) diff --git a/run_configs/lfric/lfric_common.py b/run_configs/lfric/lfric_common.py index a330b1d5..fe8eae03 100644 --- a/run_configs/lfric/lfric_common.py +++ b/run_configs/lfric/lfric_common.py @@ -12,6 +12,8 @@ logger = logging.getLogger('fab') +API = "dynamo0.3" + class Script(Tool): '''A simple wrapper that runs a shell script. diff --git a/run_configs/lfric/mesh_tools.py b/run_configs/lfric/mesh_tools.py index 634b7834..f49aa43b 100755 --- a/run_configs/lfric/mesh_tools.py +++ b/run_configs/lfric/mesh_tools.py @@ -13,7 +13,7 @@ from fab.steps.psyclone import psyclone, preprocess_x90 from fab.tools import ToolBox -from lfric_common import configurator, fparser_workaround_stop_concatenation +from lfric_common import API, configurator, fparser_workaround_stop_concatenation from grab_lfric import lfric_source_config, gpl_utils_source_config @@ -57,6 +57,7 @@ kernel_roots=[state.build_output], cli_args=['--config', Path(__file__).parent / 'psyclone.cfg'], overrides_folder=state.source_root / 'mesh_tools_overrides', + api=API, ) fparser_workaround_stop_concatenation(state) diff --git a/source/fab/steps/analyse.py b/source/fab/steps/analyse.py index 091da6c0..1b739e56 100644 --- a/source/fab/steps/analyse.py +++ b/source/fab/steps/analyse.py @@ -239,7 +239,7 @@ def _parse_files(config, files: List[Path], fortran_analyser, c_analyser) -> Set """ # fortran - fortran_files = set(filter(lambda f: f.suffix == '.f90', files)) + fortran_files = set(filter(lambda f: f.suffix in ['.f90', '.f'], files)) with TimerLogger(f"analysing {len(fortran_files)} preprocessed fortran files"): fortran_results = run_mp(config, items=fortran_files, func=fortran_analyser.run) fortran_analyses, fortran_artefacts = zip(*fortran_results) if fortran_results else (tuple(), tuple()) diff --git a/source/fab/steps/compile_fortran.py b/source/fab/steps/compile_fortran.py index 38146b1b..fa734583 100644 --- a/source/fab/steps/compile_fortran.py +++ b/source/fab/steps/compile_fortran.py @@ -28,7 +28,7 @@ logger = logging.getLogger(__name__) -DEFAULT_SOURCE_GETTER = FilterBuildTrees(suffix='.f90') +DEFAULT_SOURCE_GETTER = FilterBuildTrees(suffix=['.f', '.f90']) @dataclass diff --git a/source/fab/steps/find_source_files.py b/source/fab/steps/find_source_files.py index 570b0efc..52b03e0a 100644 --- a/source/fab/steps/find_source_files.py +++ b/source/fab/steps/find_source_files.py @@ -152,7 +152,7 @@ def find_source_files(config, source_root=None, # Fortran, C, and PSyclone config.artefact_store.copy_artefacts(output_collection, ArtefactSet.FORTRAN_BUILD_FILES, - suffixes=[".f90", ".F90"]) + suffixes=[".f", ".F", ".f90", ".F90"]) config.artefact_store.copy_artefacts(output_collection, ArtefactSet.C_BUILD_FILES, diff --git a/source/fab/steps/psyclone.py b/source/fab/steps/psyclone.py index 5ed53904..04c1cc27 100644 --- a/source/fab/steps/psyclone.py +++ b/source/fab/steps/psyclone.py @@ -15,7 +15,7 @@ import warnings from itertools import chain from pathlib import Path -from typing import Dict, List, Optional, Set, Tuple, Union, Callable +from typing import Callable, Dict, List, Optional, Set, Tuple, Union from fab.build_config import BuildConfig @@ -70,7 +70,7 @@ class MpCommonArgs: kernel_roots: List[Union[str, Path]] transformation_script: Optional[Callable[[Path, BuildConfig], Path]] cli_args: List[str] - + api: Union[str, None] all_kernel_hashes: Dict[str, int] overrides_folder: Optional[Path] override_files: List[str] # filenames (not paths) of hand crafted overrides @@ -85,7 +85,8 @@ def psyclone(config, kernel_roots: Optional[List[Path]] = None, transformation_script: Optional[Callable[[Path, BuildConfig], Path]] = None, cli_args: Optional[List[str]] = None, source_getter: Optional[ArtefactsGetter] = None, - overrides_folder: Optional[Path] = None): + overrides_folder: Optional[Path] = None, + api: Optional[str] = None): """ Psyclone runner step. @@ -132,7 +133,8 @@ def psyclone(config, kernel_roots: Optional[List[Path]] = None, # get the data in a payload object for child processes to calculate prebuild hashes mp_payload = _generate_mp_payload( - config, analysed_x90, all_kernel_hashes, overrides_folder, kernel_roots, transformation_script, cli_args) + config, analysed_x90, all_kernel_hashes, overrides_folder, + kernel_roots, transformation_script, cli_args, api=api) # run psyclone. # for every file, we get back a list of its output files plus a list of the prebuild copies. @@ -163,7 +165,8 @@ def psyclone(config, kernel_roots: Optional[List[Path]] = None, def _generate_mp_payload(config, analysed_x90, all_kernel_hashes, overrides_folder, kernel_roots, - transformation_script, cli_args) -> MpCommonArgs: + transformation_script, cli_args, + api: Union[str, None]) -> MpCommonArgs: override_files: List[str] = [] if overrides_folder: override_files = [f.name for f in file_walk(overrides_folder)] @@ -175,6 +178,7 @@ def _generate_mp_payload(config, analysed_x90, all_kernel_hashes, overrides_fold cli_args=cli_args, analysed_x90=analysed_x90, all_kernel_hashes=all_kernel_hashes, + api=api, overrides_folder=overrides_folder, override_files=override_files, ) @@ -308,7 +312,7 @@ def do_one_file(arg: Tuple[Path, MpCommonArgs]): transformation_script = mp_payload.transformation_script logger.info(f"running psyclone on '{x90_file}'.") psyclone.process(config=mp_payload.config, - api="dynamo0.3", + api=mp_payload.api, x90_file=x90_file, psy_file=psy_file, alg_file=modified_alg, @@ -385,6 +389,9 @@ def _gen_prebuild_hash(x90_file: Path, mp_payload: MpCommonArgs): # command-line arguments string_checksum(str(mp_payload.cli_args)), + + # the API + string_checksum(str(mp_payload.api)), ]) return prebuild_hash diff --git a/source/fab/tools/psyclone.py b/source/fab/tools/psyclone.py index af453178..1a2b3b40 100644 --- a/source/fab/tools/psyclone.py +++ b/source/fab/tools/psyclone.py @@ -24,10 +24,11 @@ class Psyclone(Tool): '''This is the base class for `PSyclone`. ''' - def __init__(self): + def __init__(self, api: Optional[str] = None): super().__init__("psyclone", "psyclone", Category.PSYCLONE) + self._api = api - def process(self, api: str, + def process(self, config: "BuildConfig", x90_file: Path, psy_file: Path, @@ -35,7 +36,8 @@ def process(self, api: str, transformation_script: Optional[Callable[[Path, "BuildConfig"], Path]] = None, additional_parameters: Optional[List[str]] = None, - kernel_roots: Optional[List[Union[str, Path]]] = None + kernel_roots: Optional[List[Union[str, Path]]] = None, + api: Optional[str] = None, ): # pylint: disable=too-many-arguments '''Run PSyclone with the specified parameters. @@ -50,8 +52,18 @@ def process(self, api: str, :param kernel_roots: optional directories with kernels. ''' - parameters: List[Union[str, Path]] = [ - "-api", api, "-l", "all", "-opsy", psy_file, "-oalg", alg_file] + 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. + if api: + parameters.extend(["-api", api]) + elif self._api: + parameters.extend(["-api", self._api]) + + parameters.extend(["-l", "all", "-opsy", psy_file, "-oalg", alg_file]) + if transformation_script: transformation_script_return_path = \ transformation_script(x90_file, config) diff --git a/tests/conftest.py b/tests/conftest.py index b8a95011..bd7d5a87 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -58,3 +58,10 @@ def fixture_tool_box(mock_c_compiler, mock_fortran_compiler, mock_linker): tool_box.add_tool(mock_fortran_compiler) tool_box.add_tool(mock_linker) return tool_box + + +@pytest.fixture(name="psyclone_lfric_api") +def fixture_psyclone_lfric_api(): + '''A simple fixture to provide the name of the LFRic API for + PSyclone.''' + return "dynamo0.3" diff --git a/tests/system_tests/psyclone/test_psyclone_system_test.py b/tests/system_tests/psyclone/test_psyclone_system_test.py index 325e5d2e..cf3c80d0 100644 --- a/tests/system_tests/psyclone/test_psyclone_system_test.py +++ b/tests/system_tests/psyclone/test_psyclone_system_test.py @@ -131,7 +131,7 @@ def config(self, tmp_path): multiprocessing=False) return config - def steps(self, config): + def steps(self, config, psyclone_lfric_api): here = Path(__file__).parent grab_folder(config, here / 'skeleton') find_source_files(config) @@ -145,9 +145,9 @@ def steps(self, config): config.build_output / 'kernel', # this second folder is just to test the multiple folders code, which was bugged. There's no kernels there. Path(__file__).parent / 'skeleton/algorithm', - ]) + ], api=psyclone_lfric_api) - def test_run(self, config): + def test_run(self, config, psyclone_lfric_api): # if these files exist after the run then we know: # a) the expected files were created # b) the prebuilds were protected from automatic cleanup @@ -171,20 +171,20 @@ def test_run(self, config): assert all(list(config.prebuild_folder.glob(f)) == [] for f in expect_prebuild_files) assert all(list(config.build_output.glob(f)) == [] for f in expect_build_files) with config, pytest.warns(UserWarning, match="no transformation script specified"): - self.steps(config) + self.steps(config, psyclone_lfric_api) assert all(list(config.prebuild_folder.glob(f)) != [] for f in expect_prebuild_files) assert all(list(config.build_output.glob(f)) != [] for f in expect_build_files) - def test_prebuild(self, tmp_path, config): + def test_prebuild(self, tmp_path, config, psyclone_lfric_api): with config, pytest.warns(UserWarning, match="no transformation script specified"): - self.steps(config) + self.steps(config, psyclone_lfric_api) # make sure no work gets done the second time round with mock.patch('fab.parse.x90.X90Analyser.walk_nodes') as mock_x90_walk, \ mock.patch('fab.parse.fortran.FortranAnalyser.walk_nodes') as mock_fortran_walk, \ mock.patch('fab.tools.psyclone.Psyclone.process') as mock_run, \ config, pytest.warns(UserWarning, match="no transformation script specified"): - self.steps(config) + self.steps(config, psyclone_lfric_api) mock_x90_walk.assert_not_called() mock_fortran_walk.assert_not_called() @@ -197,12 +197,12 @@ class TestTransformationScript: and whether transformation script is passed to psyclone after '-s'. """ - def test_transformation_script(self): + def test_transformation_script(self, psyclone_lfric_api): psyclone_tool = Psyclone() 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__) - psyclone_tool.process(api="dynamo0.3", + psyclone_tool.process(api=psyclone_lfric_api, x90_file=Path(__file__), psy_file=Path(__file__), alg_file=Path(__file__), @@ -216,7 +216,7 @@ def test_transformation_script(self): 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', 'dynamo0.3', '-l', 'all', + additional_parameters=['-api', psyclone_lfric_api, '-l', 'all', '-opsy', Path(__file__), '-oalg', Path(__file__), '-s', Path(__file__), diff --git a/tests/unit_tests/steps/test_psyclone_unit_test.py b/tests/unit_tests/steps/test_psyclone_unit_test.py index 13980c0d..7790bc7d 100644 --- a/tests/unit_tests/steps/test_psyclone_unit_test.py +++ b/tests/unit_tests/steps/test_psyclone_unit_test.py @@ -11,16 +11,17 @@ from fab.parse.x90 import AnalysedX90 from fab.steps.psyclone import _check_override, _gen_prebuild_hash, MpCommonArgs -from fab.util import file_checksum +from fab.util import file_checksum, string_checksum -class Test_gen_prebuild_hash(object): +class TestGenPrebuildHash: """ Tests for the prebuild hashing calculation. """ @pytest.fixture - def data(self, tmp_path) -> Tuple[MpCommonArgs, Path, int]: + def data(self, tmp_path, psyclone_lfric_api) -> Tuple[MpCommonArgs, + Path, int]: x90_file = Path('foo.x90') analysed_x90 = { @@ -38,8 +39,7 @@ def data(self, tmp_path) -> Tuple[MpCommonArgs, Path, int]: # the script is just hashed later, so any one will do - use this file! mock_transformation_script = mock.Mock(return_value=__file__) - expect_hash = 223133492 + file_checksum(__file__).file_hash # add the transformation_script_hash - + expect_hash = 3962584109 + file_checksum(__file__).file_hash # add the transformation_script_hash mp_payload = MpCommonArgs( analysed_x90=analysed_x90, all_kernel_hashes=all_kernel_hashes, @@ -47,6 +47,7 @@ def data(self, tmp_path) -> Tuple[MpCommonArgs, Path, int]: config=None, # type: ignore[arg-type] kernel_roots=[], transformation_script=mock_transformation_script, + api=psyclone_lfric_api, overrides_folder=None, override_files=None, # type: ignore[arg-type] ) @@ -80,6 +81,19 @@ def test_trans_script(self, data): # transformation_script_hash = 0 assert result == expect_hash - file_checksum(__file__).file_hash + def test_api(self, data): + # changing PSyclone's API should change the hash + mp_payload, x90_file, expect_hash = data + old_hash = string_checksum(mp_payload.api) + # Change the API by appending "_new" + mp_payload.api = mp_payload.api + "_new" + result = _gen_prebuild_hash(x90_file=x90_file, mp_payload=mp_payload) + # transformation_script_hash = 0 + new_hash = string_checksum(mp_payload.api) + # Make sure we really changed the + assert new_hash != old_hash + assert result == expect_hash - old_hash + new_hash + def test_cli_args(self, data): # changing the cli args should change the hash mp_payload, x90_file, expect_hash = data @@ -88,7 +102,7 @@ def test_cli_args(self, data): assert result != expect_hash -class Test_check_override(object): +class TestCheckOverride: def test_no_override(self): mp_payload = mock.Mock(overrides_folder=Path('/foo'), override_files=[Path('/foo/bar.f90')]) diff --git a/tests/unit_tests/tools/test_psyclone.py b/tests/unit_tests/tools/test_psyclone.py index 7d534fe2..ad7817c2 100644 --- a/tests/unit_tests/tools/test_psyclone.py +++ b/tests/unit_tests/tools/test_psyclone.py @@ -19,6 +19,14 @@ 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(): @@ -38,7 +46,7 @@ def test_psyclone_check_available(): assert not psyclone.check_available() -def test_psyclone_process(): +def test_psyclone_process(psyclone_lfric_api): '''Test running PSyclone.''' psyclone = Psyclone() mock_result = mock.Mock(returncode=0) @@ -49,7 +57,40 @@ def test_psyclone_process(): with mock.patch('fab.tools.tool.subprocess.run', return_value=mock_result) as tool_run: psyclone.process(config=config, - api="dynamo0.3", + api=psyclone_lfric_api, + 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', psyclone_lfric_api, '-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: + 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", @@ -57,7 +98,26 @@ def test_psyclone_process(): kernel_roots=["root1", "root2"], additional_parameters=["-c", "psyclone.cfg"]) tool_run.assert_called_with( - ['psyclone', '-api', 'dynamo0.3', '-l', 'all', '-opsy', 'psy_file', + ['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) + + # 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, + x90_file="x90_file", + psy_file="psy_file", + alg_file="alg_file", + api=psyclone_lfric_api, + 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.cfg', '-d', 'root1', '-d', 'root2', 'x90_file'], + capture_output=True, env=None, cwd=None, check=False)