cylc clean: remove top level dir when no run dirs left (#5033)

CHANGES.md

@@ -35,6 +35,10 @@ Maintenance release.
 
 ### Fixes
 
+[#5033](https://github.com/cylc/cylc-flow/pull/5033) - Running `cylc clean`
+on a top level dir containing run dir(s) will now remove that top level dir
+in addition to the run(s) (if there is nothing else inside it).
+
 [#5007](https://github.com/cylc/cylc-flow/pull/5007) - Fix for `cylc broadcast`
 cycle point validation in the UI.
 

cylc/flow/scripts/clean.py

@@ -47,7 +47,7 @@
   $ cylc clean foo/bar --rm log --rm work
 
   # Remove all job log files from the 2020 cycle points
-  cylc clean foo/bar --rm 'log/job/2020*'
+  $ cylc clean foo/bar --rm 'log/job/2020*'
 
   # Remove all .csv files
   $ cylc clean foo/bar --rm '**/*.csv'

cylc/flow/workflow_files.py

@@ -835,13 +835,8 @@ def clean(reg: str, run_dir: Path, rm_dirs: Optional[Set[str]] = None) -> None:
         if '' not in symlink_dirs:
             # if run dir isn't a symlink dir and hasn't been deleted yet
             remove_dir_and_target(run_dir)
-    # Tidy up if necessary
-    # Remove any empty parents of run dir up to ~/cylc-run/
-    remove_empty_parents(run_dir, reg)
-    for symlink, target in symlink_dirs.items():
-        # Remove empty parents of symlink target up to <symlink_dir>/cylc-run/
-        remove_empty_parents(target, Path(reg, symlink))
 
+    # Tidy up if necessary
     # Remove `runN` symlink if it's now broken
     runN = run_dir.parent / WorkflowFiles.RUN_N
     if (
@@ -850,6 +845,20 @@ def clean(reg: str, run_dir: Path, rm_dirs: Optional[Set[str]] = None) -> None:
         os.readlink(str(runN)) == run_dir.name
     ):
         runN.unlink()
+    # Remove _cylc-install if it's the only thing left
+    cylc_install_dir = run_dir.parent / WorkflowFiles.Install.DIRNAME
+    for entry in run_dir.parent.iterdir():
+        if entry == cylc_install_dir:
+            continue
+        break
+    else:  # no break
+        if cylc_install_dir.is_dir():
+            remove_dir_or_file(cylc_install_dir)
+    # Remove any empty parents of run dir up to ~/cylc-run/
+    remove_empty_parents(run_dir, reg)
+    for symlink, target in symlink_dirs.items():
+        # Remove empty parents of symlink target up to <symlink_dir>/cylc-run/
+        remove_empty_parents(target, Path(reg, symlink))
 
 
 def get_symlink_dirs(reg: str, run_dir: Union[Path, str]) -> Dict[str, Path]:

tests/functional/cylc-clean/03-multi.t

@@ -18,45 +18,115 @@
 # Test cleaning multiple run dirs
 
 . "$(dirname "$0")/test_header"
-set_test_number 14
+if ! command -v 'tree' > /dev/null; then
+    skip_all '"tree" command not available'
+fi
+set_test_number 18
+
+FUNCTIONAL_DIR="${TEST_SOURCE_DIR_BASE%/*}"
+
+# -----------------------------------------------------------------------------
 
 for _ in 1 2; do
     install_workflow "$TEST_NAME_BASE" basic-workflow true
 done
 
-exists_ok "${WORKFLOW_RUN_DIR}/run1"
-exists_ok "${WORKFLOW_RUN_DIR}/run2"
+TEST_NAME="tree-pre-clean-1"
+run_ok "${TEST_NAME}" tree --noreport --charset=ascii -L 4 "${HOME}/cylc-run/${CYLC_TEST_REG_BASE}"
+# Note: backticks need to be escaped in the heredoc
+cmp_ok "${TEST_NAME}.stdout" << __TREE__
+${HOME}/cylc-run/${CYLC_TEST_REG_BASE}
+\`-- ${FUNCTIONAL_DIR}
+    \`-- cylc-clean
+        \`-- ${TEST_NAME_BASE}
+            |-- _cylc-install
+            |-- run1
+            |-- run2
+            \`-- runN -> run2
+__TREE__
 
 # Test trying to clean multiple run dirs without --yes fails:
 run_fail "${TEST_NAME_BASE}-no" cylc clean "$WORKFLOW_NAME"
 exists_ok "${WORKFLOW_RUN_DIR}/run1"
 exists_ok "${WORKFLOW_RUN_DIR}/run2"
 
-
-# Should work with --yes:
+# Should work with --yes (removes top level dir too):
 run_ok "${TEST_NAME_BASE}-yes" cylc clean -y "$WORKFLOW_NAME"
-exists_fail "${WORKFLOW_RUN_DIR}/run1"
-exists_fail "${WORKFLOW_RUN_DIR}/run2"
+exists_fail "${HOME}/cylc-run/${CYLC_TEST_REG_BASE}"
 
-# Should continue cleaning a list of worflows even if one fails.
+# -----------------------------------------------------------------------------
+# Should continue cleaning a list of workflows even if one fails.
 
 for _ in 1 2; do
     install_workflow "$TEST_NAME_BASE" basic-workflow true
 done
 
-exists_ok "${WORKFLOW_RUN_DIR}/run1"
-exists_ok "${WORKFLOW_RUN_DIR}/run2"
+TEST_NAME="tree-pre-clean-2"
+run_ok "${TEST_NAME}" tree --noreport --charset=ascii -L 4 "${HOME}/cylc-run/${CYLC_TEST_REG_BASE}"
+cmp_ok "${TEST_NAME}.stdout" << __TREE__
+${HOME}/cylc-run/${CYLC_TEST_REG_BASE}
+\`-- ${FUNCTIONAL_DIR}
+    \`-- cylc-clean
+        \`-- ${TEST_NAME_BASE}
+            |-- _cylc-install
+            |-- run1
+            |-- run2
+            \`-- runN -> run2
+__TREE__
 
 mkdir "${WORKFLOW_RUN_DIR}/run1/.service"
 touch "${WORKFLOW_RUN_DIR}/run1/.service/db"  # corrupted db!
 
-TEST_NAME="${TEST_NAME_BASE}-yes-no" 
+TEST_NAME="${TEST_NAME_BASE}-yes-no"
 run_fail "${TEST_NAME}" \
     cylc clean -y "$WORKFLOW_NAME/run1" "$WORKFLOW_NAME/run2"
 
 grep_ok "Cannot clean .*/run1" "${TEST_NAME}.stderr" -e
 
-exists_ok "${WORKFLOW_RUN_DIR}/run1"
-exists_fail "${WORKFLOW_RUN_DIR}/run2"
+TEST_NAME="tree-post-clean-2"
+run_ok "${TEST_NAME}" tree --noreport --charset=ascii -L 4 "${HOME}/cylc-run/${CYLC_TEST_REG_BASE}"
+cmp_ok "${TEST_NAME}.stdout" << __TREE__
+${HOME}/cylc-run/${CYLC_TEST_REG_BASE}
+\`-- ${FUNCTIONAL_DIR}
+    \`-- cylc-clean
+        \`-- ${TEST_NAME_BASE}
+            |-- _cylc-install
+            \`-- run1
+__TREE__
+
+purge
+
+# -----------------------------------------------------------------------------
+# Should not clean top level dir if not empty.
+
+install_workflow "$TEST_NAME_BASE" basic-workflow true
+
+touch "${WORKFLOW_RUN_DIR}/jellyfish.txt"
+
+TEST_NAME="tree-pre-clean-3"
+run_ok "${TEST_NAME}" tree --noreport --charset=ascii -L 4 "${HOME}/cylc-run/${CYLC_TEST_REG_BASE}"
+cmp_ok "${TEST_NAME}.stdout" << __TREE__
+${HOME}/cylc-run/${CYLC_TEST_REG_BASE}
+\`-- ${FUNCTIONAL_DIR}
+    \`-- cylc-clean
+        \`-- ${TEST_NAME_BASE}
+            |-- _cylc-install
+            |-- jellyfish.txt
+            |-- run1
+            \`-- runN -> run1
+__TREE__
+
+run_ok "${TEST_NAME}" cylc clean -y "$WORKFLOW_NAME"
+
+TEST_NAME="tree-post-clean-3"
+run_ok "${TEST_NAME}" tree --noreport --charset=ascii -L 4 "${HOME}/cylc-run/${CYLC_TEST_REG_BASE}"
+cmp_ok "${TEST_NAME}.stdout" << __TREE__
+${HOME}/cylc-run/${CYLC_TEST_REG_BASE}
+\`-- ${FUNCTIONAL_DIR}
+    \`-- cylc-clean
+        \`-- ${TEST_NAME_BASE}
+            |-- _cylc-install
+            \`-- jellyfish.txt
+__TREE__
 
 purge

tests/unit/test_workflow_files.py

@@ -1412,6 +1412,28 @@ def test_remote_clean_cmd(
     assert constructed_cmd == ['clean', '--local-only', reg, *expected_args]
 
 
+def test_clean_top_level(tmp_run_dir: Callable):
+    """Test that cleaning last remaining run dir inside a workflow dir removes
+    the top level dir if it's empty (excluding _cylc-install)."""
+    # Setup
+    reg = 'blue/planet/run1'
+    run_dir: Path = tmp_run_dir(reg, installed=True, named=True)
+    cylc_install_dir = run_dir.parent / WorkflowFiles.Install.DIRNAME
+    assert cylc_install_dir.is_dir()
+    runN_symlink = run_dir.parent / WorkflowFiles.RUN_N
+    assert runN_symlink.exists()
+    # Test
+    clean(reg, run_dir)
+    assert not run_dir.parent.parent.exists()
+    # Now check that if the top level dir is not empty, it doesn't get removed
+    run_dir: Path = tmp_run_dir(reg, installed=True, named=True)
+    jellyfish_file = (run_dir.parent / 'jellyfish.txt')
+    jellyfish_file.touch()
+    clean(reg, run_dir)
+    assert cylc_install_dir.is_dir()
+    assert jellyfish_file.exists()
+
+
 def test_get_workflow_source_dir_numbered_run(tmp_path):
     """Test get_workflow_source_dir returns correct source for numbered run"""
     cylc_install_dir = (