diff --git a/.github/workflows/test_functional.yml b/.github/workflows/test_functional.yml index 30b9b0f1cc2..eb0ae5bd3c0 100644 --- a/.github/workflows/test_functional.yml +++ b/.github/workflows/test_functional.yml @@ -39,7 +39,7 @@ jobs: - name: Apt-Get run: | sudo apt-get update - sudo apt-get install -y sqlite3 + sudo apt-get install -y sqlite3 tree - name: Add .github/bin/ to PATH # Sets up mocked mail command & any other custom executables diff --git a/CHANGES.md b/CHANGES.md index 0b5adfeebbc..7d76a08419c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -122,9 +122,13 @@ hierarchy and ability to set site config directory. [#3883](https://github.com/cylc/cylc-flow/pull/3883) - Added a new workflow config option `[scheduling]stop after cycle point`. +[#3961](https://github.com/cylc/cylc-flow/pull/3961) - Added a new command: +`cylc clean`. + ### Fixes -[#3917](https://github.com/cylc/cylc-flow/pull/3917) - Fix a bug that caused one of the hostname resolution tests to fail in certain environments. +[#3917](https://github.com/cylc/cylc-flow/pull/3917) - Fix a bug that caused +one of the hostname resolution tests to fail in certain environments. [#3879](https://github.com/cylc/cylc-flow/pull/3879) - Removed Google Groups e-mail from pip packaging metadata. Users browsing PYPI will have diff --git a/cylc/flow/etc/cylc-bash-completion b/cylc/flow/etc/cylc-bash-completion index 129465a413d..d67bed3f83e 100644 --- a/cylc/flow/etc/cylc-bash-completion +++ b/cylc/flow/etc/cylc-bash-completion @@ -38,7 +38,7 @@ _cylc() { cur="${COMP_WORDS[COMP_CWORD]}" sec="${COMP_WORDS[1]}" opts="$(cylc scan -t name 2>/dev/null)" - suite_cmds="broadcast|bcast|cat-log|log|check-versions|diff|compare|dump|edit|ext-trigger|external-trigger|get-directory|get-suite-config|get-config|get-suite-version|get-cylc-version|graph|graph-diff|hold|insert|kill|list|ls|tui|ping|poll|print|register|release|unhold|reload|remove|report-timings|reset|restart|run|start|scan|search|grep|set-verbosity|show|set-outputs|stop|shutdown|single|suite-state|test-battery|trigger|validate|view|warranty" + suite_cmds="broadcast|bcast|cat-log|check-versions|clean|compare|diff|dump|edit|ext-trigger|external-trigger|get-directory|get-suite-config|get-config|get-suite-version|get-cylc-version|graph|graph-diff|hold|insert|kill|list|log|ls|tui|ping|poll|print|register|release|unhold|reload|remove|report-timings|reset|restart|run|start|scan|search|grep|set-verbosity|show|set-outputs|stop|shutdown|single|suite-state|test-battery|trigger|validate|view|warranty" if [[ ${COMP_CWORD} -eq 1 ]]; then diff --git a/cylc/flow/pathutil.py b/cylc/flow/pathutil.py index 7507c378377..91bba9304f8 100644 --- a/cylc/flow/pathutil.py +++ b/cylc/flow/pathutil.py @@ -202,4 +202,32 @@ def make_symlink(src, dst): try: os.symlink(src, dst, target_is_directory=True) except Exception as exc: - raise WorkflowFilesError(f"Error when symlinking '{exc}'") + raise WorkflowFilesError(f"Error when symlinking\n{exc}") + + +def remove_dir(path): + """Delete a directory including contents, including the target directory + if the specified path is a symlink. + + Args: + path (str): the absolute path of the directory to delete. + """ + if not os.path.isabs(path): + raise ValueError('Path must be absolute') + if os.path.exists(path) and not os.path.isdir(path): + raise NotADirectoryError(path) + if os.path.islink(path): + if os.path.exists(path): + target = os.path.realpath(path) + LOG.info( + f'Removing symlink target directory: ({path} ->) {target}') + rmtree(target) + LOG.info(f'Removing symlink: {path}') + else: + LOG.info(f'Removing broken symlink: {path}') + os.remove(path) + elif not os.path.exists(path): + raise FileNotFoundError(path) + else: + LOG.info(f'Removing directory: {path}') + rmtree(path) diff --git a/cylc/flow/platforms.py b/cylc/flow/platforms.py index 907c9e222ef..34bf597296d 100644 --- a/cylc/flow/platforms.py +++ b/cylc/flow/platforms.py @@ -339,7 +339,7 @@ def platform_from_job_info(platforms, job, remote): raise PlatformLookupError('No platform found matching your task') -def get_host_from_platform(platform, method=None): +def get_host_from_platform(platform, method='random'): """Placeholder for a more sophisticated function which returns a host given a platform dictionary. @@ -349,8 +349,7 @@ def get_host_from_platform(platform, method=None): method (str): Name a function to use when selecting hosts from list provided by platform. - - - None or 'random': Pick the first host from list + - 'random' (default): Pick a random host from list - 'first': Return the first host in the list Returns: @@ -361,7 +360,7 @@ def get_host_from_platform(platform, method=None): - Random Selection with check for host availability """ - if method is None or method == 'random': + if method == 'random': return random.choice(platform['hosts']) elif method == 'first': return platform['hosts'][0] diff --git a/cylc/flow/scripts/clean.py b/cylc/flow/scripts/clean.py new file mode 100644 index 00000000000..6b36cfbb2c3 --- /dev/null +++ b/cylc/flow/scripts/clean.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python3 + +# THIS FILE IS PART OF THE CYLC SUITE ENGINE. +# Copyright (C) NIWA & British Crown (Met Office) & Contributors. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +"""cylc clean [OPTIONS] ARGS + +Remove a stopped workflow from the local scheduler filesystem. + +NOTE: this command is intended for workflows installed with `cylc install`. If +this is run for a workflow that was instead written directly in ~/cylc-run and +not backed up elsewhere, it will be lost. + +It will also remove an symlink directory targets. For now, it will fail to +remove workflow files/directories on a remote host. + +Suite names can be hierarchical, corresponding to the path under ~/cylc-run. + +Examples: + # Remove the workflow at ~/cylc-run/foo + $ cylc clean foo + +""" + +from cylc.flow.option_parsers import CylcOptionParser as COP +from cylc.flow.terminal import cli_function +from cylc.flow.suite_files import clean + + +def get_option_parser(): + parser = COP( + __doc__, + argdoc=[("REG", "Suite name")] + ) + return parser + + +@cli_function(get_option_parser) +def main(parser, opts, reg): + clean(reg) + + +if __name__ == "__main__": + main() diff --git a/cylc/flow/suite_files.py b/cylc/flow/suite_files.py index 5925c90dd60..3cd3333dfd4 100644 --- a/cylc/flow/suite_files.py +++ b/cylc/flow/suite_files.py @@ -28,8 +28,9 @@ import aiofiles from cylc.flow import LOG -from cylc.flow.exceptions import SuiteServiceFileError -from cylc.flow.pathutil import get_suite_run_dir, make_localhost_symlinks +from cylc.flow.exceptions import SuiteServiceFileError, WorkflowFilesError +from cylc.flow.pathutil import ( + get_suite_run_dir, make_localhost_symlinks, remove_dir) from cylc.flow.platforms import get_platform from cylc.flow.hostuserutil import ( get_user, @@ -478,24 +479,18 @@ def register(reg=None, source=None, redirect=False): Raise: SuiteServiceFileError: - No flow.cylc file found in source location. - Illegal name (can look like a relative path, but not absolute). - Another suite already has this name (unless --redirect). - Trying to register a suite nested inside of another. + - No flow.cylc file found in source location. + - Illegal name (can look like a relative path, but not absolute). + - Another suite already has this name (unless --redirect). + - Trying to register a suite nested inside of another. """ if reg is None: reg = os.path.basename(os.getcwd()) - make_localhost_symlinks(reg) - is_valid, message = SuiteNameValidator.validate(reg) - if not is_valid: - raise SuiteServiceFileError(f'invalid suite name - {message}') - - if os.path.isabs(reg): - raise SuiteServiceFileError( - f'suite name cannot be an absolute path: {reg}') - + _validate_reg(reg) check_nested_run_dirs(reg) + make_localhost_symlinks(reg) + if source is not None: if os.path.basename(source) == SuiteFiles.FLOW_FILE: source = os.path.dirname(source) @@ -559,6 +554,92 @@ def register(reg=None, source=None, redirect=False): return reg +def clean(reg): + """Remove stopped workflows on the local scheduler filesystem. + + Deletes the run dir in ~/cylc-run and any symlink dirs. Note: if the + run dir has been manually deleted, it will not be possible to clean the + symlink dirs. + + Args: + reg (str): workflow name. + """ + _validate_reg(reg) + reg = os.path.normpath(reg) + if reg.startswith('.'): + raise WorkflowFilesError( + 'Workflow name cannot be a path that points to the cylc-run ' + 'directory or above') + run_dir = Path(get_suite_run_dir(reg)) + if not run_dir.is_dir(): + LOG.info(f'No workflow directory to clean at {run_dir}') + return + try: + detect_old_contact_file(reg) + except SuiteServiceFileError as exc: + raise SuiteServiceFileError( + f'Cannot remove running workflow.\n\n{exc}') + + # TODO: check task_jobs table in database to see what platforms are used + + possible_symlinks = [(Path(name), Path(run_dir, name)) for name in [ + 'log', 'share/cycle', 'share', 'work', '']] + # Note: 'share/cycle' must come before 'share', and '' must come last + for name, path in possible_symlinks: + if path.is_symlink(): + # Ensure symlink is pointing to expected directory. If not, + # something is wrong and we should abort + target = path.resolve() + if target.exists() and not target.is_dir(): + raise WorkflowFilesError( + f'Invalid Cylc symlink directory {path} -> {target}\n' + f'Target is not a directory') + expected_end = str(Path('cylc-run', reg, name)) + if not str(target).endswith(expected_end): + raise WorkflowFilesError( + f'Invalid Cylc symlink directory {path} -> {target}\n' + f'Expected target to end with "{expected_end}"') + # Remove /cylc-run/ + target_cylc_run_dir = str(target).rsplit(str(reg), 1)[0] + target_reg_dir = Path(target_cylc_run_dir, reg) + if target_reg_dir.is_dir(): + remove_dir(target_reg_dir) + # Remove empty parents + _remove_empty_reg_parents(reg, target_reg_dir) + + remove_dir(run_dir) + _remove_empty_reg_parents(reg, run_dir) + + +def _remove_empty_reg_parents(reg, path): + """If reg is nested e.g. a/b/c, work our way up the tree, removing empty + parents only. + + Args: + reg (str): workflow name, e.g. a/b/c + path (str): path to this directory, e.g. /foo/bar/a/b/c + + Example: + _remove_empty_reg_parents('a/b/c', '/foo/bar/a/b/c') would remove + /foo/bar/a/b (assuming it's empty), then /foo/bar/a (assuming it's + empty). + """ + reg = Path(reg) + reg_depth = len(reg.parts) - 1 + path = Path(path) + if not path.is_absolute(): + raise ValueError('Path must be absolute') + for i in range(reg_depth): + parent = path.parents[i] + if not parent.is_dir(): + continue + try: + parent.rmdir() + LOG.info(f'Removing directory: {parent}') + except OSError: + break + + def remove_keys_on_server(keys): """Removes server-held authentication keys""" # WARNING, DESTRUCTIVE. Removes old keys if they already exist. @@ -650,12 +731,36 @@ def _load_local_item(item, path): return None +def _validate_reg(reg): + """Check suite name is valid. + + Args: + reg (str): Suite name + + Raise: + SuiteServiceFileError: + - reg has form of absolute path or is otherwise not valid + """ + is_valid, message = SuiteNameValidator.validate(reg) + if not is_valid: + raise SuiteServiceFileError(f'invalid suite name "{reg}" - {message}') + if os.path.isabs(reg): + raise SuiteServiceFileError( + f'suite name cannot be an absolute path: {reg}') + + def check_nested_run_dirs(reg): """Disallow nested run dirs e.g. trying to register foo/bar where foo is already a valid suite directory. Args: reg (str): suite name + + Raise: + WorkflowFilesError: + - reg dir is nested inside a run dir + - reg dir contains a nested run dir (if not deeper than max scan + depth) """ exc_msg = ( 'Nested run directories not allowed - cannot register suite name ' @@ -665,7 +770,7 @@ def _check_child_dirs(path, depth_count=1): for result in os.scandir(path): if result.is_dir() and not result.is_symlink(): if is_valid_run_dir(result.path): - raise SuiteServiceFileError(exc_msg % (reg, result.path)) + raise WorkflowFilesError(exc_msg % (reg, result.path)) if depth_count < MAX_SCAN_DEPTH: _check_child_dirs(result.path, depth_count + 1) @@ -673,7 +778,7 @@ def _check_child_dirs(path, depth_count=1): parent_dir = os.path.dirname(reg_path) while parent_dir != '': if is_valid_run_dir(parent_dir): - raise SuiteServiceFileError( + raise WorkflowFilesError( exc_msg % (reg, get_cylc_run_abs_path(parent_dir))) parent_dir = os.path.dirname(parent_dir) @@ -683,7 +788,7 @@ def _check_child_dirs(path, depth_count=1): def is_valid_run_dir(path): - """Return True if path is a valid run directory, else False. + """Return True if path is a valid, existing run directory, else False. Args: path (str): if this is a relative path, it is taken to be relative to @@ -704,5 +809,4 @@ def get_cylc_run_abs_path(path): """ if os.path.isabs(path): return path - cylc_run_dir = os.path.expandvars(get_platform()['run directory']) - return os.path.join(cylc_run_dir, path) + return get_suite_run_dir(path) diff --git a/setup.cfg b/setup.cfg index e3743500c23..ed594936aea 100644 --- a/setup.cfg +++ b/setup.cfg @@ -70,6 +70,7 @@ cylc.command = broadcast = cylc.flow.scripts.broadcast:main cat-log = cylc.flow.scripts.cat_log:main check-versions = cylc.flow.scripts.check_versions:main + clean = cylc.flow.scripts.clean:main client = cylc.flow.scripts.client:main cycle-point = cylc.flow.scripts.cycle_point:main diff = cylc.flow.scripts.diff:main diff --git a/tests/functional/cylc-clean/00-basic.t b/tests/functional/cylc-clean/00-basic.t new file mode 100644 index 00000000000..5c09be7635d --- /dev/null +++ b/tests/functional/cylc-clean/00-basic.t @@ -0,0 +1,141 @@ +#!/bin/bash +# THIS FILE IS PART OF THE CYLC SUITE ENGINE. +# Copyright (C) NIWA & British Crown (Met Office) & Contributors. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# ----------------------------------------------------------------------------- +# Test the cylc clean command + +. "$(dirname "$0")/test_header" +if ! command -v 'tree' >'/dev/null'; then + skip_all '"tree" command not available' +fi +set_test_number 6 + +# Generate random name for symlink dirs to avoid any clashes with other tests +SYM_NAME="$(mktemp -u)" +SYM_NAME="${SYM_NAME##*tmp.}" + +create_test_global_config "" " +[symlink dirs] + [[localhost]] + run = ${TEST_DIR}/${SYM_NAME}-run + log = ${TEST_DIR}/${SYM_NAME}-log + share = ${TEST_DIR}/${SYM_NAME}-share + share/cycle = ${TEST_DIR}/${SYM_NAME}-cycle + work = ${TEST_DIR}/${SYM_NAME}-work +" +init_suite "${TEST_NAME_BASE}" << '__FLOW__' +[scheduling] + [[graph]] + R1 = darmok +__FLOW__ + +run_ok "${TEST_NAME_BASE}-val" cylc validate "$SUITE_NAME" + +# Create a fake sibling workflow dir in the sym-log dir: +mkdir "${TEST_DIR}/${SYM_NAME}-log/cylc-run/cylctb-${CYLC_TEST_TIME_INIT}/leave-me-alone" + +FUNCTIONAL_DIR="${TEST_SOURCE_DIR_BASE%/*}" +# ----------------------------------------------------------------------------- +TEST_NAME="run-dir-tree-pre-clean" +tree --charset=ascii "$SUITE_RUN_DIR" > "${TEST_NAME}.stdout" +# Remove last line of output: +sed -i '$d' "${TEST_NAME}.stdout" + +cmp_ok "${TEST_NAME}.stdout" << __TREE__ +${HOME}/cylc-run/${SUITE_NAME} +|-- log -> ${TEST_DIR}/${SYM_NAME}-log/cylc-run/${SUITE_NAME}/log +|-- share -> ${TEST_DIR}/${SYM_NAME}-share/cylc-run/${SUITE_NAME}/share +\`-- work -> ${TEST_DIR}/${SYM_NAME}-work/cylc-run/${SUITE_NAME}/work + +__TREE__ + +TEST_NAME="test-dir-tree-pre-clean" +tree --charset=ascii "${TEST_DIR}/${SYM_NAME}-"* > "${TEST_NAME}.stdout" +# Remove last line of output: +sed -i '$d' "${TEST_NAME}.stdout" +# Note: backticks need to be escaped in the heredoc +cmp_ok "${TEST_NAME}.stdout" << __TREE__ +${TEST_DIR}/${SYM_NAME}-cycle +\`-- cylc-run + \`-- cylctb-${CYLC_TEST_TIME_INIT} + \`-- ${FUNCTIONAL_DIR} + \`-- cylc-clean + \`-- ${TEST_NAME_BASE} + \`-- share + \`-- cycle +${TEST_DIR}/${SYM_NAME}-log +\`-- cylc-run + \`-- cylctb-${CYLC_TEST_TIME_INIT} + |-- ${FUNCTIONAL_DIR} + | \`-- cylc-clean + | \`-- ${TEST_NAME_BASE} + | \`-- log + \`-- leave-me-alone +${TEST_DIR}/${SYM_NAME}-run +\`-- cylc-run + \`-- cylctb-${CYLC_TEST_TIME_INIT} + \`-- ${FUNCTIONAL_DIR} + \`-- cylc-clean + \`-- ${TEST_NAME_BASE} + |-- log -> ${TEST_DIR}/${SYM_NAME}-log/cylc-run/${SUITE_NAME}/log + |-- share -> ${TEST_DIR}/${SYM_NAME}-share/cylc-run/${SUITE_NAME}/share + \`-- work -> ${TEST_DIR}/${SYM_NAME}-work/cylc-run/${SUITE_NAME}/work +${TEST_DIR}/${SYM_NAME}-share +\`-- cylc-run + \`-- cylctb-${CYLC_TEST_TIME_INIT} + \`-- ${FUNCTIONAL_DIR} + \`-- cylc-clean + \`-- ${TEST_NAME_BASE} + \`-- share + \`-- cycle -> ${TEST_DIR}/${SYM_NAME}-cycle/cylc-run/${SUITE_NAME}/share/cycle +${TEST_DIR}/${SYM_NAME}-work +\`-- cylc-run + \`-- cylctb-${CYLC_TEST_TIME_INIT} + \`-- ${FUNCTIONAL_DIR} + \`-- cylc-clean + \`-- ${TEST_NAME_BASE} + \`-- work + +__TREE__ +# ----------------------------------------------------------------------------- +run_ok "cylc-clean" cylc clean "$SUITE_NAME" +# ----------------------------------------------------------------------------- +TEST_NAME="run-dir-not-exist-post-clean" +exists_fail "$SUITE_RUN_DIR" + +TEST_NAME="test-dir-tree-post-clean" +tree --charset=ascii "${TEST_DIR}/${SYM_NAME}-"* > "${TEST_NAME}.stdout" +# Remove last line of output: +sed -i '$d' "${TEST_NAME}.stdout" + +cmp_ok "${TEST_NAME}.stdout" << __TREE__ +${TEST_DIR}/${SYM_NAME}-cycle +\`-- cylc-run +${TEST_DIR}/${SYM_NAME}-log +\`-- cylc-run + \`-- cylctb-${CYLC_TEST_TIME_INIT} + \`-- leave-me-alone +${TEST_DIR}/${SYM_NAME}-run +\`-- cylc-run +${TEST_DIR}/${SYM_NAME}-share +\`-- cylc-run +${TEST_DIR}/${SYM_NAME}-work +\`-- cylc-run + +__TREE__ +# ----------------------------------------------------------------------------- +purge +exit diff --git a/tests/functional/cylc-clean/test_header b/tests/functional/cylc-clean/test_header new file mode 120000 index 00000000000..90bd5a36f92 --- /dev/null +++ b/tests/functional/cylc-clean/test_header @@ -0,0 +1 @@ +../lib/bash/test_header \ No newline at end of file diff --git a/tests/unit/test_pathutil.py b/tests/unit/test_pathutil.py index dff14979220..df555f07c72 100644 --- a/tests/unit/test_pathutil.py +++ b/tests/unit/test_pathutil.py @@ -15,7 +15,6 @@ # along with this program. If not, see . """Tests for "cylc.flow.pathutil".""" -from tempfile import TemporaryDirectory from unittest import TestCase from unittest.mock import patch, MagicMock, call @@ -38,8 +37,10 @@ get_suite_run_config_log_dir, get_suite_run_share_dir, get_suite_run_work_dir, - get_suite_test_log_name, make_localhost_symlinks, - make_suite_run_tree, make_symlink, + get_suite_test_log_name, + make_localhost_symlinks, + make_suite_run_tree, + remove_dir ) @@ -290,6 +291,83 @@ def test_incorrect_environment_variables_raise_error( make_localhost_symlinks('test_workflow') +@pytest.mark.parametrize( + 'filetype, expected_err', + [('dir', None), + ('file', NotADirectoryError), + (None, FileNotFoundError)] +) +def test_remove_dir(filetype, expected_err, tmp_path): + """Test that remove_dir() can delete nested dirs and handle bad paths.""" + test_path = tmp_path.joinpath('foo/bar') + if filetype == 'dir': + # Test removal of sub directories too + sub_dir = test_path.joinpath('baz') + sub_dir.mkdir(parents=True) + sub_dir_file = sub_dir.joinpath('meow') + sub_dir_file.touch() + elif filetype == 'file': + test_path = tmp_path.joinpath('meow') + test_path.touch() + + if expected_err: + with pytest.raises(expected_err): + remove_dir(test_path) + else: + remove_dir(test_path) + assert test_path.exists() is False + assert test_path.is_symlink() is False + + +@pytest.mark.parametrize( + 'target, expected_err', + [('dir', None), + ('file', NotADirectoryError), + (None, None)] +) +def test_remove_dir_symlinks(target, expected_err, tmp_path): + """Test that remove_dir() can delete symlinks, including the target.""" + target_path = tmp_path.joinpath('x/y') + target_path.mkdir(parents=True) + + tmp_path.joinpath('a').mkdir() + symlink_path = tmp_path.joinpath('a/b') + + if target == 'dir': + # Add a file into the the target dir to check it removes that ok + target_path.joinpath('meow').touch() + symlink_path.symlink_to(target_path) + elif target == 'file': + target_path = target_path.joinpath('meow') + target_path.touch() + symlink_path.symlink_to(target_path) + elif target is None: + symlink_path.symlink_to(target_path) + # Break symlink + target_path.rmdir() + + if expected_err: + with pytest.raises(expected_err): + remove_dir(symlink_path) + else: + remove_dir(symlink_path) + for path in [symlink_path, target_path]: + assert path.exists() is False + assert path.is_symlink() is False + + +def test_remove_dir_relative(tmp_path): + """Test that you cannot use remove_dir() on a relative path. + + When removing a path, we want to be absolute-ly sure where it is! + """ + # cd to temp dir in case we accidentally succeed in deleting the path + os.chdir(tmp_path) + with pytest.raises(ValueError) as cm: + remove_dir('foo/bar') + assert 'Path must be absolute' in str(cm.value) + + if __name__ == '__main__': from unittest import main main() diff --git a/tests/unit/test_suite_files.py b/tests/unit/test_suite_files.py index 19cc87e538b..a33dffbf445 100644 --- a/tests/unit/test_suite_files.py +++ b/tests/unit/test_suite_files.py @@ -15,13 +15,13 @@ # along with this program. If not, see . from cylc.flow.suite_files import check_nested_run_dirs -from cylc.flow.pathutil import make_localhost_symlinks import pytest from unittest import mock import os.path +from pathlib import Path from cylc.flow import suite_files -from cylc.flow.exceptions import SuiteServiceFileError +from cylc.flow.exceptions import SuiteServiceFileError, WorkflowFilesError def get_register_test_cases(): @@ -247,7 +247,7 @@ def mkdirs_standin(_, exist_ok=False): ('/a/b/c', '/a/b/c')] ) def test_get_cylc_run_abs_path(path, expected, monkeypatch): - monkeypatch.setattr('cylc.flow.suite_files.get_platform', + monkeypatch.setattr('cylc.flow.pathutil.get_platform', lambda: {'run directory': '/mock_cylc_dir'}) assert suite_files.get_cylc_run_abs_path(path) == expected @@ -268,7 +268,7 @@ def test_is_valid_run_dir(path, expected, is_abs_path, monkeypatch): serv_dir = os.path.join(prefix, 'service', 'dir', 'exists', '.service') monkeypatch.setattr('os.path.isfile', lambda x: x == flow_file) monkeypatch.setattr('os.path.isdir', lambda x: x == serv_dir) - monkeypatch.setattr('cylc.flow.suite_files.get_platform', + monkeypatch.setattr('cylc.flow.pathutil.get_platform', lambda: {'run directory': 'mock_cylc_dir'}) path = os.path.normpath(path) if is_abs_path: @@ -293,7 +293,7 @@ def test_nested_run_dirs_raise_error(direction, monkeypatch): suite_files.check_nested_run_dirs('bright/falls') # Nested in a run dir - bad: for path in ('bright/falls/light', 'bright/falls/light/and/power'): - with pytest.raises(SuiteServiceFileError) as exc: + with pytest.raises(WorkflowFilesError) as exc: suite_files.check_nested_run_dirs(path) assert 'Nested run directories not allowed' in str(exc.value) @@ -319,10 +319,179 @@ def mock_scandir(path): for path in ('a/a/a', 'a/b'): suite_files.check_nested_run_dirs(path) # Run dir nested below - bad: - for path in ('a', 'a/a', 'a/c'): - with pytest.raises(SuiteServiceFileError) as exc: + with pytest.raises(WorkflowFilesError) as exc: check_nested_run_dirs(path) assert 'Nested run directories not allowed' in str(exc.value) # Run dir nested below max scan depth - not ideal but passes: suite_files.check_nested_run_dirs('a/d') + + +@pytest.mark.parametrize( + 'reg, expected_err', + [('foo/bar/', None), + ('/foo/bar', SuiteServiceFileError)] +) +def test_validate_reg(reg, expected_err): + if expected_err: + with pytest.raises(expected_err) as exc: + suite_files._validate_reg(reg) + assert 'cannot be an absolute path' in str(exc.value) + else: + suite_files._validate_reg(reg) + + +@pytest.mark.parametrize( + 'reg, props', + [ + ('foo/bar/', {}), + ('foo', {'no dir': True}), + ('foo/..', { + 'no dir': True, + 'err': WorkflowFilesError, + 'err msg': ('cannot be a path that points to the cylc-run ' + 'directory or above') + }), + ('foo/../..', { + 'no dir': True, + 'err': WorkflowFilesError, + 'err msg': ('cannot be a path that points to the cylc-run ' + 'directory or above') + }), + ('foo', { + 'not stopped': True, + 'err': SuiteServiceFileError, + 'err msg': 'Cannot remove running workflow' + }), + ('foo/bar', { + 'symlink dirs': { + 'log': 'sym-log', + 'share': 'sym-share', + 'share/cycle': 'sym-cycle', + 'work': 'sym-work' + } + }), + ('foo', { + 'symlink dirs': { + 'run': 'sym-run', + 'log': 'sym-log', + 'share': 'sym-share', + 'share/cycle': 'sym-cycle', + 'work': 'sym-work' + } + }), + ('foo', { + 'bad symlink': { + 'type': 'file', + 'path': 'sym-log/cylc-run/foo/meow.txt' + }, + 'err': WorkflowFilesError, + 'err msg': 'Target is not a directory' + }), + ('foo', { + 'bad symlink': { + 'type': 'dir', + 'path': 'sym-log/bad/path' + }, + 'err': WorkflowFilesError, + 'err msg': 'Expected target to end with "cylc-run/foo/log"' + }) + ] +) +def test_clean(reg, props, monkeypatch, tmp_path): + """Test the clean() function. + + Params: + reg (str): Workflow name. + props (dict): Possible values are (all optional): + 'err' (Exception): Expected error. + 'err msg' (str): Message that is expected to be in the exception. + 'no dir' (bool): If True, do not create run dir for this test case. + 'not stopped' (bool): If True, simulate that the workflow is + still running. + 'symlink dirs' (dict): As you would find in the global config + under [symlink dirs][platform]. + 'bad symlink' (dict): Simulate an invalid log symlink dir: + 'type' (str): 'file' or 'dir'. + 'path' (str): Path of the symlink target relative to tmp_path. + """ + # --- Setup --- + tmp_path.joinpath('cylc-run').mkdir() + run_dir = tmp_path.joinpath('cylc-run', reg) + run_dir_top_parent = tmp_path.joinpath('cylc-run', Path(reg).parts[0]) + symlink_dirs = props.get('symlink dirs') + bad_symlink = props.get('bad symlink') + if not props.get('no dir') and ( + not symlink_dirs or 'run' not in symlink_dirs): + run_dir.mkdir(parents=True) + + dirs_to_check = [run_dir_top_parent] + if symlink_dirs: + if 'run' in symlink_dirs: + dst = tmp_path.joinpath(symlink_dirs['run'], 'cylc-run', reg) + dst.mkdir(parents=True) + run_dir.symlink_to(dst) + dirs_to_check.append(dst) + symlink_dirs.pop('run') + for s, d in symlink_dirs.items(): + dst = tmp_path.joinpath(d, 'cylc-run', reg, s) + dst.mkdir(parents=True) + src = run_dir.joinpath(s) + src.symlink_to(dst) + dirs_to_check.append(dst.parent) + if bad_symlink: + dst = tmp_path.joinpath(bad_symlink['path']) + if bad_symlink['type'] == 'file': + dst.parent.mkdir(parents=True) + dst.touch() + else: + dst.mkdir(parents=True) + src = run_dir.joinpath('log') + src.symlink_to(dst) + + def mocked_detect_old_contact_file(reg): + if props.get('not stopped'): + raise SuiteServiceFileError('Mocked error') + + monkeypatch.setattr('cylc.flow.suite_files.detect_old_contact_file', + mocked_detect_old_contact_file) + monkeypatch.setattr('cylc.flow.suite_files.get_suite_run_dir', + lambda x: tmp_path.joinpath('cylc-run', x)) + + # --- The actual test --- + expected_err = props.get('err') + if expected_err: + with pytest.raises(expected_err) as exc: + suite_files.clean(reg) + expected_msg = props.get('err msg') + if expected_msg: + assert expected_msg in str(exc.value) + else: + suite_files.clean(reg) + for d in dirs_to_check: + assert d.exists() is False + assert d.is_symlink() is False + + +def test_remove_empty_reg_parents(tmp_path): + """Test that _remove_empty_parents() doesn't remove parents containing a + sibling.""" + reg = 'foo/bar/baz/qux' + path = tmp_path.joinpath(reg) + tmp_path.joinpath('foo/bar/baz').mkdir(parents=True) + sibling_reg = 'foo/darmok' + sibling_path = tmp_path.joinpath(sibling_reg) + sibling_path.mkdir() + suite_files._remove_empty_reg_parents(reg, path) + assert tmp_path.joinpath('foo/bar').exists() is False + assert tmp_path.joinpath('foo').exists() is True + # Also path must be absolute + with pytest.raises(ValueError) as exc: + suite_files._remove_empty_reg_parents('foo/darmok', 'meow/foo/darmok') + assert 'Path must be absolute' in str(exc.value) + # Check it skips non-existent dirs, and stops at the right place too + tmp_path.joinpath('foo/bar').mkdir() + sibling_path.rmdir() + suite_files._remove_empty_reg_parents(reg, path) + assert tmp_path.joinpath('foo').exists() is False + assert tmp_path.exists() is True