From 58ee53d852da0be86c73edab9cfeea89564c8f9a Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Tue, 16 Jul 2024 18:05:42 +0100
Subject: [PATCH] Replace "template variables" with "template_variables"
 (#2783)

Replace "template variables" with "template_variables"
before searching for check rules with the rose mini-language.
Replacing the "template variables" string after the search.
---
 .coveragerc                                   |  7 +-
 metomi/rose/macros/rule.py                    | 32 ++++++-
 metomi/rose/tests/macros/test_rule.py         | 91 +++++++++++++++++++
 t/rose-metadata-check/12-template-variables.t | 80 ++++++++++++++++
 4 files changed, 201 insertions(+), 9 deletions(-)
 create mode 100644 metomi/rose/tests/macros/test_rule.py
 create mode 100644 t/rose-metadata-check/12-template-variables.t

diff --git a/.coveragerc b/.coveragerc
index 9f5d651e53..260dde13ee 100644
--- a/.coveragerc
+++ b/.coveragerc
@@ -25,7 +25,7 @@
 branch=True
 cover_pylib=False
 concurrency=multiprocessing
-data_file=${TRAVIS_BUILD_DIR}/.coverage
+# data_file=.coverage
 disable_warnings=
     trace-changed
     module-not-python
@@ -45,12 +45,11 @@ debug=
 #    sys
 #    trace
 # Include can be used only if source is not used!
-note=
 parallel = True
 plugins=
 include=
-    *lib/python/rose*
-    *lib/python/rosie*
+    ./metomi/rose/*
+    ./metomi/rosie/*
 timid = False
 
 
diff --git a/metomi/rose/macros/rule.py b/metomi/rose/macros/rule.py
index 0385aecaaa..3d80117a21 100644
--- a/metomi/rose/macros/rule.py
+++ b/metomi/rose/macros/rule.py
@@ -18,6 +18,8 @@
 import ast
 import re
 
+from functools import partial
+
 import jinja2
 import jinja2.exceptions
 
@@ -370,11 +372,8 @@ def _process_rule(
         if log_ids is None:
             get_value_from_id = self._get_value_from_id
         else:
-            get_value_from_id = (
-                lambda id_, conf, m_conf, p_id: self._log_id_usage(
-                    id_, conf, m_conf, p_id, log_ids
-                )
-            )
+            get_value_from_id = partial(self._log_id_usage, id_set=log_ids)
+
         if not (rule.startswith('{%') or rule.startswith('{-%')):
             rule = "{% if " + rule + " %}True{% else %}False{% endif %}"
 
@@ -388,8 +387,16 @@ def _process_rule(
         sci_num_count = -1
 
         # any/all processing.
+        # n.b. Because we've made "template variables" (with space) the
+        # section header at Rose 2 we need to hack the rose mini language
+        # to allow a space in this one case:
+        rule = rule.replace('template variables', 'template_variables')
         for array_func_key, rec_regex in self.REC_ARRAY.items():
             for search_result in rec_regex.findall(rule):
+                search_result = (
+                    i.replace('template_variables', 'template variables')
+                    for i in search_result
+                )
                 start, var_id, operator, value, end = search_result
                 if var_id == "this":
                     var_id = setting_id
@@ -411,6 +418,10 @@ def _process_rule(
 
         # len(...) processing.
         for search_result in self.REC_LEN_FUNC.findall(rule):
+            search_result = (
+                i.replace('template_variables', 'template variables')
+                for i in search_result
+            )
             start, var_id, end = search_result
             if var_id == "this":
                 var_id = setting_id
@@ -455,11 +466,20 @@ def _process_rule(
                 x_id_num_str = search_result.replace("this", "").strip('()')
                 key = self.INTERNAL_ID_THIS_SETTING.format(x_id_num_str)
                 local_map[key] = value_string
+            search_result = search_result.replace(
+                'template variables', 'template_variables')
             rule = rule.replace(search_result, key, 1)
 
         # Replace ids (namelist:foo=bar) with their cast values.
         config_id_count = -1
+        # n.b. Because we've made "template variables" (with space) the
+        # section header at Rose 2 we need to hack the rose mini language
+        # to allow a space in this one case:
+        rule = rule.replace('template variables', 'template_variables')
+
         for search_result in self.REC_CONFIG_ID.findall(rule):
+            search_result = search_result.replace(
+                'template_variables', 'template variables')
             value_string = get_value_from_id(
                 search_result, config, meta_config, setting_id
             )
@@ -470,6 +490,8 @@ def _process_rule(
                 config_id_count += 1
                 key = self.INTERNAL_ID_SETTING.format(config_id_count)
                 local_map[key] = value_string
+            search_result = search_result.replace(
+                'template variables', 'template_variables')
             rule = rule.replace(search_result, key, 1)
 
         # Return the now valid Jinja2 template with a map of variables.
diff --git a/metomi/rose/tests/macros/test_rule.py b/metomi/rose/tests/macros/test_rule.py
new file mode 100644
index 0000000000..832253d45f
--- /dev/null
+++ b/metomi/rose/tests/macros/test_rule.py
@@ -0,0 +1,91 @@
+# Copyright (C) British Crown (Met Office) & Contributors.
+# This file is part of Rose, a framework for meteorological suites.
+#
+# Rose 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.
+#
+# Rose 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 Rose. If not, see <http://www.gnu.org/licenses/>.
+# -----------------------------------------------------------------------------
+
+"""Tests for rose macros rule module.
+"""
+
+import pytest
+from metomi.rose.macros.rule import RuleEvaluator
+
+
+param = pytest.param
+rule_evaluator = RuleEvaluator()
+
+
+@pytest.mark.parametrize(
+    'section',
+    (('template variables'), ('namelist'), ('empy:suite.rc')))
+@pytest.mark.parametrize(
+    'rule_in, id_in, rule_out, this, this_out', [
+        param(
+            '{section}=FOO == 42',
+            '{section}=FOO',
+            '{% if this == 42 %}True{% else %}False{% endif %}',
+            '42',
+            {'this': '42'},
+            id='basic_rule'
+        ),
+        param(
+            'all({section}=FOO == "42")',
+            '{section}=FOO',
+            '{% if (this == _value0 and this == _value0 and'
+            ' this == _value0) %}True{% else %}False{% endif %}',
+            '42,43,44',
+            {'this': '42,43,44', '_value0': '42'},
+            id='all_rule'
+        ),
+        param(
+            'len({section}=FOO) < 4',
+            '{section}=FOO',
+            '{% if 3 < 4 %}True{% else %}False{% endif %}',
+            '42,43,44',
+            {'this': '42,43,44'},
+            id='len_rule'
+        )
+    ]
+)
+def test__process_rule(rule_in, id_in, rule_out, this, this_out, section):
+    """Test processing of rules into jinja2.
+
+    Also provides tests for
+    https://github.com/metomi/rose/issues/2737
+    """
+    rule_in = rule_in.format(section=section)
+    id_in = id_in.format(section=section)
+    rule_evaluator._get_value_from_id = lambda *_: this
+    rule, map_ = rule_evaluator._process_rule(
+        rule_in, id_in, 'patchme', 'patchme')
+    assert rule == rule_out
+    assert map_ == this_out
+
+
+def test__process_rule_scientific_numbers():
+    """A string which looks like a scientific number comes out as
+    a scientific number.
+    """
+    this = "99"
+    rule = 'template variables=FOO=="9e9"'
+
+    rule_evaluator._get_value_from_id = lambda *_: this
+
+    _, map_ = rule_evaluator._process_rule(
+        rule, 'template variables=FOO', 'patchme', 'patchme')
+    assert map_ == {
+        'this': '99',
+        '_scinum0': 9000000000.0,
+        '_value0': '_scinum0'
+    }
diff --git a/t/rose-metadata-check/12-template-variables.t b/t/rose-metadata-check/12-template-variables.t
new file mode 100644
index 0000000000..b0a5cb8180
--- /dev/null
+++ b/t/rose-metadata-check/12-template-variables.t
@@ -0,0 +1,80 @@
+#!/usr/bin/env bash
+#-------------------------------------------------------------------------------
+# Copyright (C) British Crown (Met Office) & Contributors.
+#
+# This file is part of Rose, a framework for meteorological suites.
+#
+# Rose 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.
+#
+# Rose 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 Rose. If not, see <http://www.gnu.org/licenses/>.
+#-------------------------------------------------------------------------------
+# Test "rose metadata-check":
+# Check that hack allowing section name [template variables] works.
+# Follows example in https://github.com/metomi/rose/issues/2737
+
+. $(dirname $0)/test_header
+
+CHECKS=(
+    "basic@template variables=FOO == 2"
+    # Check we don't need to deal with loop: Number-like-strings into numbers.
+    "num string@template variables=FOO == \"2e1\""
+    # Check we don't need to deal with loop: Strings into proper string variables.
+    "str@template variables=FOO == \"Sir Topham Hat\""
+    "amy@any(template variables=FOO == 2)"
+    "len@len(template variables=FOO) > 42"
+    "in list@template variables=FOO in [3, 4, 8]"
+    "combined@len(template variables=FOO) > 42 and any(template variables=FOO == 2)"
+    )
+
+
+# 3 tests for each case:
+tests $(( ${#CHECKS[@]} * 5 ))
+
+setup
+
+mkdir -p ../meta
+
+# Allows array members to have spaces without splitting:
+for ((i = 0; i < ${#CHECKS[@]}; i++)); do
+    ID=$(echo "${CHECKS[$i]}" | awk -F '@' '{print $1}')
+    CHECK_=$(echo "${CHECKS[$i]}" | awk -F '@' '{print $2}')
+
+    TEST_KEY="$TEST_KEY_BASE::${ID}"
+    cat > ../meta/rose-meta.conf <<__META_CONFIG__
+[template variables=FOO]
+fail-if=${CHECK_}
+trigger=template variables=BAR: this == 1;
+
+[template variables=BAR]
+__META_CONFIG__
+
+    cat > ../rose-app.conf <<__ICI__
+[template variables]
+FOO=1
+!BAR=22
+__ICI__
+
+    run_pass "$TEST_KEY" rose metadata-check -C ../meta
+    file_cmp "$TEST_KEY.out" "$TEST_KEY.out" </dev/null
+    file_cmp "$TEST_KEY.err" "$TEST_KEY.err" </dev/null
+    # Also covers upgrade macros, because these subclass
+    # --transform macros:
+    run_pass \
+        "$TEST_KEY.macro --transform" \
+        rose macro --transform -y -C ../
+    run_pass \
+        "$TEST_KEY.macro --validate" \
+        rose macro --validate -C ../
+
+done
+
+teardown