response to review

cylc/flow/scripts/validate_reinstall.py

@@ -109,12 +109,12 @@ def vro_cli(parser: COP, options: 'Values', workflow_id: str):
     # Use this interface instead of scan, because it can have an ambiguous
     # outcome which we want to capture before we install.
     try:
-        detect_old_contact_file(workflow_id, quiet=True)
+        detect_old_contact_file(workflow_id)#, quiet=True)
     except ServiceFileError:
-        # Workflow is definately stopped:
+        # Workflow is definately running:
         workflow_running = True
     else:
-        # Workflow is definately running:
+        # Workflow is definately stopped:
         workflow_running = False
 
     # Force on the against_source option:

cylc/flow/workflow_files.py

@@ -465,7 +465,7 @@ def _is_process_running(
 
 
 def detect_old_contact_file(
-    reg: str, contact_data=None, quiet=False
+    reg: str, contact_data=None
 ) -> None:
     """Check if the workflow process is still running.
 
@@ -483,9 +483,6 @@ def detect_old_contact_file(
     Args:
         reg: workflow name
         contact_date:
-        quiet: Controls whether to return already running message -
-            this is not required if Cylc VRO is using this function to
-            decide whether to resume or reload.
 
     Raises:
         CylcError:
@@ -521,8 +518,6 @@ def detect_old_contact_file(
     fname = get_contact_file_path(reg)
     if process_is_running:
         # ... the process is running, raise an exception
-        if quiet:
-            raise ServiceFileError()
         raise ServiceFileError(
             CONTACT_FILE_EXISTS_MSG % {
                 "host": old_host,

tests/functional/cylc-combination-scripts/01-vr-reload.t

@@ -20,7 +20,7 @@
 # In this case the target workflow is running so cylc reload is run.
 
 . "$(dirname "$0")/test_header"
-set_test_number 6
+set_test_number 7
 
 
 # Setup (Must be a running workflow, note the unusual absence of --no-detach)
@@ -41,10 +41,15 @@ run_ok "${TEST_NAME_BASE}-runs" cylc vr "${WORKFLOW_NAME}"
 
 # Grep for VR reporting revalidation, reinstallation and reloading
 grep "\$" "${TEST_NAME_BASE}-runs.stdout" > VIPOUT.txt
-named_grep_ok "${TEST_NAME_BASE}-it-revalidated" "$ cylc validate --against-source" "VIPOUT.txt"
+named_grep_ok "${TEST_NAME_BASE}-it-validated" "$ cylc validate --against-source" "VIPOUT.txt"
 named_grep_ok "${TEST_NAME_BASE}-it-installed" "$ cylc reinstall" "VIPOUT.txt"
 named_grep_ok "${TEST_NAME_BASE}-it-reloaded" "$ cylc reload" "VIPOUT.txt"
 
+cylc play "${WORKFLOW_NAME}"
+
+named_grep_ok "${TEST_NAME_BASE}-it-logged-reload" \
+    "Reloading the workflow definition" \
+    "${WORKFLOW_RUN_DIR}/log/scheduler/log"
 
 # Clean Up.
 run_ok "teardown (stop workflow)" cylc stop "${WORKFLOW_NAME}" --now --now

tests/functional/cylc-combination-scripts/02-vr-restart.t

@@ -26,14 +26,12 @@ set_test_number 6
 # Setup
 WORKFLOW_NAME="cylctb-x$(< /dev/urandom tr -dc _A-Z-a-z-0-9 | head -c6)"
 cp "${TEST_SOURCE_DIR}/vr_workflow/flow.cylc" .
-run_ok "setup (vip)" \
-    cylc vip --debug \
+run_ok "setup (install)" \
+    cylc install \
     --workflow-name "${WORKFLOW_NAME}" \
     --no-run-name
-# Get the workflow into a stopped state
-cylc stop --now --now "${WORKFLOW_NAME}"
+
 export WORKFLOW_RUN_DIR="${RUN_DIR}/${WORKFLOW_NAME}"
-poll_workflow_stopped
 
 # It validates and restarts:
 

tests/functional/cylc-combination-scripts/05-vr-fail-is-running.t

@@ -37,6 +37,8 @@ run_ok "setup (vip)" \
     cylc vip --debug \
     --workflow-name "${WORKFLOW_NAME}" \
     --no-run-name
+
+
 # Get the workflow into an unreachable state
 
 CONTACTFILE="${RUN_DIR}/${WORKFLOW_NAME}/.service/contact"

tests/functional/cylc-combination-scripts/vr_workflow/flow.cylc

@@ -5,4 +5,4 @@
 
 [runtime]
     [[foo]]
-        script = sleep 500
+        script = cylc pause ${WORKFLOW_ID}

tests/integration/conftest.py

@@ -149,12 +149,6 @@ def flow(run_dir, test_dir):
     yield partial(_make_flow, run_dir, test_dir)
 
 
-@pytest.fixture
-def flow_src(tmp_path):
-    """A function for creating function-level flows."""
-    yield partial(_make_src_flow, tmp_path)
-
-
 @pytest.fixture(scope='module')
 def mod_scheduler():
     """Return a Scheduler object for a flow.

tests/integration/scripts/test_validate_integration.py

@@ -18,16 +18,15 @@
 
 """Integration tests for Cylc Validate CLI script."""
 
-from cylc.flow.scripts.validate import wrapped_main as validate
 from cylc.flow.parsec.exceptions import IllegalItemError
 import pytest
 from cylc.flow.parsec.exceptions import Jinja2Error
 
 
-async def test_revalidate_checks_source(
+async def test_validate_against_source_checks_source(
     capsys, validate, workflow_source, install, one_conf
 ):
-    """Validation fails if revalidating with broken config.
+    """Validation fails if validating against source with broken config.
     """
     src_dir = workflow_source(one_conf)
     workflow_id = install(src_dir)
@@ -44,7 +43,7 @@ async def test_revalidate_checks_source(
         validate(workflow_id, against_source=True)
 
 
-async def test_revalidate_gets_old_tvars(
+async def test_validate_against_source_gets_old_tvars(
     workflow_source, capsys, validate, scheduler, run, install, run_dir
 ):
     """Validation will retrieve template variables from a previously played

tests/integration/test_get_old_tvars.py

@@ -16,7 +16,8 @@
 
 import pytest
 from pytest import param
-from types import SimpleNamespace
+
+from cylc.flow.option_parsers import Options
 
 from cylc.flow.scripts.validate import (
     wrapped_main as validate,
@@ -70,14 +71,14 @@ def _setup(mod_scheduler, mod_flow):
         param(cylclist, list_gop, 'bar', id='list')
     )
 )
-async def test_revalidate_validate(
+async def test_validate_with_old_tvars(
     _setup, mod_start, capsys, function, parser, expect,
 ):
-    """It (A Cylc CLI Command) can get template vars stored in db.
+    """It (A Cylc CLI Command - see parameters) can
+    get template vars stored in db.
 
     Else the jinja2 in the config would cause these tasks to fail.
     """
-    from cylc.flow.option_parsers import Options
     parser = parser()
     opts = Options(parser)()
     if function == graph:

tests/integration/utils/flow_tools.py

@@ -55,7 +55,6 @@ def _make_flow(
     test_dir: Path,
     conf: Union[dict, str],
     name: Optional[str] = None,
-    is_run: Optional[bool] = True
 ) -> str:
     """Construct a workflow on the filesystem."""
     if name is None: