diff --git a/percy/parser/recipe_parser.py b/percy/parser/recipe_parser.py index 8382f36..981466d 100644 --- a/percy/parser/recipe_parser.py +++ b/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,6 +607,8 @@ 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. @@ -614,9 +616,13 @@ def render_to_new_recipe_format(self) -> str: # 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 ## diff --git a/percy/parser/types.py b/percy/parser/types.py index e8d28b7..9279b43 100644 --- a/percy/parser/types.py +++ b/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." diff --git a/percy/tests/parser/test_recipe_parser.py b/percy/tests/parser/test_recipe_parser.py index 09f8914..d1e9c9c 100644 --- a/percy/tests/parser/test_recipe_parser.py +++ b/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() == "" diff --git a/percy/tests/test_aux_files/new_format_simple-recipe.yaml b/percy/tests/test_aux_files/new_format_simple-recipe.yaml index 071fe1e..1445d83 100644 --- a/percy/tests/test_aux_files/new_format_simple-recipe.yaml +++ b/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 }}