diff --git a/CHANGES.md b/CHANGES.md index 025b1a7..3e383cc 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,11 @@ creating a new release entry be sure to copy & paste the span tag with the updated. Only the first match gets replaced, so it's fine to leave the old ones in. --> +## __cylc-rose-1.4.2 (Upcoming)__ + +[#345](https://github.com/cylc/cylc-rose/pull/345) - Merge new CLI +options with earlier ones when validating against source. + ## __cylc-rose-1.4.1 (Released 2024-07-23)__ No significant change - Updated to use feature added at Cylc 8.3.3. diff --git a/cylc/rose/entry_points.py b/cylc/rose/entry_points.py index 503382d..44afcc8 100644 --- a/cylc/rose/entry_points.py +++ b/cylc/rose/entry_points.py @@ -27,6 +27,7 @@ load_rose_config, process_config, record_cylc_install_options, + retrieve_installed_cli_opts, rose_config_exists, ) @@ -40,7 +41,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..2eda740 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,61 @@ 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/-S + # Work out whether user has used "template variables", "jinja2:suite.rc" + # or "empy:suite.rc" (There is an assumption that they aren't mixing + # them that is not guarded against): + for section in SECTIONS: + if cli_config.value.get(section, False): + template_variables = cli_config.value.pop(section) + break + + 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 (-D): + new_defines = [] + for l1_key, l1_item in cli_config.value.items(): + if isinstance(l1_item.value, Dict): + for l2_key, l2_item in l1_item.value.items(): + new_defines.append(f'[{l1_key}]{l2_key}={l2_item.value}') + else: + new_defines.append(f'{l1_key}={l1_item.value}') + opts.defines = new_defines + + 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..5a5b012 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,76 @@ 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 +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 +528,26 @@ 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..be99778 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,76 @@ def test_deprecation_warnings( assert must_include in records else: assert must_exclude not in records + + +@pytest.mark.parametrize( + 'tv_string', + (('template variables'), ('jinja2:suite.rc'), ('empy:suite.rc')), +) +def test_retrieve_installed_cli_opts(tmp_path, tv_string): + """It merges src, dest and cli. + """ + # Create a source conifg + rose_suite_conf = tmp_path / 'src/rose-suite.conf' + src = rose_suite_conf.parent + src.mkdir(parents=True) + rose_suite_conf.touch() + (src / 'opt').mkdir() + (src / 'opt/rose-suite-option1.conf').touch() + (src / 'opt/rose-suite-option2.conf').touch() + + # Create saved CLI from earlier CLI: + install_conf = tmp_path / 'run/opt/rose-suite-cylc-install.conf' + install_conf.parent.mkdir(parents=True) + install_conf.write_text( + dedent(f''' + opts = option2 + TOP_LEVEL=false + [env] + ENV_LEAVE=true + ENV_OVERRIDE=false + [{tv_string}] + TV_LEAVE=true + TV_OVERRIDE_D=false + TV_OVERRIDE_S=false + ''') + ) + + opts = SimpleNamespace() + opts.against_source = install_conf.parent.parent + opts.defines = [ + f'[{tv_string}]TV_OVERRIDE_D=True', + '[env]ENV_OVERRIDE=true', + 'TOP_LEVEL=true' + ] + opts.rose_template_vars = ['TV_OVERRIDE_S=True'] + opts.opt_conf_keys = ['option1'] + + opts = retrieve_installed_cli_opts( + srcdir=src, + opts=opts, + ) + + assert opts.opt_conf_keys == ['option2', 'option1'] + + rose_template_vars = [ + o for o in opts.rose_template_vars if 'ROSE_ORIG_HOST' not in o + ] + assert rose_template_vars == [ + 'TV_LEAVE=true', + 'TV_OVERRIDE_D=True', + 'TV_OVERRIDE_S=True', + ] + + defines = [d for d in opts.defines if 'ROSE_ORIG_HOST' not in d] + assert defines == [ + 'TOP_LEVEL=true', + '[env]ENV_LEAVE=true', + '[env]ENV_OVERRIDE=true', + ] + + +def test_retrieve_installed_cli_opts_returns_unchanged(): + """...if clear_rose_install_opts is true.""" + opts = SimpleNamespace(clear_rose_install_opts=True, against_source=True) + assert retrieve_installed_cli_opts('Irrelevant', opts) == opts diff --git a/tests/unit/test_functional_post_install.py b/tests/unit/test_functional_post_install.py index 7ff194e..4a43a80 100644 --- a/tests/unit/test_functional_post_install.py +++ b/tests/unit/test_functional_post_install.py @@ -48,7 +48,7 @@ def assert_rose_conf_full_equal(left, right, no_ignore=True): for keys_1, node_1 in left.walk(no_ignore=no_ignore): node_2 = right.get(keys_1, no_ignore=no_ignore) assert not ( # noqa: E721 - type(node_1) != type(node_2) or + type(node_1) is not type(node_2) or ( not isinstance(node_1.value, dict) and node_1.value != node_2.value @@ -120,7 +120,6 @@ def test_rose_fileinstall_uses_rose_template_vars(tmp_path): ), 'ref/rose-suite.conf': '!opts=foo (cylc-install)', 'ref/opt/rose-suite-foo.conf': '', - 'ref/rose-suite.conf': '!opts=foo (cylc-install)' }, # ENVIRONMENT VARS {}, @@ -155,7 +154,6 @@ def test_rose_fileinstall_uses_rose_template_vars(tmp_path): 'ref/opt/rose-suite-foo.conf': '', 'ref/opt/rose-suite-bar.conf': '', 'ref/opt/rose-suite-baz.conf': '', - 'ref/rose-suite.conf': '!opts=foo bar baz (cylc-install)' }, # ENVIRONMENT VARS {},