Skip to content

Commit

Permalink
Check run name as well as workflow name in Cylc install
Browse files Browse the repository at this point in the history
Update changes.d/6264.fix.md

Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>

Apply suggestions from code review

Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>

Restore workflow_files

added tests for reserved dir names
  • Loading branch information
wxtim committed Aug 1, 2024
1 parent f68e8b9 commit e040d9c
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 2 deletions.
1 change: 1 addition & 0 deletions changes.d/6264.fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix bug where `cylc install` failed to prevent invalid run names.
8 changes: 6 additions & 2 deletions cylc/flow/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,13 +285,17 @@ def install_workflow(
source = Path(expand_path(source)).resolve()
if not workflow_name:
workflow_name = get_source_workflow_name(source)
validate_workflow_name(workflow_name, check_reserved_names=True)
if run_name is not None:
if len(Path(run_name).parts) != 1:
raise WorkflowFilesError(
f'Run name cannot be a path. (You used {run_name})'
)
check_reserved_dir_names(run_name)
validate_workflow_name(
os.path.join(workflow_name, run_name),
check_reserved_names=True
)
else:
validate_workflow_name(workflow_name, check_reserved_names=True)
validate_source_dir(source, workflow_name)
run_path_base = Path(get_workflow_run_dir(workflow_name))
relink, run_num, rundir = get_run_dir_info(
Expand Down
45 changes: 45 additions & 0 deletions tests/unit/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -542,3 +542,48 @@ def test_validate_source_dir(tmp_run_dir: Callable, tmp_src_dir: Callable):
with pytest.raises(WorkflowFilesError) as exc_info:
validate_source_dir(src_dir, 'ajay')
assert "exists in source directory" in str(exc_info.value)


def test_install_workflow_failif_name_name(tmp_src_dir):
"""If a run_name is given validate_workflow_name is called on
the workflow and the run name in combination.
"""
src_dir: Path = tmp_src_dir('ludlow')
# It only has a workflow name:
with pytest.raises(WorkflowFilesError, match='can only contain'):
install_workflow(src_dir, workflow_name='foo?')
# It only has a run name:
with pytest.raises(WorkflowFilesError, match='can only contain'):
install_workflow(src_dir, run_name='foo?')
# It has a legal workflow name, but an invalid run name:
with pytest.raises(WorkflowFilesError, match='can only contain'):
install_workflow(src_dir, workflow_name='foo', run_name='bar?')


def test_install_workflow_failif_reserved_name(tmp_src_dir):
"""Reserved names cause install validation failure.
n.b. manually defined to avoid test dependency on workflow_files.
"""
src_dir = tmp_src_dir('ludlow')
is_reserved = '(that filename is reserved)'
reserved_names = {
'share',
'log',
'runN',
'suite.rc',
'work',
'_cylc-install',
'flow.cylc',
# .service fails because starting a workflow/run can't start with "."
# And that check fails first.
# '.service'
}
install_workflow(src_dir, workflow_name='ok', run_name='also_ok')
for name in reserved_names:
with pytest.raises(WorkflowFilesError, match=is_reserved):
install_workflow(src_dir, workflow_name='ok', run_name=name)
with pytest.raises(WorkflowFilesError, match=is_reserved):
install_workflow(src_dir, workflow_name=name)
with pytest.raises(WorkflowFilesError, match=is_reserved):
install_workflow(src_dir, workflow_name=name, run_name='ok')

0 comments on commit e040d9c

Please sign in to comment.