Merge branch '1.3.x' into fix.template_section_name_for_defines

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."""
 

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<section>.*)\](?P<state>!{0,2})'
+            r'(?P<key>.*)\s*=\s*(?P<value>.*)'
+        ),
+        define
+    )
+    if match:
+        groupdict = match.groupdict()
+        keys = [groupdict['section'].strip(), groupdict['key'].strip()]
+    else:
+        # Doesn't have a section:
+        match = re.match(
+            r'^(?P<state>!{0,2})(?P<key>.*)\s*=\s*(?P<value>.*)', 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<key1>.*)\](?P<state>!{0,2})'
-                r'(?P<key2>.*)\s*=\s*(?P<value>.*)'
-            ),
-            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:

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'])

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',
     [

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'))