Skip to content

Commit

Permalink
Remove unecessary require_flow_file arg from workflow_files.parse_reg()
Browse files Browse the repository at this point in the history
  • Loading branch information
@MetRonnie
MetRonnie committed Jul 1, 2021
1 parent bc72520 commit 5a3d081
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 69 deletions.
29 changes: 8 additions & 21 deletions cylc/flow/workflow_files.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1005,37 +1005,29 @@ def get_platforms_from_db(run_dir):


@overload
def parse_reg(
reg: str, src: Literal[False] = False, require_flow_file: bool = True
) -> str:
def parse_reg(reg: str, src: Literal[False] = False) -> str:
...


@overload
def parse_reg(
reg: str, src: Literal[True], require_flow_file: bool = True
) -> Tuple[str, Path]:
def parse_reg(reg: str, src: Literal[True]) -> Tuple[str, Path]:
...


@overload
def parse_reg(
reg: str, src: bool, require_flow_file: bool
) -> Union[str, Tuple[str, Path]]:
def parse_reg(reg: str, src: bool) -> Union[str, Tuple[str, Path]]:
... # Need this 3rd overload https://github.com/python/mypy/issues/6113


def parse_reg(
reg: str, src: bool = False, require_flow_file: bool = True
) -> Union[str, Tuple[str, Path]]:
def parse_reg(reg: str, src: bool = False) -> Union[str, Tuple[str, Path]]:
"""Centralised parsing of the workflow argument, to be used by most
cylc commands (script modules).
Infers the latest numbered run if a specific one is not given (e.g.
foo -> foo/run3, foo/runN -> foo/run3).
"Offline" commands (e.g. cylc validate) can usually be used on
workflow sources so will need src = True and require_flow_file = True.
workflow sources so will need src = True.
"Online" commands (e.g. cylc stop) are usually only used on workflows in
the cylc-run dir so will need src = False.
Expand All @@ -1049,18 +1041,13 @@ def parse_reg(
src: Whether the workflow arg can be a workflow source (i.e. an
absolute path (which might not be in ~/cylc-run) and/or a
flow.cylc file (or any file really), or '.' for cwd).
require_flow_file: (Ignored if src is False, or if reg is already a
path to a file.) Whether a flow.cylc (or suite.rc) file is required
to be present in the path looked up from the workflow arg.
Returns:
reg: The normalised workflow arg.
path: (Only if src is True) The absolute path to the workflow file
(flow.cylc or suite.rc), or just the workflow dir if
require_flow_file is False.
(flow.cylc or suite.rc).
"""
if not src:
require_flow_file = False
validate_workflow_name(reg)
reg = expand_path(os.path.normpath(reg))
if src and (reg == '.'):
Expand All @@ -1074,7 +1061,7 @@ def parse_reg(
reg_path if reg_path.is_absolute()
else Path(get_workflow_run_dir(reg_path))
)
if require_flow_file and not os.path.lexists(abs_path):
if src and not os.path.lexists(abs_path):
raise WorkflowFilesError(f"{os.strerror(errno.ENOENT)}: {abs_path}")
if abs_path.is_file():
if not src:
Expand All @@ -1090,7 +1077,7 @@ def parse_reg(
reg_path = reg_path.parent
reg_path = reg_path / latest_run.name
abs_path = latest_run
if require_flow_file:
if src:
abs_path = check_flow_file(abs_path)
reg = str(reg_path)
if src:
Expand Down
76 changes: 28 additions & 48 deletions tests/unit/test_workflow_files.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,142 +222,121 @@ def setup__test_parse_reg(mod_tmp_run_dir: Callable) -> Path:
(non_numbered_workflow / 'flow.cylc').touch()
(cylc_run_dir / 'empty').mkdir()
tmp_path = cylc_run_dir.parent
(tmp_path / 'flow.cylc').touch()
(tmp_path / 'random_file.cylc').touch()
return cylc_run_dir


@pytest.mark.parametrize(
('reg', 'src', 'require_flow_file',
'expected_reg', 'expected_path', 'expected_err_msg'),
'reg, src, expected_reg, expected_path, expected_err_msg',
[
pytest.param(
'numbered/workflow', False, False,
'numbered/workflow', False,
'numbered/workflow/run1',
None,
None,
id="numbered workflow, src=False"
),
pytest.param(
'numbered/workflow', True, True,
'numbered/workflow', True,
'numbered/workflow/run1',
'{cylc_run_dir}/numbered/workflow/run1/flow.cylc',
None,
id="numbered workflow, src=True"
),
pytest.param(
'numbered/workflow/runN', False, False,
'numbered/workflow/runN', False,
'numbered/workflow/run1',
None,
None,
id="numbered workflow targeted by runN, src=False"
),
pytest.param(
'non_numbered/workflow', False, False,
'non_numbered/workflow', False,
'non_numbered/workflow',
None,
None,
id="non-numbered, src=False"
),
pytest.param(
'non_numbered/workflow', True, True,
'non_numbered/workflow', True,
'non_numbered/workflow',
'{cylc_run_dir}/non_numbered/workflow/flow.cylc',
None,
id="non-numbered, src=True"
),
pytest.param(
'empty', True, False,
'empty',
'{cylc_run_dir}/empty',
None,
id="empty run dir, src=True, require_flow_file=False"
),
pytest.param(
'empty', True, True,
'empty', True,
None,
None,
"no flow.cylc or suite.rc in ",
id="empty run dir, src=True, require_flow_file=True"
id="empty run dir, src=True"
),
pytest.param(
'non_exist', False, False,
'non_exist', False,
'non_exist',
None,
None,
id="non-existent workflow, src=False"
),
pytest.param(
'non_exist', True, True,
'non_exist', True,
None,
None,
os.strerror(errno.ENOENT),
id="non-existent workflow, src=True"
),
pytest.param(
'non_exist', True, False,
'non_exist',
'{cylc_run_dir}/non_exist',
None,
id="non-existent workflow, src=True, require_flow_file=False"
),
pytest.param(
'non_exist', False, True,
'non_exist',
None,
None,
id="require_flow_file=True ignored when src=False"
),
pytest.param(
'.', True, False,
'{cwd}',
'.', True,
'{cwd}',
'{cwd}/flow.cylc',
None,
id="dot for cwd, src=True, require_flow_file=False"
id="dot for cwd, src=True"
),
pytest.param(
'non_numbered/workflow/flow.cylc', False, False,
'non_numbered/workflow/flow.cylc', False,
None,
None,
"Workflow name must refer to a directory, not a file",
id="reg refers to a file, src=False"
),
pytest.param(
'non_numbered/workflow/flow.cylc', True, True,
'non_numbered/workflow/flow.cylc', True,
'non_numbered/workflow',
'{cylc_run_dir}/non_numbered/workflow/flow.cylc',
None,
id="reg refers to a file, src=True"
),
pytest.param(
'{cylc_run_dir}/non_numbered/workflow', False, False,
'{cylc_run_dir}/non_numbered/workflow', False,
None,
None,
"workflow name cannot be an absolute path",
id="reg is absolute path, src=False"
),
pytest.param(
'{cylc_run_dir}/non_numbered/workflow', True, True,
'{cylc_run_dir}/non_numbered/workflow', True,
'non_numbered/workflow',
'{cylc_run_dir}/non_numbered/workflow/flow.cylc',
None,
id="reg is absolute path, src=True"
),
pytest.param(
'{tmp_path}/random_file.cylc', True, True,
'{tmp_path}/random_file.cylc', True,
'{tmp_path}',
'{tmp_path}/random_file.cylc',
None,
id="reg is absolute path not in ~/cylc-run, src=True"
),
pytest.param(
'foo/../../random_file.cylc', False, False,
'foo/../../random_file.cylc', False,
None,
None,
"cannot be a path that points to the cylc-run directory or above",
id="reg points to path above ~/cylc-run, src=False"
),
pytest.param(
'foo/../../random_file.cylc', True, True,
'foo/../../random_file.cylc', True,
'..',
'{tmp_path}/random_file.cylc',
None,
Expand All @@ -368,22 +347,23 @@ def setup__test_parse_reg(mod_tmp_run_dir: Callable) -> Path:
def test_parse_reg(
reg: str,
src: bool,
require_flow_file: bool,
expected_reg: str,
expected_path: Optional[str],
expected_err_msg: Optional[str],
setup__test_parse_reg: Path
setup__test_parse_reg: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Test parse_reg()."""
tmp_path = setup__test_parse_reg.parent
monkeypatch.chdir(tmp_path)
paths = {
'cylc_run_dir': setup__test_parse_reg,
'tmp_path': setup__test_parse_reg.parent,
'cwd': os.getcwd()
'tmp_path': tmp_path,
'cwd': tmp_path
}
reg = reg.format(**paths)
if expected_err_msg:
with pytest.raises(WorkflowFilesError) as exc_info:
parse_reg(reg, src, require_flow_file)
parse_reg(reg, src)
assert expected_err_msg in str(exc_info.value)
else:
expected: Union[str, Tuple[str, Path]]
Expand All @@ -393,7 +373,7 @@ def test_parse_reg(
expected = (expected_reg, Path(expected_path))
else:
expected = expected_reg
assert parse_reg(reg, src, require_flow_file) == expected
assert parse_reg(reg, src) == expected


@pytest.mark.parametrize(
Expand Down

0 comments on commit 5a3d081

Please sign in to comment.