Get tests passing again.

source/fab/steps/analyse.py

@@ -78,7 +78,7 @@ def analyse(
         special_measure_analysis_results: Optional[Iterable[FortranParserWorkaround]] = None,
         unreferenced_deps: Optional[Iterable[str]] = None,
         ignore_mod_deps: Optional[Iterable[str]] = None,
-        name='analyser'):
+        name='analyser') -> None:
     """
     Produce one or more build trees by analysing source code dependencies.
 

source/fab/steps/grab/git.py

@@ -3,24 +3,29 @@
 #  For further details please refer to the file COPYRIGHT
 #  which you should have received as part of this distribution
 # ##############################################################################
-
-'''This module contains the git related steps.
-'''
-
+"""
+Contains the git related steps.
+"""
+from pathlib import Path
+from typing import Optional
 import warnings
 
+from fab.build_config import BuildConfig
 from fab.steps import step
-from fab.tools import Category
+from fab.tools import Category, Git
 
 
 # todo: allow cli args, e.g to set the depth
 @step
-def git_checkout(config, src: str, dst_label: str = '', revision=None):
+def git_checkout(config: BuildConfig,
+                 src: Path,
+                 dst_label: str = '',
+                 revision: Optional[str] = None):
     """
     Checkout or update a Git repo.
-
     """
     git = config.tool_box[Category.GIT]
+    assert isinstance(git, Git)  # ToDo: Problem with typing.
     dst = config.source_root / dst_label
 
     # create folder?
@@ -37,12 +42,13 @@ def git_checkout(config, src: str, dst_label: str = '', revision=None):
 
 
 @step
-def git_merge(config, src: str, dst_label: str = '', revision=None):
+def git_merge(config: BuildConfig,
+              src: Path, dst_label: str = '', revision: Optional[str] = None):
     """
-    Merge a git repo into a local working copy.
-
+    Merges a git repository into a local working copy.
     """
     git = config.tool_box[Category.GIT]
+    assert isinstance(git, Git)  # ToDo: Problem with typing.
     dst = config.source_root / dst_label
     git.fetch(src=src, dst=dst, revision=revision)
     git.merge(dst=dst, revision=revision)

source/fab/steps/grab/svn.py

@@ -3,53 +3,52 @@
 #  For further details please refer to the file COPYRIGHT
 #  which you should have received as part of this distribution
 # ##############################################################################
-
-'''This file contains the steps related to SVN. It is also used by the various
-fcm steps, which call the functions here with just a different category (FCM)
+"""
+Contains the steps related to Subversion. It is also used by the various
+FCM steps, which call the functions here with just a different category (FCM)
 from the tool box.
