From 5073f5b3d1348040ec3a02023a1737e04670dbc2 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Wed, 1 May 2024 10:00:01 +0100 Subject: [PATCH 1/2] Delete opts after post install (#312) delete unwanted opts after post install step. * Removed test because it duplicated test_no_rose_suite_conf_in_devdir * added test to confim that post instlall removes Rose settings from opts objects. * lower pin cylc version for this fix (requires cylc-flow change) * unset ROSE_SUITE_OPT_CONF_KEYS --------- Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Co-authored-by: Oliver Sanders --- cylc/rose/entry_points.py | 15 +++- cylc/rose/utilities.py | 7 +- setup.cfg | 2 +- tests/conftest.py | 79 +++++++++++++++++++- tests/functional/test_ROSE_ORIG_HOST.py | 4 +- tests/functional/test_reinstall.py | 16 ++-- tests/functional/test_reinstall_clean.py | 6 +- tests/functional/test_reinstall_overrides.py | 76 +++++++++++++++++++ tests/functional/test_rose_fileinstall.py | 4 +- tests/unit/test_functional_post_install.py | 4 +- 10 files changed, 188 insertions(+), 25 deletions(-) create mode 100644 tests/functional/test_reinstall_overrides.py diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 8da910b0..96ee4951 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -27,6 +27,7 @@ from metomi.rose.config import ConfigLoader, ConfigDumper from cylc.rose.utilities import ( ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING, + ROSE_SUITE_OPT_CONF_KEYS, deprecation_warnings, dump_rose_log, get_rose_vars_from_config_node, @@ -54,12 +55,12 @@ def __str__(self): return msg -def pre_configure(srcdir=None, opts=None, rundir=None): - srcdir, rundir = paths_to_pathlib([srcdir, rundir]) +def pre_configure(srcdir, opts): + srcdir = Path(srcdir) return get_rose_vars(srcdir=srcdir, opts=opts) -def post_install(srcdir=None, opts=None, rundir=None): +def post_install(srcdir, opts, rundir): if not rose_config_exists(srcdir, opts): return False srcdir, rundir = paths_to_pathlib([srcdir, rundir]) @@ -75,6 +76,14 @@ def post_install(srcdir=None, opts=None, rundir=None): if results['fileinstall']: dump_rose_log(rundir=rundir, node=results['fileinstall']) + # Having dumped the config we clear rose options + # as they do not apply after this. + # see https://github.com/cylc/cylc-rose/pull/312 + opts.rose_template_vars = [] + opts.opt_conf_keys = [] + opts.defines = [] + os.unsetenv(ROSE_SUITE_OPT_CONF_KEYS) + return results diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index 586812bc..6b8e382f 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -38,6 +38,7 @@ from optparse import Values +ROSE_SUITE_OPT_CONF_KEYS = "ROSE_SUITE_OPT_CONF_KEYS" SECTIONS = {'jinja2:suite.rc', 'empy:suite.rc', 'template variables'} SET_BY_CYLC = 'set by Cylc' ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING = ( @@ -257,7 +258,7 @@ def rose_config_tree_loader(srcdir=None, opts=None): opt_conf_keys = [] # get optional config key set as environment variable: - opt_conf_keys_env = os.getenv("ROSE_SUITE_OPT_CONF_KEYS") + opt_conf_keys_env = os.getenv(ROSE_SUITE_OPT_CONF_KEYS) if opt_conf_keys_env: opt_conf_keys += shlex.split(opt_conf_keys_env) @@ -591,8 +592,8 @@ def merge_opts(config, opt_conf_keys): all_opt_conf_keys = [] if 'opts' in config: all_opt_conf_keys.append(config['opts'].value) - if "ROSE_SUITE_OPT_CONF_KEYS" in os.environ: - all_opt_conf_keys.append(os.environ["ROSE_SUITE_OPT_CONF_KEYS"]) + if ROSE_SUITE_OPT_CONF_KEYS in os.environ: + all_opt_conf_keys.append(os.environ[ROSE_SUITE_OPT_CONF_KEYS]) if opt_conf_keys and isinstance(opt_conf_keys, str): all_opt_conf_keys.append(opt_conf_keys) if opt_conf_keys and isinstance(opt_conf_keys, list): diff --git a/setup.cfg b/setup.cfg index 6c954c36..31373618 100644 --- a/setup.cfg +++ b/setup.cfg @@ -56,7 +56,7 @@ python_requires = >=3.7 include_package_data = True install_requires = metomi-rose>=2.1.0, <2.3.0 - cylc-flow==8.2.* + cylc-flow>8.2.5, <8.3 metomi-isodatetime ansimarkup jinja2 diff --git a/tests/conftest.py b/tests/conftest.py index 4031397e..e3dc17db 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -15,9 +15,14 @@ # along with this program. If not, see . """Functional tests for top-level function record_cylc_install_options and """ - +from functools import partial +from subprocess import run +from shlex import split +from pathlib import Path import pytest +from time import sleep from types import SimpleNamespace +from uuid import uuid4 from cylc.flow import __version__ as CYLC_VERSION from cylc.flow.option_parsers import Options @@ -38,6 +43,11 @@ ) +def test_workflow_name(): + """Return a UUID to use as a test name""" + return f'cylc-rose-test-{str(uuid4())[:8]}' + + @pytest.fixture(scope='module') def mod_capsys(request): from _pytest.capture import SysCapture @@ -128,8 +138,14 @@ def _inner(srcpath, args=None): srcpath: args: Dictionary of arguments. """ + if not args or not args.get('workflow_name', ''): + id_ = test_workflow_name() + args = {'workflow_name': id_} + + args.update({'no_run_name': True}) options = Options(install_gop(), args)() output = SimpleNamespace() + output.id = args['workflow_name'] try: cylc_install(options, str(srcpath)) @@ -197,3 +213,64 @@ def cylc_validate_cli(capsys, caplog): @pytest.fixture(scope='module') def mod_cylc_validate_cli(mod_capsys, mod_caplog): return _cylc_validate_cli(mod_capsys, mod_caplog) + + +@pytest.fixture +def file_poll(): + """Poll for the existance of a file. + """ + def _inner( + fpath: "Path", timeout: int = 5, inverse: bool = False + ): + timeout_func( + lambda: fpath.exists() != inverse, + f"file {fpath} {'still' if inverse else 'not'} found after " + f"{timeout} seconds", + timeout + ) + return _inner + + +@pytest.fixture +def purge_workflow(run_ok, file_poll): + """Ensure workflow is stopped and cleaned""" + def _inner(id_, timeout=5): + stop = f'cylc stop {id_} --now --now' + clean = f'cylc clean {id_}' + timeout_func( + partial(run_ok, stop), + message=f'Not run after {timeout} seconds: {stop}', + timeout=timeout + ) + file_poll( + Path.home() / 'cylc-run' / id_ / '.service/contact', + inverse=True, + ) + timeout_func( + partial(run_ok, clean), + message=f'Not run after {timeout} seconds: {clean}', + timeout=timeout + ) + return _inner + + +@pytest.fixture +def run_ok(): + """Run a bash script. + Fail if it fails and return its output. + """ + def _inner(script: str): + result = run(split(script), capture_output=True) + assert result.returncode == 0, f'{script} failed: {result.stderr}' + return result + return _inner + + +def timeout_func(func, message, timeout=5): + """Wrap a function in a timeout""" + for _ in range(timeout): + if func(): + break + sleep(1) + else: + raise TimeoutError(message) diff --git a/tests/functional/test_ROSE_ORIG_HOST.py b/tests/functional/test_ROSE_ORIG_HOST.py index b5037da8..743bb518 100644 --- a/tests/functional/test_ROSE_ORIG_HOST.py +++ b/tests/functional/test_ROSE_ORIG_HOST.py @@ -120,7 +120,7 @@ def fixture_install_flow( ) install_conf_path = ( fixture_provide_flow['flowpath'] / - 'runN/opt/rose-suite-cylc-install.conf' + 'opt/rose-suite-cylc-install.conf' ) text = install_conf_path.read_text() text = re.sub('ROSE_ORIG_HOST=.*', 'ROSE_ORIG_HOST=foo', text) @@ -143,7 +143,7 @@ def test_cylc_validate_srcdir(fixture_install_flow, mod_cylc_validate_cli): def test_cylc_validate_rundir(fixture_install_flow, mod_cylc_validate_cli): """Sanity check that workflow validates: """ - flowpath = fixture_install_flow['flowpath'] / 'runN' + flowpath = fixture_install_flow['flowpath'] result = mod_cylc_validate_cli(flowpath) assert 'ROSE_ORIG_HOST (env) is:' in result.logging diff --git a/tests/functional/test_reinstall.py b/tests/functional/test_reinstall.py index db4c5e46..d7507824 100644 --- a/tests/functional/test_reinstall.py +++ b/tests/functional/test_reinstall.py @@ -117,14 +117,14 @@ def test_cylc_install_run(fixture_install_flow): 'file_, expect', [ ( - 'run1/rose-suite.conf', ( + 'rose-suite.conf', ( '# Config Options \'b c (cylc-install)\' from CLI appended to' ' options already in `rose-suite.conf`.\n' 'opts=a b c (cylc-install)\n' ) ), ( - 'run1/opt/rose-suite-cylc-install.conf', ( + 'opt/rose-suite-cylc-install.conf', ( '# This file records CLI Options.\n\n' '!opts=b c\n' f'\n[env]\n#{ROHIOS}\nROSE_ORIG_HOST={HOST}\n' @@ -152,7 +152,7 @@ def fixture_reinstall_flow( """ monkeymodule.delenv('ROSE_SUITE_OPT_CONF_KEYS', raising=False) result = mod_cylc_reinstall_cli( - f'{fixture_provide_flow["test_flow_name"]}/run1', + f'{fixture_provide_flow["test_flow_name"]}', { 'opt_conf_keys': ['d'] } @@ -171,14 +171,14 @@ def test_cylc_reinstall_run(fixture_reinstall_flow): 'file_, expect', [ ( - 'run1/rose-suite.conf', ( + 'rose-suite.conf', ( '# Config Options \'b c d (cylc-install)\' from CLI appended ' 'to options already in `rose-suite.conf`.\n' 'opts=a b c d (cylc-install)\n' ) ), ( - 'run1/opt/rose-suite-cylc-install.conf', ( + 'opt/rose-suite-cylc-install.conf', ( '# This file records CLI Options.\n\n' '!opts=b c d\n' f'\n[env]\n#{ROHIOS}\nROSE_ORIG_HOST={HOST}\n' @@ -213,7 +213,7 @@ def fixture_reinstall_flow2( 'opts=z\n' ) result = mod_cylc_reinstall_cli( - f'{fixture_provide_flow["test_flow_name"]}/run1' + f'{fixture_provide_flow["test_flow_name"]}' ) yield { 'fixture_provide_flow': fixture_provide_flow, @@ -229,14 +229,14 @@ def test_cylc_reinstall_run2(fixture_reinstall_flow2): 'file_, expect', [ ( - 'run1/rose-suite.conf', ( + 'rose-suite.conf', ( '# Config Options \'b c d (cylc-install)\' from CLI appended ' 'to options already in `rose-suite.conf`.\n' 'opts=z b c d (cylc-install)\n' ) ), ( - 'run1/opt/rose-suite-cylc-install.conf', ( + 'opt/rose-suite-cylc-install.conf', ( '# This file records CLI Options.\n\n' '!opts=b c d\n' f'\n[env]\n#{ROHIOS}\nROSE_ORIG_HOST={HOST}\n' diff --git a/tests/functional/test_reinstall_clean.py b/tests/functional/test_reinstall_clean.py index 2883afb5..f443d0ef 100644 --- a/tests/functional/test_reinstall_clean.py +++ b/tests/functional/test_reinstall_clean.py @@ -108,7 +108,7 @@ def test_cylc_install_run(fixture_install_flow): 'file_, expect', [ ( - 'run1/opt/rose-suite-cylc-install.conf', ( + 'opt/rose-suite-cylc-install.conf', ( '# This file records CLI Options.\n\n' '!opts=bar\n\n' '[env]\n' @@ -141,7 +141,7 @@ def fixture_reinstall_flow( """ monkeymodule.delenv('ROSE_SUITE_OPT_CONF_KEYS', raising=False) result = mod_cylc_reinstall_cli( - f'{fixture_provide_flow["test_flow_name"]}/run1', + f'{fixture_provide_flow["test_flow_name"]}', { 'opt_conf_keys': ['baz'], 'defines': ['[env]BAR=2'], @@ -162,7 +162,7 @@ def test_cylc_reinstall_run(fixture_reinstall_flow): 'file_, expect', [ ( - 'run1/opt/rose-suite-cylc-install.conf', ( + 'opt/rose-suite-cylc-install.conf', ( '# This file records CLI Options.\n\n' '!opts=baz\n\n' '[env]\n' diff --git a/tests/functional/test_reinstall_overrides.py b/tests/functional/test_reinstall_overrides.py new file mode 100644 index 00000000..8c9a0367 --- /dev/null +++ b/tests/functional/test_reinstall_overrides.py @@ -0,0 +1,76 @@ +# THIS FILE IS PART OF THE ROSE-CYLC PLUGIN FOR 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 . +"""Functional tests checking Cylc reinstall and reload behaviour is correct +WRT to deletion of Rose Options at the end of the post install plugin. + +https://github.com/cylc/cylc-rose/pull/312 +""" + + +from pathlib import Path +from textwrap import dedent + + +def test_reinstall_overrides( + cylc_install_cli, + cylc_reinstall_cli, + file_poll, + tmp_path, + purge_workflow, + run_ok +): + """When reinstalling and reloading the new installation are picked up. + > cylc install this -S 'var=CLIinstall' + > cylc play this --pause + > cylc reinstall this -S 'var=CLIreinstall' + > cylc play this --pause + See https://github.com/cylc/cylc-flow/issues/5968 + """ + (tmp_path / 'flow.cylc').write_text(dedent(""" #!jinja2 + [scheduling] + [[graph]] + R1 = foo + [runtime] + [[foo]] + script = cylc message -- {{var}} + """)) + (tmp_path / 'rose-suite.conf').write_text( + '[template variables]\nvar="rose-suite.conf"') + + # Install workflow. + install_results = cylc_install_cli( + tmp_path, {'rose_template_vars': ['var="CLIinstall"']}) + assert install_results.ret == 0 + + # Play workflow + run_ok(f'cylc play --pause {install_results.id}') + + # Reinstall the workflow: + reinstall_results = cylc_reinstall_cli( + install_results.id, + {'rose_template_vars': ['var="CLIreinstall"']}) + assert reinstall_results.ret == 0 + + # Reload the workflow: + run_ok(f'cylc reload {install_results.id}') + + # The config being run has been modified: + run_dir = Path.home() / 'cylc-run' / install_results.id + config_log = (run_dir / 'log/config/02-reload-01.cylc') + file_poll(config_log) + assert 'cylc message -- CLIreinstall' in config_log.read_text() + + purge_workflow(install_results.id) diff --git a/tests/functional/test_rose_fileinstall.py b/tests/functional/test_rose_fileinstall.py index e11a496f..0b134122 100644 --- a/tests/functional/test_rose_fileinstall.py +++ b/tests/functional/test_rose_fileinstall.py @@ -85,7 +85,7 @@ def test_rose_fileinstall_subfolders(fixture_install_flow): """File installed into a sub directory: """ _, datapath, _, _, destpath = fixture_install_flow - assert ((destpath / 'runN/lib/python/lion.py').read_text() == + assert ((destpath / 'lib/python/lion.py').read_text() == (datapath / 'lion.py').read_text()) @@ -93,7 +93,7 @@ def test_rose_fileinstall_concatenation(fixture_install_flow): """Multiple files concatenated on install(source contained wildcard): """ _, datapath, _, _, destpath = fixture_install_flow - assert ((destpath / 'runN/data').read_text() == + assert ((destpath / 'data').read_text() == ((datapath / 'randoms1.data').read_text() + (datapath / 'randoms3.data').read_text() )) diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py index 9f8791be..9ff625e6 100644 --- a/tests/unit/test_functional_post_install.py +++ b/tests/unit/test_functional_post_install.py @@ -58,7 +58,7 @@ def assert_rose_conf_full_equal(left, right, no_ignore=True): def test_no_rose_suite_conf_in_devdir(tmp_path): - result = post_install(srcdir=tmp_path) + result = post_install(srcdir=tmp_path, rundir=None, opts=None) assert result is False @@ -359,4 +359,4 @@ def test_cylc_no_rose(tmp_path): """A Cylc workflow that contains no ``rose-suite.conf`` installs OK. """ from cylc.rose.entry_points import post_install - assert post_install(srcdir=tmp_path, rundir=tmp_path) is False + assert post_install(srcdir=tmp_path, rundir=tmp_path, opts={}) is False From f33490d1f5aee5172128498817b64e2a05470cd8 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Thu, 2 May 2024 13:17:05 +0100 Subject: [PATCH 2/2] Update tests/functional/test_reinstall_overrides.py --- tests/functional/test_reinstall_overrides.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/test_reinstall_overrides.py b/tests/functional/test_reinstall_overrides.py index be108a35..4a0ff7f9 100644 --- a/tests/functional/test_reinstall_overrides.py +++ b/tests/functional/test_reinstall_overrides.py @@ -74,4 +74,4 @@ async def test_reinstall_overrides( file_poll(config_log) assert 'cylc message -- CLIreinstall' in config_log.read_text() - purge_workflow(wid) \ No newline at end of file + purge_workflow(wid)