Merge branch 'add_shell_tool_clean' into update_to_fparser_0_2

source/fab/tools/psyclone.py

@@ -38,20 +38,10 @@ def check_available(self) -> bool:
 
         # 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)
+            version_output = self.run(["--version"], capture_output=True)
+        except RuntimeError:
+            # Something is wrong, report as not available
+            return False
 
         # Search for the version info:
         exp = r"PSyclone version: (\d[\d.]+\d)"
@@ -64,29 +54,8 @@ def check_available(self) -> bool:
 
         # 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 for now, till the official release
-                    # is done.
-                    version = (2, 5, 0, 1)
-
-        self._version = version
+        self._version = tuple(int(x) for x in matches.groups()[0].split('.'))
+
         return True
 
     def process(self,
@@ -156,7 +125,7 @@ def process(self,
         # transformation tool only, so calling PSyclone without api is
         # actually valid.
         if api:
-            if self._version > (2, 5, 0):
+            if self._version >= (3, 0, 0):
                 api_param = "--psykal-dsl"
                 # Mapping from old names to new names:
                 mapping = {"dynamo0.3": "lfric",
@@ -176,11 +145,11 @@ def process(self,
             # 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):
+            if self._version >= (3, 0, 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 -opsy
+                # 2.5.0 or earlier: needs api nemo, output name is -oalg
                 parameters.extend(["-api", "nemo", "-opsy", transformed_file])
         parameters.extend(["-l", "all"])
 

tests/unit_tests/tools/test_psyclone.py

@@ -40,70 +40,24 @@ def test_psyclone_constructor():
     assert psyclone.flags == []
 
 
-def test_psyclone_check_available_2_4_0():
-    '''Tests the is_available functionality with version 2.4.0.
-    We get only one call.
+@pytest.mark.parametrize("version", ["2.4.0", "2.5.0", "3.0.0", "3.1.0"])
+def test_psyclone_check_available_and_version(version):
+    '''Tests the is_available functionality and version number detection
+    with PSyclone. Note that the version number is only used internally,
+    so we test with the private attribute.
     '''
     psyclone = Psyclone()
-
-    mock_result = get_mock_result("2.4.0")
+    version_tuple = tuple(int(i) for i in version.split("."))
+    mock_result = get_mock_result(version)
     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", 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()
+        assert psyclone._version == version_tuple
 
-    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,
+    tool_run.assert_called_once_with(
+        ["psyclone", "--version"], 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.
@@ -270,14 +224,12 @@ def test_psyclone_process_nemo_api_old_psyclone(version):
                                  ("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.
+    '''Test running PSyclone 3.0.0. 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")
+    mock_result = get_mock_result("3.0.0")
     transformation_function = mock.Mock(return_value="script_called")
     config = mock.Mock()
     with mock.patch('fab.tools.tool.subprocess.run',
@@ -298,12 +250,11 @@ def test_psyclone_process_api_new_psyclone(api):
 
 
 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.
+    '''Test running the PSyclone 3.0.0 without an API, i.e. as transformation
+    only.
     '''
     psyclone = Psyclone()
-    mock_result = get_mock_result("2.5.0.1")
+    mock_result = get_mock_result("3.0.0")
     transformation_function = mock.Mock(return_value="script_called")
     config = mock.Mock()
 
@@ -324,13 +275,11 @@ def test_psyclone_process_no_api_new_psyclone():
 
 
 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.
+    '''Test running PSyclone 3.0.0 and test 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")
+    mock_result = get_mock_result("3.0.0")
     transformation_function = mock.Mock(return_value="script_called")
     config = mock.Mock()