-'''
+"""
 
 from pathlib import Path
 from typing import Optional, Union, Tuple
 import xml.etree.ElementTree as ET
 
+from fab.build_config import BuildConfig
 from fab.steps import step
-from fab.tools import Category, Versioning
+from fab.tools import Category, Subversion, Tool, Versioning
 
 
-def _get_revision(src, revision=None) -> Tuple[str, Union[str, None]]:
+def split_repo_url(url: str,
+                   revision: Optional[str] = None) -> Tuple[str, Optional[str]]:
     """
     Pull out the revision if it's part of the url.
 
     Some operations need it separated from the url,
     e.g. when calling fcm update, which accepts revision but no url.
 
-    :param src:
-        Repo url.
-    :param revision:
-        Optional revision.
-    Returns (src, revision)
-
+    :param url: Repository url.
+    :param revision: Expected revision or None.
+    :return: URL and revision.
     """
     url_revision = None
-    at_split = src.split('@')
+    at_split = url.split('@')
     if len(at_split) == 2:
         url_revision = at_split[1]
         if url_revision and revision and url_revision != revision:
             raise ValueError('Conflicting revisions in url and argument. '
                              'Please provide as argument only.')
-        src = at_split[0]
+        url = at_split[0]
     else:
         assert len(at_split) == 1
 
-    return src, revision or url_revision
+    return url, revision or url_revision
 
 
-def _svn_prep_common(config, src: str,
+def _svn_prep_common(config: BuildConfig, src: str,
                      dst_label: Optional[str],
                      revision: Optional[str]) -> Tuple[str, Path,
                                                        Optional[str]]:
-    src, revision = _get_revision(src, revision)
+    src, revision = split_repo_url(src, revision)
     if not config.source_root.exists():
         config.source_root.mkdir(parents=True, exist_ok=True)
     dst: Path = config.source_root / (dst_label or '')
@@ -58,23 +57,26 @@ def _svn_prep_common(config, src: str,
 
 
 @step
-def svn_export(config, src: str,
+def svn_export(config: BuildConfig, src: str,
                dst_label: Optional[str] = None,
-               revision=None,
+               revision: Optional[str] = None,
                category=Category.SUBVERSION):
     # todo: params in docstrings
     """
     Export an FCM repo folder to the project workspace.
 
     """
     svn = config.tool_box[category]
+    assert isinstance(svn, Subversion)  # ToDo: Better way to handle classes.
     src, dst, revision = _svn_prep_common(config, src, dst_label, revision)
     svn.export(src, dst, revision)
 
 
 @step
-def svn_checkout(config, src: str, dst_label: Optional[str] = None,
-                 revision=None, category=Category.SUBVERSION):
+def svn_checkout(config: BuildConfig,
+                 src: str, dst_label: Optional[str] = None,
+                 revision: Optional[str] = None,
+                 category=Category.SUBVERSION) -> None:
     """
     Checkout or update an FCM repo.
 
@@ -85,6 +87,7 @@ def svn_checkout(config, src: str, dst_label: Optional[str] = None,
 
     """
     svn = config.tool_box[category]
+    assert isinstance(svn, Subversion)  # ToDo: Better way to handle classes.
     src, dst, revision = _svn_prep_common(config, src, dst_label, revision)
 
     # new folder?
@@ -96,22 +99,27 @@ def svn_checkout(config, src: str, dst_label: Optional[str] = None,
         svn.update(dst, revision)
 
 
-def svn_merge(config, src: str, dst_label: Optional[str] = None, revision=None,
+def svn_merge(config: BuildConfig,
+              src: str, dst_label: Optional[str] = None,
+              revision: Optional[str] = None,
               category=Category.SUBVERSION):
     """
     Merge an FCM repo into a local working copy.
 
     """
     svn = config.tool_box[category]
+    assert isinstance(svn, Subversion)  # ToDo: Better way to handle classes.
     src, dst, revision = _svn_prep_common(config, src, dst_label, revision)
 
     svn.merge(src, dst, revision)
     check_conflict(svn, dst)
 
 
-def check_conflict(tool: Versioning, dst: Union[str, Path]):
-    '''Check if there's a conflict
-    '''
+def check_conflict(tool: Tool, dst: Union[str, Path]) -> bool:
+    """
+    Check if there's a conflict
+    """
+    assert isinstance(tool, Versioning)  # ToDo: Better way to handel classes.
     xml_str = tool.run(['status', '--xml'], cwd=dst, capture_output=True)
     root = ET.fromstring(xml_str)
 

source/fab/tools/tool.py

@@ -18,6 +18,7 @@
 import subprocess
 from typing import Dict, List, Optional, Union
 
+from fab import FabException
 from fab.tools.category import Category
 from fab.tools.flags import Flags
 
@@ -150,14 +151,14 @@ def run(self,
         # is available or not. Testing for `False` only means this `run`
         # function can be used to test if a tool is available.
         if self._is_available is False:
-            raise RuntimeError(f"Tool '{self.name}' is not available to run "
+            raise FabException(f"Tool '{self.name}' is not available to run "
                                f"'{command}'.")
         self._logger.debug(f'run_command: {" ".join(command)}')
         try:
-            res = subprocess.run(command, capture_output=capture_output,
+            res = subprocess.run(command, capture_output=True,
                                  env=env, cwd=cwd, check=False)
         except FileNotFoundError as err:
-            raise RuntimeError(f"Command '{command}' could not be "
+            raise FabException(f"Command '{command}' could not be "
                                f"executed.") from err
         if res.returncode != 0:
             msg = (f'Command failed with return code {res.returncode}:\n'
@@ -166,7 +167,7 @@ def run(self,
                 msg += f'\n{res.stdout.decode()}'
             if res.stderr:
                 msg += f'\n{res.stderr.decode()}'
-            raise RuntimeError(msg)
+            raise FabException(msg)
         if capture_output:
             return res.stdout.decode()
         return ""

source/fab/tools/versioning.py

@@ -11,6 +11,7 @@
 from pathlib import Path
 from typing import Dict, List, Optional, Union
 
+from fab import FabException
 from fab.tools.category import Category
 from fab.tools.tool import Tool
 
@@ -79,8 +80,8 @@ def fetch(self, src: Union[str, Path],
             command.append(revision)
         self.run(command, cwd=str(dst), capture_output=False)
 
-    def checkout(self, src: str,
-                 dst: str = '',
+    def checkout(self, src: Path,
+                 dst: Path = Path(),
                  revision: Optional[str] = None):
         """Checkout or update a Git repo.
 
@@ -103,9 +104,9 @@ def merge(self, dst: Union[str, Path],
         """
         try:
             self.run(['merge', 'FETCH_HEAD'], cwd=dst, capture_output=False)
-        except RuntimeError as err:
+        except FabException as err:
             self.run(['merge', '--abort'], cwd=dst, capture_output=False)
-            raise RuntimeError(f"Error merging {revision}. "
+            raise FabException(f"Error merging {revision}. "
                                f"Merge aborted.\n{err}") from err
 
 

tests/system_tests/MinimalFortran/test_MinimalFortran.py

@@ -39,5 +39,6 @@ def test_minimal_fortran(tmp_path):
     # run
     command = [str(config.artefact_store[EXECUTABLES][0])]
     res = subprocess.run(command, capture_output=True)
+    assert res.returncode == 0
     output = res.stdout.decode()
     assert output.strip() == 'Hello world!'

tests/system_tests/git/test_git.py

@@ -22,6 +22,7 @@
 
 import pytest
 
+from fab import FabException
 from fab.build_config import BuildConfig
 from fab.steps.grab.git import git_checkout, git_merge
 from fab.tools import Git, ToolBox
@@ -90,9 +91,8 @@ def test_vanilla(self, repo_url, config):
             git_merge(config, src=repo_url, dst_label='tiny_fortran', revision='experiment_a')
             assert 'This is sentence one, with Experiment A modification.' in open(check_file).read()
 
-        with pytest.raises(RuntimeError):
+        with pytest.raises(FabException):
             git_merge(config, src=repo_url, dst_label='tiny_fortran', revision='experiment_b')
 
         # The conflicted merge must have been aborted, check that we can do another checkout of master
-        with pytest.warns(UserWarning, match="_metric_send_conn not set, cannot send metrics"):
-            git_checkout(config, src=repo_url, dst_label='tiny_fortran', revision='master')
+        git_checkout(config, src=repo_url, dst_label='tiny_fortran', revision='master')

tests/system_tests/svn_fcm/test_svn_fcm_system_test.py

@@ -4,8 +4,7 @@
 #  which you should have received as part of this distribution
 # ##############################################################################
 """
-Test svn and fcm steps, if their underlying cli tools are available.
-
+Tests Subversion and FCM steps, if their underlying cli tools are available.
 """
 import shutil
 from pathlib import Path
@@ -15,9 +14,9 @@
 
 import pytest
 
-import fab
+from fab import FabException
 from fab.build_config import BuildConfig
-from fab.tools import Fcm, Subversion, ToolBox
+from fab.tools import Fcm, Subversion, ToolBox, tool
 from fab.steps.grab.fcm import fcm_checkout, fcm_export, fcm_merge
 from fab.steps.grab.svn import svn_checkout, svn_export, svn_merge
 
@@ -175,7 +174,7 @@ def test_working_copy(self, file2_experiment, config, checkout_func):
             assert False
 
         with mock.patch('fab.tools.tool.subprocess.run',
-                        wraps=fab.tools.tool.subprocess.run) as wrap, \
+                        wraps=tool.subprocess.run) as wrap, \
              pytest.warns(UserWarning, match="_metric_send_conn not set, cannot send metrics"):
 
             checkout_func(config, src=file2_experiment, dst_label='proj', revision='7')
@@ -199,7 +198,7 @@ def test_not_working_copy(self, trunk, config, export_func, checkout_func):
             export_func(config, src=trunk, dst_label='proj')
 
         # if we try to checkout into that folder, it should fail
-        with pytest.raises(RuntimeError):
+        with pytest.raises(FabException):
             checkout_func(config, src=trunk, dst_label='proj')
 
 
@@ -241,7 +240,7 @@ def test_not_working_copy(self, trunk, file2_experiment, config, export_func, me
             export_func(config, src=trunk, dst_label='proj')
 
         # try to merge into an export
-        with pytest.raises(RuntimeError):
+        with pytest.raises(FabException):
             merge_func(config, src=file2_experiment, dst_label='proj', revision=7)
 
     @pytest.mark.parametrize('checkout_func,merge_func', zip(checkout_funcs, merge_funcs))

tests/system_tests/zero_config/test_zero_config.py

@@ -1,3 +1,4 @@
+import os
 from pathlib import Path
 
 import pytest
@@ -31,7 +32,12 @@ def test_c_fortran_interop(self, tmp_path):
 
             assert (config.project_workspace / 'main').exists()
 
-    def test_fortran_explicit_gfortran(self, tmp_path):
+    def test_fortran_explicit_gfortran(self, tmp_path: Path,
+                                       monkeypatch: pytest.MonkeyPatch):
+        # The linker will make use of LDFLAGS so make sure it is clear.
+        #
+        monkeypatch.delenv('LDFLAGS', raising=False)
+
         # test the sample project in the fortran dependencies system test
         kwargs = {'project_label': 'fortran explicit gfortran', 'fab_workspace': tmp_path, 'multiprocessing': False}