diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 503382d..a1948f8 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -27,8 +27,10 @@ load_rose_config, process_config, record_cylc_install_options, + retrieve_installed_cli_opts, rose_config_exists, ) +from metomi.rose.config import ConfigNode if TYPE_CHECKING: from cylc.flow.option_parsers import Values @@ -40,7 +42,15 @@ def pre_configure(srcdir: Path, opts: 'Values') -> dict: # nothing to do here return {} - # load the Rose config + # If we are validating against source, load saved CLI options + # from previous install, as saved in the rose-suite-cylc-install.conf + if ( + getattr(opts, 'against_source', False) + and isinstance(opts.against_source, Path) + ): + opts = retrieve_installed_cli_opts(srcdir, opts) + + # load the source Rose config config_tree = load_rose_config(Path(srcdir), opts=opts) # extract plugin return information from the Rose config diff --git a/cylc/rose/stem.py b/cylc/rose/stem.py index 5972f4c..22fe7ee 100644 --- a/cylc/rose/stem.py +++ b/cylc/rose/stem.py @@ -575,7 +575,7 @@ def get_rose_stem_opts(): " the groups into a series of tasks to run." ), action="append", - metavar="PATH/TO/FLOW", + metavar="STRING", default=[], dest="stem_groups") rose_stem_options.add_option( diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index 570138d..ebd2ad4 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -883,6 +883,7 @@ def record_cylc_install_options( srcdir: Path, rundir: Path, opts: 'Values', + modify: bool = True, ) -> Tuple[ConfigNode, ConfigNode]: """Create/modify files recording Cylc install config options. @@ -909,6 +910,9 @@ def record_cylc_install_options( Equivalent of ``rose suite-run --define KEY=VAL`` - rose_template_vars (list of str): Equivalent of ``rose suite-run --define-suite KEY=VAL`` + modify: + If ``True``, the modified rose-suite-cylc-install.conf will be + written. If ``False``, this will only read files. Returns: Tuple - (cli_config, rose_suite_conf) @@ -957,8 +961,9 @@ def record_cylc_install_options( ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING ] - cli_config.comments = [' This file records CLI Options.'] - dumper.dump(cli_config, str(conf_filepath)) + if modify: + cli_config.comments = [' This file records CLI Options.'] + dumper.dump(cli_config, str(conf_filepath)) # Merge the opts section of the rose-suite.conf with those set by CLI: rose_conf_filepath.touch() @@ -968,7 +973,8 @@ def record_cylc_install_options( ) identify_templating_section(rose_suite_conf) - dumper(rose_suite_conf, rose_conf_filepath) + if modify: + dumper(rose_suite_conf, rose_conf_filepath) return cli_config, rose_suite_conf @@ -999,3 +1005,52 @@ def copy_config_file( shutil.copy2(srcdir_rose_conf, rundir_rose_conf) return True + + +def retrieve_installed_cli_opts(srcdir, opts): + """Merge options from previous install, this install and the CLI. + + Allows validation of merged config for pre-configure where the + --against-source argument is used in a Cylc script. + """ + + # if opts.against_source is a path then we are validating a source + # directory against installed options + rundir = opts.against_source + + # If the calling script doesn't have this option (Everything except + # Cylc VR) then this defaults to false. If it's true we skip all + # following logic: + if not getattr(opts, 'clear_rose_install_opts', False): + opts.clear_rose_install_opts = False + else: + return opts + + # NOTE: we only need to load the rose-suite-cylc-install for this + cli_config, _ = record_cylc_install_options( + srcdir, rundir, opts, modify=False + ) + + # Get opt_conf_keys stored in rose-suite-cylc-install.conf + opt_conf_keys = cli_config.value.pop('opts').value.split(' ') + if any(opt_conf_keys): + opts.opt_conf_keys = opt_conf_keys + + # Get --suite-defines/-D + template_variables = cli_config.value.pop('template variables') + if any(template_variables): + opts.rose_template_vars = [ + f'{k}={v.value}' for k,v + in template_variables.value.items() + if v.state == '' + ] + + # Get all other keys: + for l1_key, l1_item in cli_config.value.items(): + if isinstance(l1_item, ConfigNode): + for l2_key, l2_item in l1_item.value.items(): + opts.defines.append(f'[{l1_key}]{l2_key}={l2_item.value}') + else: + opts.defines.append(f'{l1_key}={l1_item.value}') + + return opts diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..9aa717e --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,23 @@ +# Not mandated to use these tools, but if you do: + +[tool.ruff] +line-length = 79 +target-version = "py37" + +[tool.ruff.format] +quote-style = "preserve" + + +[tool.black] +line-length = 79 +target-version = ['py37'] +skip-string-normalization = true + + +[tool.isort] +profile = "black" +line_length = 79 +force_grid_wrap = 2 +lines_after_imports = 2 +combine_as_imports = true +force_sort_within_sections = true \ No newline at end of file diff --git a/tests/conftest.py b/tests/conftest.py index 7c46c04..1c8d1d1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -16,16 +16,18 @@ import asyncio from functools import partial +import importlib from io import StringIO from pathlib import Path from shlex import split -from shutil import rmtree +from shutil import rmtree, copytree from subprocess import run from time import sleep from types import SimpleNamespace from uuid import uuid4 import pytest +from pytest import UsageError from cylc.flow import __version__ as CYLC_VERSION from cylc.flow.option_parsers import Options @@ -169,15 +171,20 @@ def _pytest_passed(request: pytest.FixtureRequest) -> bool: )) -def _cylc_validate_cli(capsys, caplog): - """Access the validate CLI""" - async def _inner(srcpath, args=None): - parser = validate_gop() +def _cylc_inspection_cli(capsys, caplog, script, gop): + """Access the CLI for cylc scripts inspecting configurations + """ + async def _inner(srcpath, args=None, n_args=3): + parser = gop() options = Options(parser, args)() output = SimpleNamespace() try: - await cylc_validate(parser, options, str(srcpath)) + if n_args == 3: + await script(parser, options, str(srcpath)) + if n_args == 2: + # Don't include the parser: + await script(options, str(srcpath)) output.ret = 0 output.exc = '' except Exception as exc: @@ -270,12 +277,88 @@ def mod_cylc_reinstall_cli(mod_test_dir): @pytest.fixture def cylc_validate_cli(capsys, caplog): - return _cylc_validate_cli(capsys, caplog) - + return _cylc_inspection_cli(capsys, caplog, cylc_validate, validate_gop) @pytest.fixture(scope='module') def mod_cylc_validate_cli(mod_capsys, mod_caplog): - return _cylc_validate_cli(mod_capsys, mod_caplog) + return _cylc_inspection_cli( + mod_capsys, mod_caplog, cylc_validate, validate_gop + ) + +@pytest.fixture +def cylc_list_cli(capsys, caplog): + from cylc.flow.scripts.list import ( + get_option_parser as list_gop, + _main as cylc_list, + ) + return _cylc_inspection_cli(capsys, caplog, cylc_list, list_gop) + + +@pytest.fixture +def cylc_graph_cli(capsys, caplog): + from cylc.flow.scripts.graph import get_option_parser, _main + return _cylc_inspection_cli(capsys, caplog, _main, get_option_parser) + +@pytest.fixture +async def cylc_inspect_scripts(capsys, caplog): + """Run all the common Cylc Test scripts likely to call pre-configure. + + * config + * graph + * list + * validate + * view + + n.b. + * Function adds arg ``--reference`` to supress image being displayed. + """ + + async def _inner(wid, args): + results = {} + + # Handle scripts taking a parser or just the output of the parser: + for script_name, n_args in { + 'config': 3, + 'list': 3, + 'graph': 3, + 'view': 2, + 'validate': 3, + }.items(): + + # Import the script modules: + script_module = importlib.import_module( + f'cylc.flow.scripts.{script_name}' + ) + + # Deal with inconsistent API from Cylc: + if hasattr(script_module, '_main'): + script = script_module._main + elif hasattr(script_module, 'run'): + script = script_module.run + else: + raise UsageError( + f'Script "{script}\'s" module does not contain a ' + '"_main" or "run" function' + ) + + # Supress cylc-graph giving a graphical output: + if script_name == 'graph': + args['reference'] = True + + results[script_name] = await _cylc_inspection_cli( + capsys, + caplog, + script, + script_module.get_option_parser, + )(wid, args, n_args=n_args) + + # Check outputs + assert all([output.ret == 0 for output in results.values()]) + + # Return results for more checking if required: + return results + + return _inner @pytest.fixture @@ -457,3 +540,23 @@ def timeout_func(func, message, timeout=5): sleep(1) else: raise TimeoutError(message) + + +@pytest.fixture +def setup_workflow_source_dir(tmp_path): + """Copy a workflow from the codebase to a temp-file-path + and provide that path for use in tests + """ + def _inner(code_src): + nonlocal tmp_path + # Set up paths for test: + srcpath = tmp_path / 'src' + srcpath.mkdir() + + # the files to install are stored in a directory alongside this test file + datapath = Path(__file__).parent / code_src + copytree(datapath, srcpath, dirs_exist_ok=True) + + # Create source workflow: + return srcpath, datapath + yield _inner diff --git a/tests/functional/15_reinstall_using_old_clivars/flow.cylc b/tests/functional/15_reinstall_using_old_clivars/flow.cylc new file mode 100644 index 0000000..a877115 --- /dev/null +++ b/tests/functional/15_reinstall_using_old_clivars/flow.cylc @@ -0,0 +1,11 @@ +#!jinja2 +{{ assert(SUITE is defined, "1.1 Test --rose-template-variable") }} +{{ assert(DEFINE is defined, "1.2 Test --define") }} +{{ assert(VAL_FROM_OPT is defined, "1.3 Test --opt-conf-keys") }} + +# Just enough Cylc to validate: +[scheduler] + allow implicit tasks = True +[scheduling] + [[graph]] + R1 = foo diff --git a/tests/functional/15_reinstall_using_old_clivars/opt/rose-suite-choc.conf b/tests/functional/15_reinstall_using_old_clivars/opt/rose-suite-choc.conf new file mode 100644 index 0000000..9523798 --- /dev/null +++ b/tests/functional/15_reinstall_using_old_clivars/opt/rose-suite-choc.conf @@ -0,0 +1,2 @@ +[template variables] +VAL_FROM_OPT=3 diff --git a/tests/functional/15_reinstall_using_old_clivars/rose-suite.conf b/tests/functional/15_reinstall_using_old_clivars/rose-suite.conf new file mode 100644 index 0000000..e69de29 diff --git a/tests/functional/test_pre_configure.py b/tests/functional/test_pre_configure.py index 5b597ba..83ce285 100644 --- a/tests/functional/test_pre_configure.py +++ b/tests/functional/test_pre_configure.py @@ -219,3 +219,39 @@ def test_cylc_script(monkeypatch, option, envvars, cmd, expect): '05_opts_set_from_rose_suite_conf') output = run(split(f'{cmd} {srcpath} {option}'), capture_output=True) assert expect in output.stdout + + +async def test_validate_against_source( + setup_workflow_source_dir, + cylc_validate_cli, + cylc_install_cli, + cylc_inspect_scripts, +): + """Ensure that validation against source picks up + on existing configs saved in rose-suite-cylc-install.conf + """ + src, _ = setup_workflow_source_dir( + 'functional/15_reinstall_using_old_clivars' + ) + + opts = { + "rose_template_vars": ["SUITE=1"], + "defines": ["[template variables]DEFINE=2"], + "opt_conf_keys": ['choc'], + } + + # Check that we can validate the source dir with options: + await cylc_inspect_scripts(src, opts) + + # Install our workflow with options. + wid, _ = await cylc_install_cli(src, opts=opts) + + # Check all scripts: + await cylc_inspect_scripts(wid, {"against_source": True}) + + # Reinstall fails if we clear rose install opts: + clear_install_validate = await cylc_validate_cli( + wid, {"against_source": True, 'clear_rose_install_opts': True} + ) + assert clear_install_validate.ret != 0 + assert 'Jinja2 Assertion Error' in str(clear_install_validate.exc.args[0]) diff --git a/tests/unit/test_config_node.py b/tests/unit/test_config_node.py index 8254dc1..e77197c 100644 --- a/tests/unit/test_config_node.py +++ b/tests/unit/test_config_node.py @@ -16,6 +16,7 @@ """Tests the plugin with Rose suite configurations via the Python API.""" +from textwrap import dedent from types import SimpleNamespace from metomi.isodatetime.datetimeoper import DateTimeOperator @@ -34,6 +35,7 @@ process_config, id_templating_section, identify_templating_section, + retrieve_installed_cli_opts, ) @@ -345,3 +347,55 @@ def test_deprecation_warnings( assert must_include in records else: assert must_exclude not in records + + +def test_retrieve_installed_cli_opts(tmp_path): + """It merges src, dest and cli. + """ + rose_suite_conf = tmp_path / 'src/rose-suite.conf' + install_conf = tmp_path / 'run/opt/rose-suite-cylc-install.conf' + rose_suite_conf.parent.mkdir(parents=True) + install_conf.parent.mkdir(parents=True) + + rose_suite_conf.write_text( + dedent(''' + # On reinstallation this should be a change: + CHANGE_FM_SRC=True + # On reinstallation this should be added: + ADD=True + ''') + ) + install_conf.write_text( + dedent(''' + [template variables] + CHANGE_FM_SRC=False + CHANGE_FM_CLI_S=False + CHANGE_FM_CLI_D=False + ONLY_FROM_PREV=True + ''') + ) + + opts = SimpleNamespace() + opts.against_source = install_conf.parent.parent + opts.defines = ['[template variables]CHANGE_FM_CLI_D=True'] + opts.suite_defines = ['CHANGE_FM_CLI_S=True'] + + opts = retrieve_installed_cli_opts( + srcdir=rose_suite_conf.parent, + opts=opts, + ) + from cylc.rose.utilities import load_rose_config + from pathlib import Path + + config = load_rose_config(Path(rose_suite_conf.parent), opts=opts) + results = { + k: v.value + for k, v in config.node.value['template variables'].value.items() + if v.value in ['True', 'False'] + } + assert results == { + 'CHANGE_FM_CLI_D': 'True', + 'CHANGE_FM_SRC': 'True', + 'CHANGE_FM_CLI_S': 'True', + 'ONLY_FROM_PREV': 'True', + }