Permitted warnings about root-dir for all versions of Cylc. (#231)

prevent warnings about root-dir for all versions of Cylc. Co-authored-by: Oliver Sanders <[email protected]> Co-authored-by: Ronnie Dutta <[email protected]>
cylc · Jul 17, 2023 · ca95d8e · ca95d8e
1 parent 0acb74f
commit ca95d8e
Show file tree

Hide file tree

Showing 5 changed files with 93 additions and 28 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -6,14 +6,17 @@ 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.3.0 (<span actions:bind='release-date'>Awaiting Release</span>)__
+## __cylc-rose-1.2.1 (<span actions:bind='release-date'>Upcoming</span>)__
 
 ### Fixes
 
 [#229](https://github.com/cylc/cylc-rose/pull/229) -
 Fix bug which stops rose-stem suites using the new `[template variables]` section
 in their `rose-suite.conf` files.
 
+[#231](https://github.com/cylc/cylc-rose/pull/231) - Show warning about
+`root-dir` config setting in compatibility mode.
+
 ## __cylc-rose-1.2.0 (<span actions:bind='release-date'>Released 2023-01-16</span>)__
 
 ### Fixes

diff --git a/cylc/rose/utilities.py b/cylc/rose/utilities.py
@@ -25,7 +25,7 @@
 
 from cylc.flow.hostuserutil import get_host
 from cylc.flow import LOG
-import cylc.flow.flags as flags
+from cylc.flow.flags import cylc7_back_compat
 from cylc.rose.jinja2_parser import Parser, patch_jinja2_leading_zeros
 from metomi.rose import __version__ as ROSE_VERSION
 from metomi.isodatetime.datetimeoper import DateTimeOperator
@@ -42,6 +42,8 @@
 ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING = (
     ' ROSE_ORIG_HOST set by cylc install.'
 )
+MESSAGE = 'message'
+ALL_MODES = 'all modes'
 
 
 class MultipleTemplatingEnginesError(Exception):
@@ -656,30 +658,50 @@ def deprecation_warnings(config_tree):
         - "root-dir"
         - "jinja2:suite.rc"
         - "empy:suite.rc"
+        - root-dir
 
+    If ALL_MODES is True this deprecation will ignore whether there is a
+    flow.cylc or suite.rc in the workflow directory.
     """
 
     deprecations = {
-        'empy:suite.rc': (
-            "'rose-suite.conf[empy:suite.rc]' is deprecated."
-            " Use [template variables] instead."),
-        'jinja2:suite.rc': (
-            "'rose-suite.conf[jinja2:suite.rc]' is deprecated."
-            " Use [template variables] instead."),
-        'empy:flow.cylc': (
-            "'rose-suite.conf[empy:flow.cylc]' is not used by Cylc."
-            " Use [template variables] instead."),
-        'jinja2:flow.cylc': (
-            "'rose-suite.conf[jinja2:flow.cylc]' is not used by Cylc."
-            " Use [template variables] instead."),
-        'root-dir': (
-            'You have set "rose-suite.conf[root-dir]", '
-            'which is not supported at '
-            'Cylc 8. Use `[install] symlink dirs` in global.cylc '
-            'instead.')
+        'empy:suite.rc': {
+            MESSAGE: (
+                "'rose-suite.conf[empy:suite.rc]' is deprecated."
+                " Use [template variables] instead."),
+            ALL_MODES: False,
+        },
+        'jinja2:suite.rc': {
+            MESSAGE: (
+                "'rose-suite.conf[jinja2:suite.rc]' is deprecated."
+                " Use [template variables] instead."),
+            ALL_MODES: False,
+        },
+        'empy:flow.cylc': {
+            MESSAGE: (
+                "'rose-suite.conf[empy:flow.cylc]' is not used by Cylc."
+                " Use [template variables] instead."),
+            ALL_MODES: False,
+        },
+        'jinja2:flow.cylc': {
+            MESSAGE: (
+                "'rose-suite.conf[jinja2:flow.cylc]' is not used by Cylc."
+                " Use [template variables] instead."),
+            ALL_MODES: False,
+        },
+        'root-dir': {
+            MESSAGE: (
+                'You have set "rose-suite.conf[root-dir]", '
+                'which is not supported at '
+                'Cylc 8. Use `[install] symlink dirs` in global.cylc '
+                'instead.'),
+            ALL_MODES: True,
+        },
     }
-    if not flags.cylc7_back_compat:
-        for string in list(config_tree.node):
-            for deprecation in deprecations.keys():
-                if deprecation in string.lower():
-                    LOG.warning(deprecations[deprecation])
+    for string in list(config_tree.node):
+        for name, info in deprecations.items():
+            if (
+                (info[ALL_MODES] or not cylc7_back_compat)
+                and name in string.lower()
+            ):
+                LOG.warning(info[MESSAGE])
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -102,7 +102,6 @@ def _cylc_validate_cli(capsys, caplog):
     def _inner(srcpath, args=None):
         parser = validate_gop()
         options = Options(parser, args)()
-
         output = SimpleNamespace()
 
         try:
@@ -130,7 +129,6 @@ def _inner(srcpath, args=None):
             args: Dictionary of arguments.
         """
         options = Options(install_gop(), args)()
-
         output = SimpleNamespace()
 
         try:
@@ -156,7 +154,6 @@ def _inner(workflow_id, opts=None):
             args: Dictionary of arguments.
         """
         options = Options(reinstall_gop(), opts)()
-
         output = SimpleNamespace()
 
         try:

diff --git a/tests/functional/test_pre_configure.py b/tests/functional/test_pre_configure.py
@@ -157,7 +157,7 @@ def test_warn_if_old_templating_set(
 ):
     """Test using unsupported root-dir config raises error."""
     monkeypatch.setattr(
-        cylc.rose.utilities.flags, 'cylc7_back_compat', compat_mode
+        cylc.rose.utilities, 'cylc7_back_compat', compat_mode
     )
     (tmp_path / 'rose-suite.conf').write_text(f'[{rose_config}]')
     get_rose_vars(srcdir=tmp_path)

diff --git 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."""
 
 import pytest
+from types import SimpleNamespace
 
 from metomi.isodatetime.datetimeoper import DateTimeOperator
 from metomi.rose import __version__ as ROSE_VERSION
@@ -28,6 +29,7 @@
     ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING,
     get_rose_vars_from_config_node,
     add_cylc_install_to_rose_conf_node_opts,
+    deprecation_warnings,
     dump_rose_log,
     identify_templating_section,
     MultipleTemplatingEnginesError
@@ -285,3 +287,44 @@ def test_ROSE_ORIG_HOST_replacement_behaviour(
             ROSE_ORIG_HOST_INSTALLED_OVERRIDE_STRING)
         assert not caplog.records
         assert node['env']['ROSE_ORIG_HOST'].value == 'IMPLAUSIBLE_HOST_NAME'
+
+
+@pytest.mark.parametrize(
+    'compat_mode, must_include, must_exclude',
+    (
+        (True, None, 'Use [template variables]'),
+        (True, 'root-dir', None),
+        (False, 'Use [template variables]', None),
+        (False, 'root-dir', None),
+    )
+)
+def test_deprecation_warnings(
+    caplog, monkeypatch, compat_mode, must_include, must_exclude
+):
+    """Method logs warnings correctly.
+
+    Two node items are set:
+
+    * ``jinja2:suite.rc`` should not cause a warning in compatibility mode.
+    * ``root-dir=/somewhere`` should always lead to a warning being logged.
+
+    Error messages about
+    """
+    # Create a node to pass to the method
+    # (It's not a tree test because we can use a simpleNamespace in place of
+    # a tree object):
+    node = ConfigNode()
+    node.set(['jinja2:suite.rc'])
+    node.set(['root-dir', '~foo'])
+    tree = SimpleNamespace(node=node)
+
+    # Patch compatibility mode flag and run the function under test:
+    monkeypatch.setattr('cylc.rose.utilities.cylc7_back_compat', compat_mode)
+    deprecation_warnings(tree)
+
+    # Check that warnings have/not been logged:
+    records = '\n'.join([i.message for i in caplog.records])
+    if must_include:
+        assert must_include in records
+    else:
+        assert must_exclude not in records