Skip to content

Commit 6a2b40f

Browse files
hikerlukehoffmannjasonjunweilyu
authored
Add shell tool clean (#368)
* Support new and old style of PSyclone command line (no more nemo api etc) * Fix mypy errors. * Added missing tests for calling psyclone, and converting old style to new stle arguments and vice versa. * Updated comment. * Removed mixing, use a simple regex instead. * Added support for ifx/icx compiler as intel-llvm class. * Added support for nvidia compiler. * Add preliminary support for Cray compiler. * Added Cray compiler wrapper ftn and cc. * Follow a more consistent naming scheme for crays, even though the native compiler names are longer (crayftn-cray, craycc-cray). * Changed names again. * Renamed cray compiler wrapper to be CrayCcWrapper and CrayFtnWrapper, to avoid confusion with Craycc. * Fixed incorrect name in comments. * Additional compilers (#349) * 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. * Make compiler version a tuple of integers * Update some tests to use tuple versions * Explicitly test handling of bad version format * Fix formatting * Tidying up * Make compiler raise an error for any invalid version string Assume these compilers don't need to be hashed. Saves dealing with empty tuples. * Check compiler version string for compiler name * Fix formatting * Add compiler.get_version_string() method Includes other cleanup from PR comments * Add mpi and openmp settings to BuildConfig, made compiler MPI aware. * Looks like the circular dependency has been fixed. * Revert "Looks like the circular dependency has been fixed." ... while it works with the tests, a real application still triggered it. This reverts commit 150dc37. * Don't even try to find a C compiler if no C files are to be compiled. * Updated gitignore to ignore (recently renamed) documentation. * Fixed failing test. * Return from compile Fortran early if there are no files to compiles. Fixed coding style. * Add MPI enables wrapper for intel and gnu compiler. * Fixed test. * Automatically add openmp flag to compiler and linker based on BuildConfig. * Removed enforcement of keyword parameters, which is not supported in python 3.7. * Fixed failing test. * Support more than one tool of a given suite by sorting them. * Use different version checkout for each compiler vendor with mixins * Refactoring, remove unittest compiler class * Fix some mypy errors * Use 'Union' type hint to fix build checks * Added option to add flags to a tool. * Introduce proper compiler wrapper, used this to implement properly wrapper MPI compiler. * Fixed typo in types. * Return run_version_command to base Compiler class Provides default version command that can be overridden for other compilers. Also fix some incorrect tests Other tidying * Add a missing type hint * Added (somewhat stupid) 'test' to reach 100% coverage of PSyclone tool. * Simplified MPI support in wrapper. * More compiler wrapper coverage. * Removed duplicated function. * Removed debug print. * Removed permanently changing compiler attributes, which can cause test failures later. * More test for C compiler wrapper. * More work on compiler wrapper tests. * Fixed version and availability handling, added missing tests for 100% coverage. * Fixed typing error. * Try to fix python 3.7. * Tried to fix failing tests. * Remove inheritance from mixins and use protocol * Simplify compiler inheritance Mixins have static methods with unique names, overrides only happen in concrete classes * Updated wrapper and tests to handle error raised in get_version. * Simplified regular expressions (now tests cover detection of version numbers with only a major version). * Test for missing mixin. * Use the parsing mixing from the compiler in a compiler wrapper. * Use setattr instead of assignment to make mypy happy. * Simplify usage of compiler-specific parsing mixins. * Minor code cleanup. * Updated documentation. * Simplify usage of compiler-specific parsing mixins. * Test for missing mixin. * Fixed test. * Added missing openmp_flag property to compiler wrapper. * Don't use isinstance for consistency check, which does not work for CompilerWrappers. * Fixed isinstance test for C compilation which doesn't work with a CompilerWrapper. * Use a linker's compiler to determine MPI support. Removed mpi property from CompilerSuite. * Added more tests for invalid version numbers. * Added more test cases for invalid version number, improved regex to work as expected. * Fixed typo in test. * Fixed flake/mypy errors. * Combine wrapper flags with flags from wrapped compiler. * Made mypy happy. * Fixed test. * Split tests into smaller individual ones, fixed missing asssert in test. * Parameterised compiler version tests to also test wrapper. * Added missing MPI parameter when getting the compiler. * Fixed comments. * Order parameters to be in same order for various compiler classes. * Remove stray character * Added getter for wrapped compiler. * Fixed small error that would prevent nested compiler wrappers from being used. * Added a cast to make mypy happy. * Add simple getter for linker library flags * Add getter for linker flags by library * Fix formatting * Add optional libs argument to link function * Reorder and clean up linker tests * Make sure `Linker.link()` raises for unknown lib * Add missing type * Fix typing error * Add 'libs' argument to link_exe function * Try to add documentation for the linker libs feature * Use correct list type in link_exe hint * Add silent replace option to linker.add_lib_flags * Fixed spelling mistake in option. * Clarified documentation. * Removed unnecessary functions in CompilerWrapper. * Fixed failing test triggered by executing them in specific order (tools then steps) * Fixed line lengths. * Add tests for linker LDFLAG * Add pre- and post- lib flags to link function * Fix syntax in built-in lib flags * Remove netcdf as a built-in linker library Bash-style substitution is not currently handled * Configure pre- and post-lib flags on the Linker object Previously they were passed into the Linker.link() function * Use more realistic linker lib flags * Formatting fix * Removed mixing, use a simple regex instead. * Added support for ifx/icx compiler as intel-llvm class. * Added support for nvidia compiler. * Add preliminary support for Cray compiler. * Added Cray compiler wrapper ftn and cc. * Made mpi and openmp default to False in the BuildConfig constructor. * Removed white space. * Follow a more consistent naming scheme for crays, even though the native compiler names are longer (crayftn-cray, craycc-cray). * Changed names again. * Support compilers that do not support OpenMP. * Added documentation for openmp parameter. * Renamed cray compiler wrapper to be CrayCcWrapper and CrayFtnWrapper, to avoid confusion with Craycc. * Fixed incorrect name in comments. --------- Co-authored-by: jasonjunweilyu <[email protected]> Co-authored-by: Luke Hoffmann <[email protected]> Co-authored-by: Luke Hoffmann <[email protected]> * Support new and old style of PSyclone command line (no more nemo api etc) * Fix mypy errors. * Added missing tests for calling psyclone, and converting old style to new stle arguments and vice versa. * Added shell tool. * Try to make mypy happy. * Removed debug code. * ToolRepository now only returns default that are available. Updated tests to make tools as available. * Fixed typos and coding style. * Support new and old style of PSyclone command line (no more nemo api etc) * Fix mypy errors. * Added missing tests for calling psyclone, and converting old style to new stle arguments and vice versa. * Updated comment. * Fixed failing tests. * Fixed double _. * Updated documentation and small issues raised in review. * Fixed failing test. --------- Co-authored-by: Luke Hoffmann <[email protected]> Co-authored-by: jasonjunweilyu <[email protected]> Co-authored-by: Luke Hoffmann <[email protected]>
1 parent bff37de commit 6a2b40f

File tree

12 files changed

+230
-48
lines changed

12 files changed

+230
-48
lines changed

Documentation/source/site-specific-config.rst

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,12 @@ contains all the tools that will be used during the build process, but
2020
it can only store one tool per category. If a certain tool should not
2121
be defined in the toolbox, the default from the `ToolRepository` will
2222
be used. This is useful for many standard tools like `git`, `rsync`
23-
etc that de-facto will never be changed.
23+
etc that de-facto will never be changed. Fab will check if a tool
24+
is actually available on the system before adding it to a ToolBox.
25+
This is typically done by trying to run the tool with some testing
26+
parameters, for example requesting its version number. If this fails,
27+
the tool is considered not to be available and will not be used (unless
28+
the user explicitly puts a tool into the ToolBox).
2429

2530
.. note:: If you need to use for example different compilers for
2631
different files, you would implement this as a `meta-compiler`:

source/fab/tools/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
from fab.tools.psyclone import Psyclone
2020
from fab.tools.rsync import Rsync
2121
from fab.tools.preprocessor import Cpp, CppFortran, Fpp, Preprocessor
22+
from fab.tools.shell import Shell
2223
from fab.tools.tool import Tool, CompilerSuiteTool
2324
# Order here is important to avoid a circular import
2425
from fab.tools.tool_repository import ToolRepository
@@ -56,6 +57,7 @@
5657
"Preprocessor",
5758
"Psyclone",
5859
"Rsync",
60+
"Shell",
5961
"Subversion",
6062
"Tool",
6163
"ToolBox",

source/fab/tools/category.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ class Category(Enum):
2525
SUBVERSION = auto()
2626
AR = auto()
2727
RSYNC = auto()
28+
SHELL = auto()
2829
MISC = auto()
2930

3031
def __str__(self):

source/fab/tools/compiler.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ def __init__(self, name: str,
5656
compile_flag: Optional[str] = None,
5757
output_flag: Optional[str] = None,
5858
openmp_flag: Optional[str] = None,
59-
availability_option: Optional[str] = None):
59+
availability_option: Optional[Union[str, List[str]]] = None):
6060
super().__init__(name, exec_name, suite, category=category,
6161
availability_option=availability_option)
6262
self._version: Union[Tuple[int, ...], None] = None

source/fab/tools/psyclone.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ def process(self,
180180
# New version: no API, parameter, but -o for output name:
181181
parameters.extend(["-o", transformed_file])
182182
else:
183-
# 2.5.0 or earlier: needs api nemo, output name is -oalg
183+
# 2.5.0 or earlier: needs api nemo, output name is -opsy
184184
parameters.extend(["-api", "nemo", "-opsy", transformed_file])
185185
parameters.extend(["-l", "all"])
186186

source/fab/tools/shell.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
##############################################################################
2+
# (c) Crown copyright Met Office. All rights reserved.
3+
# For further details please refer to the file COPYRIGHT
4+
# which you should have received as part of this distribution
5+
##############################################################################
6+
7+
"""This file contains a base class for shells. This can be used to execute
8+
other scripts.
9+
"""
10+
11+
from pathlib import Path
12+
from typing import List, Union
13+
14+
from fab.tools.category import Category
15+
from fab.tools.tool import Tool
16+
17+
18+
class Shell(Tool):
19+
'''A simple wrapper that runs a shell script. There seems to be no
20+
consistent way to simply check if a shell is working - not only support
21+
a version command (e.g. sh and dash don't). Instead, availability
22+
is tested by running a simple 'echo' command.
23+
24+
:name: the path to the script to run.
25+
'''
26+
def __init__(self, name: str):
27+
super().__init__(name=name, exec_name=name,
28+
availability_option=["-c", "echo hello"],
29+
category=Category.SHELL)
30+
31+
def exec(self, command: Union[str, List[Union[Path, str]]]) -> str:
32+
'''Executes the specified command.
33+
34+
:param command: the command and potential parameters to execute.
35+
36+
:returns: stdout of the result.
37+
'''
38+
# Make mypy happy:
39+
params: List[Union[str, Path]]
40+
if isinstance(command, str):
41+
params = ["-c", command]
42+
else:
43+
params = ["-c"]
44+
params.extend(command)
45+
return super().run(additional_parameters=params,
46+
capture_output=True)

source/fab/tools/tool.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
import logging
1717
from pathlib import Path
1818
import subprocess
19-
from typing import Dict, List, Optional, Union
19+
from typing import Dict, List, Optional, Sequence, Union
2020

2121
from fab.tools.category import Category
2222
from fab.tools.flags import Flags
@@ -36,7 +36,7 @@ class Tool:
3636

3737
def __init__(self, name: str, exec_name: Union[str, Path],
3838
category: Category = Category.MISC,
39-
availability_option: Optional[str] = None):
39+
availability_option: Optional[Union[str, List[str]]] = None):
4040
self._logger = logging.getLogger(__name__)
4141
self._name = name
4242
self._exec_name = str(exec_name)
@@ -107,7 +107,7 @@ def name(self) -> str:
107107
return self._name
108108

109109
@property
110-
def availability_option(self) -> str:
110+
def availability_option(self) -> Union[str, List[str]]:
111111
''':returns: the option to use to check if the tool is available.'''
112112
return self._availability_option
113113

@@ -139,7 +139,7 @@ def __str__(self):
139139

140140
def run(self,
141141
additional_parameters: Optional[
142-
Union[str, List[Union[Path, str]]]] = None,
142+
Union[str, Sequence[Union[Path, str]]]] = None,
143143
env: Optional[Dict[str, str]] = None,
144144
cwd: Optional[Union[Path, str]] = None,
145145
capture_output=True) -> str:
@@ -210,7 +210,7 @@ class CompilerSuiteTool(Tool):
210210
'''
211211
def __init__(self, name: str, exec_name: Union[str, Path], suite: str,
212212
category: Category,
213-
availability_option: Optional[str] = None):
213+
availability_option: Optional[Union[str, List[str]]] = None):
214214
super().__init__(name, exec_name, category,
215215
availability_option=availability_option)
216216
self._suite = suite

source/fab/tools/tool_repository.py

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
from fab.tools.versioning import Fcm, Git, Subversion
2424
from fab.tools import (Ar, Cpp, CppFortran, Craycc, Crayftn,
2525
Gcc, Gfortran, Icc, Icx, Ifort, Ifx,
26-
Nvc, Nvfortran, Psyclone, Rsync)
26+
Nvc, Nvfortran, Psyclone, Rsync, Shell)
2727

2828

2929
class ToolRepository(dict):
@@ -63,7 +63,7 @@ def __init__(self):
6363
# Add the FAB default tools:
6464
# TODO: sort the defaults so that they actually work (since not all
6565
# tools FAB knows about are available). For now, disable Fpp (by not
66-
# adding it). IF someone actually uses it it can added.
66+
# adding it). If someone actually uses it it can added.
6767
for cls in [Craycc, Crayftn,
6868
Gcc, Gfortran,
6969
Icc, Icx, Ifort, Ifx,
@@ -72,6 +72,12 @@ def __init__(self):
7272
Ar, Fcm, Git, Psyclone, Rsync, Subversion]:
7373
self.add_tool(cls())
7474

75+
# Add the common shells. While Fab itself does not need this,
76+
# it is a very convenient tool for user configuration (e.g. to
77+
# query nc-config etc)
78+
for shell_name in ["sh", "bash", "ksh", "dash"]:
79+
self.add_tool(Shell(shell_name))
80+
7581
# Now create the potential mpif90 and Cray ftn wrapper
7682
all_fc = self[Category.FORTRAN_COMPILER][:]
7783
for fc in all_fc:
@@ -95,9 +101,10 @@ def __init__(self):
95101

96102
def add_tool(self, tool: Tool):
97103
'''Creates an instance of the specified class and adds it
98-
to the tool repository.
104+
to the tool repository. If the tool is a compiler, it automatically
105+
adds the compiler as a linker as well (named "linker-{tool.name}").
99106
100-
:param cls: the tool to instantiate.
107+
:param tool: the tool to add.
101108
'''
102109

103110
# We do not test if a tool is actually available. The ToolRepository
@@ -155,17 +162,20 @@ def set_default_compiler_suite(self, suite: str):
155162
def get_default(self, category: Category,
156163
mpi: Optional[bool] = None,
157164
openmp: Optional[bool] = None):
158-
'''Returns the default tool for a given category. For most tools
159-
that will be the first entry in the list of tools. The exception
160-
are compilers and linker: in this case it must be specified if
161-
MPI support is required or not. And the default return will be
165+
'''Returns the default tool for a given category that is available.
166+
For most tools that will be the first entry in the list of tools. The
167+
exception are compilers and linker: in this case it must be specified
168+
if MPI support is required or not. And the default return will be
162169
the first tool that either supports MPI or not.
163170
164171
:param category: the category for which to return the default tool.
165172
:param mpi: if a compiler or linker is required that supports MPI.
166-
:param open: if a compiler or linker is required that supports OpenMP.
173+
:param openmp: if a compiler or linker is required that supports
174+
OpenMP.
167175
168176
:raises KeyError: if the category does not exist.
177+
:raises RuntimeError: if no tool in the requested category is
178+
available on the system.
169179
:raises RuntimeError: if no compiler/linker is found with the
170180
requested level of MPI support (yes or no).
171181
'''
@@ -176,7 +186,12 @@ def get_default(self, category: Category,
176186

177187
# If not a compiler or linker, return the first tool
178188
if not category.is_compiler and category != Category.LINKER:
179-
return self[category][0]
189+
for tool in self[category]:
190+
if tool.is_available:
191+
return tool
192+
tool_names = ",".join(i.name for i in self[category])
193+
raise RuntimeError(f"Can't find available '{category}' tool. "
194+
f"Tools are '{tool_names}'.")
180195

181196
if not isinstance(mpi, bool):
182197
raise RuntimeError(f"Invalid or missing mpi specification "
@@ -191,8 +206,8 @@ def get_default(self, category: Category,
191206
# ignore it.
192207
if openmp and not tool.openmp:
193208
continue
194-
# If the tool supports/does not support MPI, return it.
195-
if mpi == tool.mpi:
209+
# If the tool supports/does not support MPI, return the first one
210+
if tool.is_available and mpi == tool.mpi:
196211
return tool
197212

198213
# Don't bother returning an MPI enabled tool if no-MPI is requested -

tests/unit_tests/steps/test_archive_objects.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,6 @@ def test_for_library(self):
8484
def test_incorrect_tool(self, tool_box):
8585
'''Test that an incorrect archive tool is detected
8686
'''
87-
8887
config = BuildConfig('proj', tool_box)
8988
cc = tool_box.get_tool(Category.C_COMPILER, config.mpi, config.openmp)
9089
# And set its category to be AR

tests/unit_tests/steps/test_grab.py

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33
# For further details please refer to the file COPYRIGHT
44
# which you should have received as part of this distribution
55
##############################################################################
6+
7+
'''Test various grab implementation - folders and fcm.
8+
'''
9+
610
from pathlib import Path
711
from types import SimpleNamespace
812
from unittest import mock
@@ -15,24 +19,37 @@
1519

1620

1721
class TestGrabFolder:
22+
'''Test grab folder functionality.'''
1823

1924
def test_trailing_slash(self):
20-
with pytest.warns(UserWarning, match="_metric_send_conn not set, cannot send metrics"):
21-
self._common(grab_src='/grab/source/', expect_grab_src='/grab/source/')
25+
'''Test folder grabbing with a trailing slash.'''
26+
with pytest.warns(UserWarning, match="_metric_send_conn not set, "
27+
"cannot send metrics"):
28+
self._common(grab_src='/grab/source/',
29+
expect_grab_src='/grab/source/')
2230

2331
def test_no_trailing_slash(self):
24-
with pytest.warns(UserWarning, match="_metric_send_conn not set, cannot send metrics"):
25-
self._common(grab_src='/grab/source', expect_grab_src='/grab/source/')
32+
'''Test folder grabbing without a trailing slash.'''
33+
with pytest.warns(UserWarning, match="_metric_send_conn not set, "
34+
"cannot send metrics"):
35+
self._common(grab_src='/grab/source',
36+
expect_grab_src='/grab/source/')
2637

2738
def _common(self, grab_src, expect_grab_src):
2839
source_root = Path('/workspace/source')
2940
dst = 'bar'
3041

3142
mock_config = SimpleNamespace(source_root=source_root,
3243
tool_box=ToolBox())
44+
# Since is_available calls run, in order to test a single run call,
45+
# we patch is_available to be always true.
3346
with mock.patch('pathlib.Path.mkdir'):
3447
with mock.patch('fab.tools.tool.Tool.run') as mock_run:
35-
grab_folder(mock_config, src=grab_src, dst_label=dst)
48+
with mock.patch(
49+
'fab.tools.tool.Tool.is_available',
50+
new_callable=mock.PropertyMock) as is_available:
51+
is_available.return_value = True
52+
grab_folder(mock_config, src=grab_src, dst_label=dst)
3653

3754
expect_dst = mock_config.source_root / dst
3855
mock_run.assert_called_once_with(
@@ -41,24 +58,34 @@ def _common(self, grab_src, expect_grab_src):
4158

4259

4360
class TestGrabFcm:
61+
'''Test FCM functionality.'''
4462

4563
def test_no_revision(self):
64+
'''Test FCM without specifying a revision.'''
4665
source_root = Path('/workspace/source')
4766
source_url = '/www.example.com/bar'
4867
dst_label = 'bar'
4968

5069
mock_config = SimpleNamespace(source_root=source_root,
5170
tool_box=ToolBox())
5271
with mock.patch('pathlib.Path.mkdir'):
53-
with mock.patch('fab.tools.tool.Tool.run') as mock_run, \
54-
pytest.warns(UserWarning, match="_metric_send_conn not set, cannot send metrics"):
55-
fcm_export(config=mock_config, src=source_url, dst_label=dst_label)
72+
with mock.patch('fab.tools.tool.Tool.is_available',
73+
new_callable=mock.PropertyMock) as is_available:
74+
is_available.return_value = True
75+
with mock.patch('fab.tools.tool.Tool.run') as mock_run, \
76+
pytest.warns(UserWarning,
77+
match="_metric_send_conn not "
78+
"set, cannot send metrics"):
79+
fcm_export(config=mock_config, src=source_url,
80+
dst_label=dst_label)
5681

5782
mock_run.assert_called_once_with(['export', '--force', source_url,
5883
str(source_root / dst_label)],
59-
env=None, cwd=None, capture_output=True)
84+
env=None, cwd=None,
85+
capture_output=True)
6086

6187
def test_revision(self):
88+
'''Test that the revision is passed on correctly in fcm export.'''
6289
source_root = Path('/workspace/source')
6390
source_url = '/www.example.com/bar'
6491
dst_label = 'bar'
@@ -67,12 +94,19 @@ def test_revision(self):
6794
mock_config = SimpleNamespace(source_root=source_root,
6895
tool_box=ToolBox())
6996
with mock.patch('pathlib.Path.mkdir'):
70-
with mock.patch('fab.tools.tool.Tool.run') as mock_run, \
71-
pytest.warns(UserWarning, match="_metric_send_conn not set, cannot send metrics"):
72-
fcm_export(mock_config, src=source_url, dst_label=dst_label, revision=revision)
97+
with mock.patch('fab.tools.tool.Tool.is_available',
98+
new_callable=mock.PropertyMock) as is_available:
99+
is_available.return_value = True
100+
with mock.patch('fab.tools.tool.Tool.run') as mock_run, \
101+
pytest.warns(
102+
UserWarning, match="_metric_send_conn not set, "
103+
"cannot send metrics"):
104+
fcm_export(mock_config, src=source_url,
105+
dst_label=dst_label, revision=revision)
73106

74107
mock_run.assert_called_once_with(
75-
['export', '--force', '--revision', '42', f'{source_url}', str(source_root / dst_label)],
108+
['export', '--force', '--revision', '42', f'{source_url}',
109+
str(source_root / dst_label)],
76110
env=None, cwd=None, capture_output=True)
77111

78112
# todo: test missing repo

0 commit comments

Comments
 (0)