From d594eb6d982681cf054ce668b4d3f4288b9cbdb4 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Wed, 31 Aug 2022 15:16:52 +0100 Subject: [PATCH 1/3] Don't pass rose variables with state ! or !! to Cylc. (#171) replace cylc validate with cylc view --jinja2 in test --- CHANGES.md | 8 ++++ cylc/rose/utilities.py | 4 ++ .../test_validate_no_rose_ignore_items.py | 45 +++++++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 tests/functional/test_validate_no_rose_ignore_items.py diff --git a/CHANGES.md b/CHANGES.md index 1b2dd805..3b05ae59 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -5,6 +5,14 @@ creating a new release entry be sure to copy & paste the span tag with the `actions:bind` attribute, which is used by a regex to find the text to be updated. Only the first match gets replaced, so it's fine to leave the old ones in. --> +## __cylc-rose-1.1.1 (Upcoming)__ + +### Fixes + +[#171](https://github.com/cylc/cylc-rose/pull/171) - Fix bug where Cylc Rose +passed `rose-suite.conf` items commented with `!` or `!!` to Cylc regardless. + + ## __cylc-rose-1.1.0 (Released 2022-07-28)__ ### Fixes diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index 29d2e4ca..606fb2b1 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -129,19 +129,23 @@ def get_rose_vars_from_config_node(config, config_node, environ): # For each of the template language sections extract items to a simple # dict to be returned. if 'env' in config_node.value: + config['env'] = { item[0][1]: item[1].value for item in config_node.value['env'].walk() + if item[1].state == ConfigNode.STATE_NORMAL } if templating in config_node.value: config['template_variables'] = { item[0][1]: item[1].value for item in config_node.value[templating].walk() + if item[1].state == ConfigNode.STATE_NORMAL } elif 'template variables' in config_node.value: config['template_variables'] = { item[0][1]: item[1].value for item in config_node.value['template variables'].walk() + if item[1].state == ConfigNode.STATE_NORMAL } # Add the entire config to ROSE_SUITE_VARIABLES to allow for programatic diff --git a/tests/functional/test_validate_no_rose_ignore_items.py b/tests/functional/test_validate_no_rose_ignore_items.py new file mode 100644 index 00000000..5f79b194 --- /dev/null +++ b/tests/functional/test_validate_no_rose_ignore_items.py @@ -0,0 +1,45 @@ +# 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 Test: It ignores commented items in rose-suite.conf + +See https://github.com/cylc/cylc-rose/pull/171 +""" + +from shlex import split +from subprocess import run + + +def test_cylc_validate(tmp_path): + """It doesn't pass commented vars to Cylc. + """ + (tmp_path / 'flow.cylc').write_text("""#!jinja2 +{{ assert(UNCOMMENTED is defined, "UNCOMMENTED is not defined") }} +{{ assert(SINGLE is not defined, "SINGLE is defined") }} +{{ assert(DOUBLE is not defined, "DOUBLE is defined") }} + """) + (tmp_path / 'rose-suite.conf').write_text( + '[template variables]\n' + 'UNCOMMENTED="bar"\n' + '!SINGLE="bar"\n' + '!!DOUBLE="baz"\n' + ) + result = run( + split(f'cylc view --jinja2 {str(tmp_path)}'), capture_output=True) + + if result.returncode == 0: + assert True + else: + raise Exception(result.stderr.decode()) From c596df358c86c282a94c677e36539f85d860a18d Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Mon, 12 Sep 2022 13:23:57 +0100 Subject: [PATCH 2/3] Rose Stem Fixes (#172) * Ensured that opts.source is not unset when opts is passed to the StemRunner object update the help docs to match Rose 2019 fix broken test increase or decrease the verbosity of the logging. * fix stuff * fixed opts.verbose --- CHANGES.md | 3 + cylc/rose/stem.py | 118 ++++++++++++++++++++++------- tests/functional/test_rose_stem.py | 14 ++-- tests/unit/test_rose_stem_units.py | 15 ++-- 4 files changed, 108 insertions(+), 42 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 3b05ae59..a4677a0a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -30,6 +30,9 @@ a database locking issue with the `rose_prune` built-in app. ### Fixes +[#172](https://github.com/cylc/cylc-rose/pull/172) - Allow getting a workflow +name when source is not an SVN repo. + [#139](https://github.com/cylc/cylc-rose/pull/139) - Make `rose stem` command work correctly with changes made to `cylc install` in [cylc-flow PR #4823](https://github.com/cylc/cylc-flow/pull/4823) diff --git a/cylc/rose/stem.py b/cylc/rose/stem.py index 04fb0592..d94d3410 100644 --- a/cylc/rose/stem.py +++ b/cylc/rose/stem.py @@ -23,25 +23,54 @@ To run a rose-stem suite use "cylc play". -Looks for a suite to install either in $PWD/rose-stem, or in a specified -location - e.g: - -# Install a rose-stem suite from PWD/rose-stem -$ rose stem - -# Install a rose-stem suite from PWD/relative-path. -$ rose stem relative-path - -# Install a rose-stem suite from specified abs path. -$ rose stem /absolute/path +Install a suitable suite with a specified set of source tree(s). + +Default values of some of these settings are suite-dependent, specified +in the `rose-suite.conf` file. + +Examples + + rose stem --group=developer + rose stem --source=/path/to/source --source=/other/source --group=mygroup + rose stem --source=foo=/path/to/source --source=bar=fcm:bar_tr@head + +Jinja2 Variables + + Note that `` refers to the FCM keyword name of the repository in + upper case. + + HOST_SOURCE_ + The complete list of source trees for a given project. Working copies + in this list have their hostname prefixed, e.g. `host:/path/wc`. + HOST_SOURCE__BASE + The base of the project specified on the command line. This is + intended to specify the location of `fcm-make` config files. Working + copies in this list have their hostname prefixed. + RUN_NAMES + A list of groups to run in the rose-stem suite. + SOURCE_ + The complete list of source trees for a given project. Unlike the + `HOST_` variable of similar name, paths to working copies do NOT + include the host name. + SOURCE__BASE + The base of the project specified on the command line. This is + intended to specify the location of `fcm-make` config files. Unlike + the `HOST_` variable of similar name, paths to working copies do NOT + include the host name. + SOURCE__REV + The revision of the project specified on the command line. This + is intended to specify the revision of `fcm-make` config files. """ +from ansimarkup import parse as cparse from contextlib import suppress +from optparse import OptionGroup import os from pathlib import Path import re import sys +from cylc.flow.exceptions import CylcError from cylc.flow.scripts.install import ( get_option_parser, install as cylc_install @@ -55,6 +84,7 @@ from metomi.rose.resource import ResourceLocator +EXC_EXIT = cparse('{name}: {exc}') DEFAULT_TEST_DIR = 'rose-stem' ROSE_STEM_VERSION = 1 SUITE_RC_PREFIX = '[jinja2:suite.rc]' @@ -96,7 +126,7 @@ def __repr__(self): __str__ = __repr__ -class ProjectNotFoundException(Exception): +class ProjectNotFoundException(CylcError): """Exception class when unable to determine project a source belongs to.""" @@ -482,6 +512,7 @@ def get_source_opt_from_args(opts, args): """ if len(args) == 0: # sourcedir not given: + opts.source = None return opts elif os.path.isabs(args[-1]): # sourcedir given, and is abspath: @@ -501,38 +532,69 @@ def main(): # TODO: add any rose stem specific CLI args that might exist # On inspection of rose/lib/python/rose/opt_parse.py it turns out that # opts.group is stored by the --task option. - parser.add_option( - "--task", "--group", - help="Specifies a source tree to include in a suite.", + rose_stem_options = OptionGroup(parser, 'Rose Stem Specific Options') + rose_stem_options.add_option( + "--task", "--group", "-t", "-g", + help=( + "Specify a group name to run. Additional groups can be specified" + "with further `--group` arguments. The suite will then convert the" + "groups into a series of tasks to run." + ), action="append", metavar="PATH/TO/FLOW", default=[], dest="stem_groups") - parser.add_option( - "--source", - help="Specifies a source tree to include in a suite.", + rose_stem_options.add_option( + "--source", '-s', + help=( + "Specify a source tree to include in a rose-stem suite. The first" + "source tree must be a working copy as the location of the suite" + "and fcm-make config files are taken from it. Further source" + "trees can be added with additional `--source` arguments. " + "The project which is associated with a given source is normally " + "automatically determined using FCM, however the project can " + "be specified by putting the project name as the first part of " + "this argument separated by an equals sign as in the third " + "example above. Defaults to `.` if not specified." + ), action="append", metavar="PATH/TO/FLOW", default=[], dest="stem_sources") - # Hard-set for now, but could be set based upon cylc verbosity levels? - parser.add_option('--verbosity', default=1) - parser.add_option('--quietness', default=0) + parser.add_option_group(rose_stem_options) + parser.usage = __doc__ opts, args = parser.parse_args(sys.argv[1:]) # sliced sys.argv to drop 'rose-stem' opts = get_source_opt_from_args(opts, args) - # modify the CLI options to add whatever rose stem would like to add - opts = StemRunner(opts).process() + # Verbosity is set using the Cylc options which decrement and increment + # verbosity as part of the parser, but rose needs opts.quietness too, so + # hard set it. + opts.quietness = 0 - # call cylc install - if hasattr(opts, 'source'): - cylc_install(parser, opts, opts.source) - else: - cylc_install(parser, opts) + try: + # modify the CLI options to add whatever rose stem would like to add + opts = StemRunner(opts).process() + + # call cylc install + if hasattr(opts, 'source'): + cylc_install(parser, opts, opts.source) + else: + cylc_install(parser, opts) + + except CylcError as exc: + if opts.verbosity > 1: + raise exc + print( + EXC_EXIT.format( + name=exc.__class__.__name__, + exc=exc + ), + file=sys.stderr + ) if __name__ == "__main__": diff --git a/tests/functional/test_rose_stem.py b/tests/functional/test_rose_stem.py index b2b122b2..f00ae0fd 100644 --- a/tests/functional/test_rose_stem.py +++ b/tests/functional/test_rose_stem.py @@ -21,6 +21,8 @@ Tests for cylc.rose.stem ======================== +These tests are based on the Rose 2019 tests for Rose Stem. + Structure --------- @@ -173,7 +175,10 @@ def setup_stem_repo(tmp_path_factory, monkeymodule, request): 'suite_install_dir': suite_install_dir } # Only clean up if all went well. - if not request.session.testsfailed: + if ( + not request.session.testsfailed + and Path(suite_install_dir).exists() + ): shutil.rmtree(suite_install_dir) @@ -613,7 +618,6 @@ class TestProjectNotInKeywords: def test_project_not_in_keywords(self, project_not_in_keywords): """test for successful execution with site/user configuration """ - assert project_not_in_keywords.returncode == 1 - assert (b'Cannot ascertain project for source tree' in - project_not_in_keywords.stderr - ) + assert project_not_in_keywords.returncode == 0 + stderr = project_not_in_keywords.stderr.decode() + assert 'Cannot ascertain project for source tree' in stderr diff --git a/tests/unit/test_rose_stem_units.py b/tests/unit/test_rose_stem_units.py index 7b19c900..b9e5d5e4 100644 --- a/tests/unit/test_rose_stem_units.py +++ b/tests/unit/test_rose_stem_units.py @@ -27,7 +27,7 @@ [ pytest.param( [], - '', + None, id='no-path' ), pytest.param( @@ -43,17 +43,14 @@ ] ) def test_get_source_opt_from_args(tmp_path, monkeypatch, args, expect): - # Basic setup + """It converts Rose 2 CLI features to options usable by Rose Stem + """ monkeypatch.chdir(tmp_path) opts = SimpleNamespace() - # Run function - result = get_source_opt_from_args(opts, args) + result = get_source_opt_from_args(opts, args).source - # If an arg is given we are expecting source to be added to the options. - # Otherwise options should be returned unchanged. - if expect: - expect = SimpleNamespace(source=expect.format(tmp_path=tmp_path)) + if expect is None: assert result == expect else: - assert result == opts + assert result == expect.format(tmp_path=str(tmp_path)) From 0a13ddeefa4733564657e188a2d898eb2c441fdf Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 14 Sep 2022 12:43:03 +0100 Subject: [PATCH 3/3] Prepare release: 1.1.1 (#183) * Prepare release 1.1.1 Workflow: Release stage 1 - create release PR, run: 27 * fixed changes.md Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Tim Pillinger <26465611+wxtim@users.noreply.github.com> --- CHANGES.md | 7 +++---- cylc/rose/__init__.py | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a4677a0a..4b39952c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -5,13 +5,15 @@ creating a new release entry be sure to copy & paste the span tag with the `actions:bind` attribute, which is used by a regex to find the text to be updated. Only the first match gets replaced, so it's fine to leave the old ones in. --> -## __cylc-rose-1.1.1 (Upcoming)__ +## __cylc-rose-1.1.1 (Released 2022-09-14)__ ### Fixes [#171](https://github.com/cylc/cylc-rose/pull/171) - Fix bug where Cylc Rose passed `rose-suite.conf` items commented with `!` or `!!` to Cylc regardless. +[#172](https://github.com/cylc/cylc-rose/pull/172) - Allow getting a workflow +name when source is not an SVN repo. ## __cylc-rose-1.1.0 (Released 2022-07-28)__ @@ -30,9 +32,6 @@ a database locking issue with the `rose_prune` built-in app. ### Fixes -[#172](https://github.com/cylc/cylc-rose/pull/172) - Allow getting a workflow -name when source is not an SVN repo. - [#139](https://github.com/cylc/cylc-rose/pull/139) - Make `rose stem` command work correctly with changes made to `cylc install` in [cylc-flow PR #4823](https://github.com/cylc/cylc-flow/pull/4823) diff --git a/cylc/rose/__init__.py b/cylc/rose/__init__.py index 3f8a416b..c5ae7301 100644 --- a/cylc/rose/__init__.py +++ b/cylc/rose/__init__.py @@ -205,4 +205,4 @@ """ -__version__ = '1.1.1.dev' +__version__ = '1.1.1'