diff --git a/cylc/rose/stem.py b/cylc/rose/stem.py index fd156f02..afa4ccbc 100644 --- a/cylc/rose/stem.py +++ b/cylc/rose/stem.py @@ -146,15 +146,17 @@ def __repr__(self): __str__ = __repr__ -class RoseStemVersionException(Exception): +class RoseStemVersionException(CylcError): """Exception class when running the wrong rose-stem version.""" def __init__(self, version): + Exception.__init__(self, version) if version is None: self.suite_version = ( - "does not have ROSE_VERSION set in the rose-suite.conf" + "does not have ROSE_STEM_VERSION set in the " + "rose-suite.conf" ) else: self.suite_version = "at version %s" % (version) @@ -166,7 +168,7 @@ def __repr__(self): __str__ = __repr__ -class RoseSuiteConfNotFoundException(Exception): +class RoseSuiteConfNotFoundException(CylcError): """Exception class when unable to find rose-suite.conf.""" diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py index 919d798c..65d788a0 100644 --- a/cylc/rose/utilities.py +++ b/cylc/rose/utilities.py @@ -339,6 +339,67 @@ def merge_rose_cylc_suite_install_conf(old, new): return old +def parse_cli_defines(define: str) -> Union[ + bool, Tuple[ + List[Union[str, Any]], + Union[str, Any], + Union[str, Any] + ] +]: + """Parse a define string. + + Args: + define: + A string in one of two forms: + - `key = "value"` + - `[section]key = "value"` + + With optional `!` and `!!` prepended, indicating an ignored state, + which should lead to a warning being logged. + + Returns: + False: If state is ignored or trigger-ignored, otherwise... + (keys, value, state) + + Examples: + # Top level key + >>> parse_cli_defines('root-dir = "foo"') + (['root-dir'], '"foo"', '') + + # Marked as ignored + >>> parse_cli_defines('!root-dir = "foo"') + False + + # Inside a section + >>> parse_cli_defines('[section]orange = "segment"') + (['section', 'orange'], '"segment"', '') + """ + match = re.match( + ( + r'^\[(?P
.*)\](?P!{0,2})' + r'(?P.*)\s*=\s*(?P.*)' + ), + define + ) + if match: + groupdict = match.groupdict() + keys = [groupdict['section'].strip(), groupdict['key'].strip()] + else: + # Doesn't have a section: + match = re.match( + r'^(?P!{0,2})(?P.*)\s*=\s*(?P.*)', define) + if match and not match['state']: + groupdict = match.groupdict() + keys = [groupdict['key'].strip()] + else: + # This seems like it ought to be an error, + # But behaviour is consistent with Rose 2019 + # See: https://github.com/cylc/cylc-rose/issues/217 + return False + + return (keys, match['value'], match['state']) + + def get_cli_opts_node(opts=None, srcdir=None): """Create a ConfigNode representing options set on the command line. @@ -379,28 +440,12 @@ def get_cli_opts_node(opts=None, srcdir=None): defines.append(f'[env]ROSE_ORIG_HOST={rose_orig_host}') rose_template_vars.append(f'ROSE_ORIG_HOST={rose_orig_host}') - # Construct new ouput based on optional Configs: + # Construct new config node representing CLI config items: newconfig = ConfigNode() - - # For each __define__ determine whether it is an env or template define. for define in defines: - match = re.match( - ( - r'^\[(?P.*)\](?P!{0,2})' - r'(?P.*)\s*=\s*(?P.*)' - ), - define - ).groupdict() - if match['key1'] == '' and match['state'] in ['!', '!!']: - LOG.warning( - 'CLI opts set to ignored or trigger-ignored will be ignored.' - ) - else: - newconfig.set( - keys=[match['key1'], match['key2']], - value=match['value'], - state=match['state'] - ) + parsed_define = parse_cli_defines(define) + if parsed_define: + newconfig.set(*parsed_define) # For each __suite define__ add define. if srcdir is not None: diff --git a/tests/functional/test_rose_stem.py b/tests/functional/test_rose_stem.py index 122c41e2..01caeb72 100644 --- a/tests/functional/test_rose_stem.py +++ b/tests/functional/test_rose_stem.py @@ -570,7 +570,7 @@ def test_with_config2(self, with_config2, expected): assert expected in with_config2['jobout_content'] -def test_incompatible_versions(setup_stem_repo, monkeymodule): +def test_incompatible_versions(setup_stem_repo, monkeymodule, caplog, capsys): """It fails if trying to install an incompatible version. """ # Copy suite into working copy. @@ -587,7 +587,8 @@ def test_incompatible_versions(setup_stem_repo, monkeymodule): str(setup_stem_repo['workingcopy']), "fcm:foo.x_tr@head", ], - 'workflow_name': str(setup_stem_repo['suitename']) + 'workflow_name': str(setup_stem_repo['suitename']), + 'verbosity': 2, } monkeymodule.setattr('sys.argv', ['stem']) diff --git a/tests/unit/test_config_tree.py b/tests/unit/test_config_tree.py index c93ddc2d..3718785b 100644 --- a/tests/unit/test_config_tree.py +++ b/tests/unit/test_config_tree.py @@ -467,24 +467,6 @@ def test_merge_opts( assert merge_opts(conf, opt_conf_keys) == expected -@pytest.mark.parametrize( - 'state', - ['!', '!!'] -) -def test_cli_defines_ignored_are_ignored( - state, caplog -): - opts = SimpleNamespace( - opt_confs='', defines=[f'[]{state}opts=ignore me'], - rose_template_vars=[] - ) - - get_cli_opts_node(opts) - assert (caplog.records[0].message == - 'CLI opts set to ignored or trigger-ignored will be ignored.' - ) - - @pytest.mark.parametrize( 'opt_confs, defines, rose_template_vars, expect', [ diff --git a/tests/unit/test_rose_stem_units.py b/tests/unit/test_rose_stem_units.py index 460cb1fa..4a9fb6cd 100644 --- a/tests/unit/test_rose_stem_units.py +++ b/tests/unit/test_rose_stem_units.py @@ -278,7 +278,7 @@ def test__check_suite_version_incompatible(get_StemRunner, tmp_path): stemrunner = get_StemRunner( {}, {'stem_sources': [], 'workflow_conf_dir': str(tmp_path)}) with pytest.raises( - RoseStemVersionException, match='ROSE_VERSION' + RoseStemVersionException, match='ROSE_STEM_VERSION' ): stemrunner._check_suite_version(str(tmp_path / 'rose-suite.conf'))