From 7f7a716ce7add3b9a40dc6a1a3386af01485ed40 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Tue, 10 Jan 2023 15:24:10 +0000 Subject: [PATCH] response to review --- cylc/flow/scripts/validate_reinstall.py | 6 +-- cylc/flow/workflow_files.py | 7 +-- .../cylc-combination-scripts/01-vr-reload.t | 9 +++- .../cylc-combination-scripts/02-vr-restart.t | 8 ++- .../cylc-combination-scripts/03-vr-resume.t | 54 ------------------- .../05-vr-fail-is-running.t | 2 + .../vr_workflow/flow.cylc | 2 +- tests/integration/conftest.py | 6 --- .../scripts/test_validate_integration.py | 7 ++- tests/integration/test_get_old_tvars.py | 9 ++-- tests/integration/utils/flow_tools.py | 1 - 11 files changed, 25 insertions(+), 86 deletions(-) delete mode 100644 tests/functional/cylc-combination-scripts/03-vr-resume.t diff --git a/cylc/flow/scripts/validate_reinstall.py b/cylc/flow/scripts/validate_reinstall.py index 4b6c6fc05e3..202259985fb 100644 --- a/cylc/flow/scripts/validate_reinstall.py +++ b/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) 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: diff --git a/cylc/flow/workflow_files.py b/cylc/flow/workflow_files.py index 553f678a134..29bbefdab02 100644 --- a/cylc/flow/workflow_files.py +++ b/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, diff --git a/tests/functional/cylc-combination-scripts/01-vr-reload.t b/tests/functional/cylc-combination-scripts/01-vr-reload.t index a0750b3b72f..2091b01d111 100644 --- a/tests/functional/cylc-combination-scripts/01-vr-reload.t +++ b/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 diff --git a/tests/functional/cylc-combination-scripts/02-vr-restart.t b/tests/functional/cylc-combination-scripts/02-vr-restart.t index cfe1761dc0b..2a746a4c769 100644 --- a/tests/functional/cylc-combination-scripts/02-vr-restart.t +++ b/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: diff --git a/tests/functional/cylc-combination-scripts/03-vr-resume.t b/tests/functional/cylc-combination-scripts/03-vr-resume.t deleted file mode 100644 index 43653b7ae7b..00000000000 --- a/tests/functional/cylc-combination-scripts/03-vr-resume.t +++ /dev/null @@ -1,54 +0,0 @@ -#!/usr/bin/env bash -# THIS FILE IS PART OF THE CYLC WORKFLOW 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 `cylc vr` (Validate Reinstall restart) -# In this case the target workflow is paused so cylc reload & cylc play are run. - -. "$(dirname "$0")/test_header" -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 \ - --workflow-name "${WORKFLOW_NAME}" \ - --no-run-name --pause - -while [[ -z $(cylc scan --name "${WORKFLOW_NAME}" --states=paused) ]]; do - sleep 1 -done - - -# It validates, reloads and resumes: - -# Change source workflow and run vr: -sed -i 's@P1Y@P5Y@' flow.cylc -run_ok "${TEST_NAME_BASE}-runs" cylc vr "${WORKFLOW_NAME}" - -# Grep for reporting of revalidation, reinstallation, reloading and playing: -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-installed" "$ cylc reinstall" "VIPOUT.txt" -named_grep_ok "${TEST_NAME_BASE}-it-reloaded" "$ cylc reload" "VIPOUT.txt" - - -# Clean Up: -run_ok "teardown (stop workflow)" cylc stop "${WORKFLOW_NAME}" --now --now -purge -exit 0 diff --git a/tests/functional/cylc-combination-scripts/05-vr-fail-is-running.t b/tests/functional/cylc-combination-scripts/05-vr-fail-is-running.t index 81455d7b365..1bcd939d015 100644 --- a/tests/functional/cylc-combination-scripts/05-vr-fail-is-running.t +++ b/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" diff --git a/tests/functional/cylc-combination-scripts/vr_workflow/flow.cylc b/tests/functional/cylc-combination-scripts/vr_workflow/flow.cylc index f49edbdcecb..6014d4dfd36 100644 --- a/tests/functional/cylc-combination-scripts/vr_workflow/flow.cylc +++ b/tests/functional/cylc-combination-scripts/vr_workflow/flow.cylc @@ -5,4 +5,4 @@ [runtime] [[foo]] - script = sleep 500 + script = cylc pause ${WORKFLOW_ID} diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index c2ace181ede..65fdab9713a 100644 --- a/tests/integration/conftest.py +++ b/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. diff --git a/tests/integration/scripts/test_validate_integration.py b/tests/integration/scripts/test_validate_integration.py index 57d1a3a91fa..e271726dafb 100644 --- a/tests/integration/scripts/test_validate_integration.py +++ b/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 diff --git a/tests/integration/test_get_old_tvars.py b/tests/integration/test_get_old_tvars.py index bd9b6b0dc4a..cd1ee2f118d 100644 --- a/tests/integration/test_get_old_tvars.py +++ b/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: diff --git a/tests/integration/utils/flow_tools.py b/tests/integration/utils/flow_tools.py index 6911661c4b1..2a172a09140 100644 --- a/tests/integration/utils/flow_tools.py +++ b/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: