Adds messaging system, filters unsupported JINJA types

- Adds a messaging system to the conversion utility to track errors and
  warnings
  - This leaves it up to the calling program to handle logging
- Unsupported JINJA types are now filtered-out out in the new file
  - This uncovered a bug in which the variable-table order was not being
    preserved

percy/parser/recipe_parser.py

@@ -37,7 +37,7 @@
 )
 from percy.parser.enums import SelectorConflictMode
 from percy.parser.exceptions import JsonPatchValidationException
-from percy.parser.types import JSON_PATCH_SCHEMA, TAB_AS_SPACES, TAB_SPACE_COUNT
+from percy.parser.types import JSON_PATCH_SCHEMA, TAB_AS_SPACES, TAB_SPACE_COUNT, MessageCategory, MessageTable
 from percy.types import PRIMITIVES_TUPLE, JsonPatchType, JsonType, Primitives, SentinelType
 
 
@@ -592,7 +592,7 @@ def render_to_object(self, replace_variables: bool = False) -> JsonType:
 
         return data
 
-    def render_to_new_recipe_format(self) -> str:
+    def render_to_new_recipe_format(self) -> tuple[str, MessageTable]:
         # pylint: disable=protected-access
         """
         Takes the current recipe representation and renders it to the new format WITHOUT modifying the current recipe
@@ -607,16 +607,22 @@ def render_to_new_recipe_format(self) -> str:
         # Approach: In the event that we want to expand support later, this function should be implemented in terms
         # of a `RecipeParser` tree. This will make it easier to build an upgrade-path, if we so choose to pursue one.
 
+        msg_tbl = MessageTable()
+
         # `copy.deepcopy()` produced some bizarre artifacts, namely single-line comments were being incorrectly rendered
         # as list members. Although inefficient, we have tests that validate round-tripping the parser and there
         # is no development cost in utilizing tools we already must maintain.
         new_recipe: RecipeParser = RecipeParser(self.render())
 
         # Convert the JINJA variable table to a `context` section. Empty tables still add the `context` section for
         # future developers' convenience.
-        new_recipe.patch(
-            {"op": "add", "path": "/context", "value": new_recipe._vars_tbl if new_recipe._vars_tbl else None}
-        )
+        new_recipe.patch({"op": "add", "path": "/context", "value": None})
+        # Filter-out any value not covered in the new format
+        for name, value in new_recipe._vars_tbl.items():
+            if not isinstance(value, (str, int, float, bool)):
+                msg_tbl.add_message(MessageCategory.WARNING, f"The variable `{name}` is an unsupported type.")
+                continue
+            new_recipe.patch({"op": "add", "path": f"/context/{name}", "value": value})
 
         # Hack: `add` has no concept of ordering and new fields are appended to the end. Logically, `context` should be
         # at the top of the file, so we'll force it to the front of root's child list.
@@ -633,6 +639,9 @@ def render_to_new_recipe_format(self) -> str:
             value = new_recipe.get_value(path)
             # Values that match the regex should only be strings. This prevents crashes that should not occur.
             if not isinstance(value, str):
+                msg_tbl.add_message(
+                    MessageCategory.WARNING, f"A non-string value was found as a JINJA substitution: {value}"
+                )
                 continue
             value = value.replace("{{", "${{")
             # TODO Fix: Inclusion of 's in `value` breaks `patch()`, breaking the `multi-output.yaml` test
@@ -681,7 +690,7 @@ def render_to_new_recipe_format(self) -> str:
         # Hack: Wipe the existing table so the JINJA `set` statements don't render the final form
         new_recipe._vars_tbl = {}
 
-        return new_recipe.render()
+        return new_recipe.render(), msg_tbl
 
     ## YAML Access Functions ##
 

percy/parser/types.py

@@ -4,6 +4,7 @@
 """
 from __future__ import annotations
 
+from enum import StrEnum, auto
 from typing import Final
 
 from percy.types import Primitives, SchemaType
@@ -88,3 +89,69 @@
     ],
     "additionalProperties": False,
 }
+
+
+class MessageCategory(StrEnum):
+    """
+    Categories to classify `RecipeParser` messages into.
+    """
+
+    ERROR = auto()
+    WARNING = auto()
+
+
+class MessageTable:
+    """
+    Stores and tags messages that may come up during `RecipeParser` operations. It is up to the client program to
+    handle the logging of these messages.
+    """
+
+    def __init__(self):
+        """
+        Constructs an empty message table
+        """
+        self._tbl: dict[MessageCategory, list[str]] = {}
+
+    def add_message(self, category: MessageCategory, message: str) -> None:
+        """
+        Adds a message to the table
+        :param category:
+        :param message:
+        """
+        if category not in self._tbl:
+            self._tbl[category] = []
+        self._tbl[category].append(message)
+
+    def get_messages(self, category: MessageCategory) -> list[str]:
+        """
+        Returns all the messages stored in a given category
+        :param category: Category to target
+        :returns: A list containing all the messages stored in a category.
+        """
+        if category not in self._tbl:
+            return []
+        return self._tbl[category]
+
+    def get_totals_message(self) -> str:
+        """
+        Convenience function that returns a displayable count of the number of warnings and errors contained in the
+        messaging object.
+        :returns: A message indicating the number of errors and warnings that have been accumulated. If there are none,
+                  an empty string is returned.
+        """
+        if not self._tbl:
+            return ""
+
+        def _pluralize(n: int, s: str) -> str:
+            if n == 1:
+                return s
+            return f"{s}s"
+
+        num_errors: Final[int] = 0 if MessageCategory.ERROR not in self._tbl else len(self._tbl[MessageCategory.ERROR])
+        errors: Final[str] = f"{num_errors} {_pluralize(num_errors, 'error')}"
+        num_warnings: Final[int] = (
+            0 if MessageCategory.WARNING not in self._tbl else len(self._tbl[MessageCategory.WARNING])
+        )
+        warnings: Final[str] = f"{num_warnings} {_pluralize(num_warnings, 'warning')}"
+
+        return f"{errors} and {warnings} were found."

percy/tests/parser/test_recipe_parser.py

@@ -259,7 +259,9 @@ def test_render_to_new_recipe_format(file_base: str) -> None:
     TODO: re-enable the "multi-output.yaml" test when the single-quoted strings bug is resolved.
     """
     parser = load_recipe(file_base)
-    assert parser.render_to_new_recipe_format() == load_file(f"{TEST_FILES_PATH}/new_format_{file_base}")
+    # TODO: validate messages returned
+    result, _ = parser.render_to_new_recipe_format()
+    assert result == load_file(f"{TEST_FILES_PATH}/new_format_{file_base}")
     # Ensure that the original file was untouched
     assert not parser.is_modified()
     assert parser.diff() == ""

percy/tests/test_aux_files/new_format_simple-recipe.yaml

@@ -1,9 +1,9 @@
 schema_version: 1
 
 context:
+  zz_non_alpha_first: 42
   name: types-toml
   version: 0.10.8.6
-  zz_non_alpha_first: 42
 
 package:
   name: ${{ name|lower }}