response to review

cylc/flow/option_parsers.py

@@ -424,8 +424,7 @@ class CylcOptionParser(OptionParser):
                 'Set the value of a Jinja2 template variable in the'
                 ' workflow definition as a comma separated'
                 ' list of Python strings.'
-                ' Values containing commas must be quoted, but this is'
-                ' not otherwise necessary.'
+                ' Values containing commas must be quoted.'
                 " e.g. '+s STR=a,b,c' => ['a', 'b', 'c']"
                 " or '+ s STR=a,\"b,c\"' => ['a', 'b,c']"
                 + CAN_BE_USED_MULTIPLE

cylc/flow/templatevars.py

@@ -17,7 +17,7 @@
 
 from ast import literal_eval
 from optparse import Values
-from typing import Any, Dict, List, Optional, Union
+from typing import Any, Dict, List, Optional, Set, Union
 
 from cylc.flow import LOG
 from cylc.flow.exceptions import InputError
@@ -29,8 +29,8 @@
 
 
 OVERWRITE_WARNING = (
-    'Template variable {} defined redefined:'
-    ' previous definitions will be overwritten.'
+    'Template variable {} redefined:'
+    ' the previous value will be overwritten.'
 )
 
 
@@ -84,7 +84,7 @@ def eval_var(var):
 
 
 def parse_string_list(stringlist: str) -> List:
-    """Parse a string comma separated list into a python list of strings.
+    """Parse a comma separated string list into a Python string list.
 
     Examples:
         >>> parse_string_list('a,b,c')
@@ -115,11 +115,11 @@ def load_template_vars(
     templatevars_lists: Optional[List[str]] = None
 ) -> Dict[str, Any]:
     """Load template variables from key=value strings."""
-    keys: set = set()
+    keys: Set[str] = set()
     invalid_vars: str = ''
 
     # Parse Template vars set by file (-S)
-    file_tvars: Dict = {}
+    file_tvars: Dict[str, Any] = {}
     if template_vars_file:
         with open(template_vars_file, 'r') as handle:
             for line in handle:
@@ -134,8 +134,8 @@ def load_template_vars(
                 file_tvars[key.strip()] = eval_var(val.strip())
                 keys.add(key.strip())
 
-    cli_tvars: Dict = {}
-    tvars_lists: Dict = {}
+    cli_tvars: Dict[str, Any] = {}
+    tvars_lists: Dict[str, str] = {}
     for input_, result, func in (
         (template_vars, cli_tvars, eval_var),
         (templatevars_lists, tvars_lists, parse_string_list)
@@ -156,8 +156,7 @@ def load_template_vars(
     # Raise an error if there are any args without the form key=value.:
     if invalid_vars:
         raise InputError(
-            'Template variables should be in the form key=value(s) -'
-            '\nThe following arguments did not validate:'
+            'Template variables must be set with key=value(s):'
             + invalid_vars
         )
 
@@ -176,8 +175,7 @@ def load_template_vars(
     # Raise an error if there are any key-clashes between tvars and tvars_list
     if badkeys:
         raise InputError(
-            'Setting the same variable using -s and -z is not permitted -'
-            '\nThe following clashing template variables:\n * '
+            "You can't set the same template variable with both -s and -z:"
             + badkeys
         )
 

tests/unit/test_loggingutil.py

@@ -34,7 +34,7 @@
     set_timestamps,
 )
 
-@pytest.mark.skip
+
 @mock.patch("cylc.flow.loggingutil.glbl_cfg")
 def test_value_error_raises_system_exit(
     mocked_glbl_cfg,