Fix bugs in cylc broadcast

changes.d/5933.fix.md

@@ -0,0 +1 @@
+Fixed bug in `cylc broadcast` (and the GUI Edit Runtime command) where everything after a `#` character in a setting would be stripped out.

cylc/flow/parsec/validate.py

@@ -373,7 +373,7 @@ def coerce_range(cls, value, keys):
         return Range((int(min_), int(max_)))
 
     @classmethod
-    def coerce_str(cls, value, keys):
+    def coerce_str(cls, value, keys) -> str:
         """Coerce value to a string.
 
         Examples:
@@ -385,7 +385,7 @@ def coerce_str(cls, value, keys):
         """
         if isinstance(value, list):
             # handle graph string merging
-            vraw = []
+            vraw: List[str] = []
             vals = [value]
             while vals:
                 val = vals.pop()
@@ -512,35 +512,36 @@ def parse_int_range(cls, value):
             return None
 
     @classmethod
-    def strip_and_unquote(cls, keys, value):
+    def strip_and_unquote(cls, keys: List[str], value: str) -> str:
         """Remove leading and trailing spaces and unquote value.
 
         Args:
-            keys (list):
+            keys:
                 Keys in nested dict that represents the raw configuration.
-            value (str):
+            value:
                 String value in raw configuration.
 
-        Return (str):
+        Return:
             Processed value.
 
         Examples:
             >>> ParsecValidator.strip_and_unquote(None, '" foo "')
             'foo'
 
         """
-        for substr, rec in [
-                ["'''", cls._REC_MULTI_LINE_SINGLE],
-                ['"""', cls._REC_MULTI_LINE_DOUBLE],
-                ['"', cls._REC_DQ_VALUE],
-                ["'", cls._REC_SQ_VALUE]]:
+        for substr, rec in (
+            ("'''", cls._REC_MULTI_LINE_SINGLE),
+            ('"""', cls._REC_MULTI_LINE_DOUBLE),
+            ('"', cls._REC_DQ_VALUE),
+            ("'", cls._REC_SQ_VALUE)
+        ):
             if value.startswith(substr):
                 match = rec.match(value)
                 if not match:
                     raise IllegalValueError("string", keys, value)
                 value = match.groups()[0]
                 break
-        else:
+        else:  # no break
             # unquoted
             value = value.split(r'#', 1)[0]
 
@@ -1136,23 +1137,25 @@ def _coerce_type(cls, value):
         return val
 
 
-# BACK COMPAT: BroadcastConfigValidator
-# The DB at 8.0.x stores Interval values as neither ISO8601 duration
-# string or DurationFloat. This has been fixed at 8.1.0, and
-# the following class acts as a bridge between fixed and broken.
-# url:
-#     https://github.com/cylc/cylc-flow/pull/5138
-# from:
-#    8.0.x
-# to:
-#    8.1.x
-# remove at:
-#    8.x
 class BroadcastConfigValidator(CylcConfigValidator):
     """Validate and Coerce DB loaded broadcast config to internal objects."""
     def __init__(self):
         CylcConfigValidator.__init__(self)
 
+    @classmethod
+    def coerce_str(cls, value, keys) -> str:
+        """Coerce value to a string.
+
+        Examples:
+            >>> BroadcastConfigValidator.coerce_str('abc#def', None)
+            'abc#def'
+        """
+        # Prevent ParsecValidator from assuming '#' means comments;
+        # '#' has valid uses in shell script such as parameter substitution
+        if isinstance(value, str) and '#' in value:
+            value = f'"{value}"'
+        return ParsecValidator.coerce_str(value, keys)
+
     @classmethod
     def strip_and_unquote_list(cls, keys, value):
         """Remove leading and trailing spaces and unquote list value.
@@ -1177,6 +1180,18 @@ def strip_and_unquote_list(cls, keys, value):
             value = value.lstrip('[').rstrip(']')
         return ParsecValidator.strip_and_unquote_list(keys, value)
 
+    # BACK COMPAT: BroadcastConfigValidator.coerce_interval
+    # The DB at 8.0.x stores Interval values as neither ISO8601 duration
+    # string or DurationFloat. This has been fixed at 8.1.0, and
+    # the following class acts as a bridge between fixed and broken.
+    # url:
+    #     https://github.com/cylc/cylc-flow/pull/5138
+    # from:
+    #    8.0.x
+    # to:
+    #    8.1.x
+    # remove at:
+    #    8.x
     @classmethod
     def coerce_interval(cls, value, keys):
         """Coerce an ISO 8601 interval into seconds.

cylc/flow/scripts/broadcast.py

@@ -82,6 +82,7 @@
 from ansimarkup import parse as cparse
 from copy import deepcopy
 from functools import partial
+import os.path
 import re
 import sys
 from tempfile import NamedTemporaryFile
@@ -203,7 +204,7 @@ def files_to_settings(settings, setting_files, cancel_mode=False):
                 handle.seek(0, 0)
                 cfg.loadcfg(handle.name)
         else:
-            cfg.loadcfg(setting_file)
+            cfg.loadcfg(os.path.abspath(setting_file))
     stack = [([], cfg.get(sparse=True))]
     while stack:
         keys, item = stack.pop()

tests/functional/broadcast/10-file-1/broadcast.cylc

@@ -1,6 +1,8 @@
 script="""
 printenv CYLC_FOOBAR
 
+# (This hash char should not cause the rest of the script to be stripped out)
+
 if (($CYLC_TASK_TRY_NUMBER < 2 )); then
     false
 fi

tests/unit/parsec/test_validate.py

@@ -456,11 +456,9 @@ def test_coerce_int_list():
             validator.coerce_int_list(value, ['whatever'])
 
 
-def test_coerce_str():
-    """Test coerce_str."""
-    validator = ParsecValidator()
-    # The good
-    for value, result in [
+@pytest.mark.parametrize(
+    'value, expected',
+    [
         ('', ''),
         ('Hello World!', 'Hello World!'),
         ('"Hello World!"', 'Hello World!'),
@@ -474,9 +472,15 @@ def test_coerce_str():
          'Hello:\n    foo\nGreet\n    baz'),
         ('False', 'False'),
         ('None', 'None'),
-        (['a', 'b'], 'a\nb')
-    ]:
-        assert validator.coerce_str(value, ['whatever']) == result
+        (['a', 'b'], 'a\nb'),
+        ('abc#def', 'abc'),
+    ]
+)
+def test_coerce_str(value: str, expected: str):
+    """Test coerce_str."""
+    validator = ParsecValidator()
+    # The good
+    assert validator.coerce_str(value, ['whatever']) == expected
 
 
 def test_coerce_str_list():