Skip to content

Commit

Permalink
Adds more support for interpretting multi-line strings
Browse files Browse the repository at this point in the history 
- Multiline strings are now interpreted correctly in `get_value()` and
  `render_to_object()`
- Multiline strings containing JINJA substitutions will now correctly
  subsitute those values
- Adds more unit tests, including some missing coverage of edge cases
  • Loading branch information
@schuylermartin45
schuylermartin45 committed Feb 6, 2024
1 parent 4f30a39 commit 53fe417
Show file tree
Hide file tree
Showing 4 changed files with 230 additions and 126 deletions.
20 changes: 19 additions & 1 deletion percy/parser/_utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from typing import cast

from percy.parser._types import PERCY_SUB_MARKER, ROOT_NODE_VALUE, Regex, StrStack, StrStackImmutable
from percy.parser.types import MultilineVariant, NodeValue
from percy.parser.types import TAB_AS_SPACES, MultilineVariant, NodeValue
from percy.types import H, SentinelType


Expand Down Expand Up @@ -121,6 +121,24 @@ def stringify_yaml(
return val


def normalize_multiline_strings(val: NodeValue, variant: MultilineVariant) -> NodeValue:
"""
Utility function that takes in a Node's value and "normalizes" multiline strings so that they can be accurately
interpreted by PyYaml. We use PyYaml to handle the various ways in which a multiline string can be interpreted.
:param val: Value to normalize
:param variant: Multiline variant rules to follow
:returns: If the value is a multiline string, this returns the "normalized" string to be re-evaluated by PyYaml.
Otherwise, returns the original value.
"""
if variant == MultilineVariant.NONE:
return val

# Prepend the multiline marker to the string to have PyYaml interpret how the whitespace should be handled. JINJA
# substitutions in multi-line strings do not break the PyYaml parser.
multiline_str = f"\n{TAB_AS_SPACES}".join(cast(list[str], val))
return f"{variant}\n{TAB_AS_SPACES}{multiline_str}"


def dedupe_and_preserve_order(l: list[H]) -> list[H]:
"""
Takes a list of strings
Expand Down
33 changes: 18 additions & 15 deletions percy/parser/recipe_parser.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
from percy.parser._types import PERCY_SUB_MARKER, ROOT_NODE_VALUE, ForceIndentDumper, Regex, StrStack
from percy.parser._utils import (
dedupe_and_preserve_order,
normalize_multiline_strings,
num_tab_spaces,
stack_path_to_str,
str_to_stack_path,
Expand Down Expand Up @@ -223,10 +224,9 @@ def _render_jinja_vars(self, s: str) -> JsonType:
"""
Helper function that replaces Jinja substitutions with their actual set values.
:param s: String to be re-rendered
:returns: The original value, augmented with Jinja substitutions. If substitutions have taken place, the type is
re-evaluated.
:returns: The original value, augmented with Jinja substitutions. Types are re-rendered to account for multiline
strings that may have been "normalized" prior to this call.
"""
replacement = False
# Search the string, replacing all substitutions we can recognize
for match in cast(list[str], Regex.JINJA_SUB.findall(s)):
lower_case = False
Expand All @@ -245,10 +245,7 @@ def _render_jinja_vars(self, s: str) -> JsonType:
if lower_case:
value = value.lower()
s = s.replace(match, value)
replacement = True
if replacement:
return cast(JsonType, yaml.safe_load(s))
return s
return cast(JsonType, yaml.safe_load(s))

def _rebuild_selectors(self) -> None:
"""
Expand Down Expand Up @@ -562,10 +559,13 @@ def _render_object_tree(self, node: Node, replace_variables: bool, data: JsonTyp
if child.is_comment():
continue

# Handle multiline strings
value = child.value if child.multiline_variant == MultilineVariant.NONE else "\n".join(child.value)
if replace_variables and isinstance(value, str):
value = self._render_jinja_vars(value)
# Handle multiline strings and variable replacement
value = normalize_multiline_strings(child.value, child.multiline_variant)
if isinstance(value, str):
if replace_variables:
value = self._render_jinja_vars(value)
elif child.multiline_variant != MultilineVariant.NONE:
value = cast(str, yaml.safe_load(value))

# Empty keys are interpreted to point to `None`
if child.is_empty_key():
Expand Down Expand Up @@ -805,12 +805,15 @@ def get_value(self, path: str, default: JsonType | SentinelType = _sentinel, sub
if node.is_single_key() and not node.is_root():
# As of writing, Jinja substitutions are not used
if node.children[0].multiline_variant != MultilineVariant.NONE:
# PyYaml will not preserve newlines passed into strings, so we can directly check for variable
# substitutions on a multiline string
multiline_str = "\n".join(cast(str, node.children[0].value))
multiline_str = cast(
str,
normalize_multiline_strings(
cast(list[str], node.children[0].value), node.children[0].multiline_variant
),
)
if sub_vars:
return self._render_jinja_vars(multiline_str)
return multiline_str
return cast(JsonType, yaml.safe_load(multiline_str))
return_value = cast(Primitives, node.children[0].value)
# Leaf nodes can return their value directly
elif node.is_leaf():
Expand Down
231 changes: 166 additions & 65 deletions percy/tests/parser/test_recipe_parser.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,21 @@
str
] = "This is a PEP '561 type stub package for the toml package.\nIt can be used by type-checking tools like mypy, pyright,\npytype, PyCharm, etc. to check code that uses toml." # pylint: disable=line-too-long

# Multiline string used to validate interpretation of the various multiline variations YAML allows
QUICK_FOX_PIPE: Final[str] = "The quick brown\n{{fox}}\n\njumped over the lazy dog\n"
QUICK_FOX_PIPE_PLUS: Final[str] = "The quick brown\n{{fox}}\n\njumped over the lazy dog\n"
QUICK_FOX_PIPE_MINUS: Final[str] = "The quick brown\n{{fox}}\n\njumped over the lazy dog"
QUICK_FOX_CARROT: Final[str] = "The quick brown {{fox}}\njumped over the lazy dog\n"
QUICK_FOX_CARROT_PLUS: Final[str] = "The quick brown {{fox}}\njumped over the lazy dog\n"
QUICK_FOX_CARROT_MINUS: Final[str] = "The quick brown {{fox}}\njumped over the lazy dog"
# Substitution variants of the multiline string
QUICK_FOX_SUB_PIPE: Final[str] = "The quick brown\ntiger\n\njumped over the lazy dog\n"
QUICK_FOX_SUB_PIPE_PLUS: Final[str] = "The quick brown\ntiger\n\njumped over the lazy dog\n"
QUICK_FOX_SUB_PIPE_MINUS: Final[str] = "The quick brown\ntiger\n\njumped over the lazy dog"
QUICK_FOX_SUB_CARROT: Final[str] = "The quick brown tiger\njumped over the lazy dog\n"
QUICK_FOX_SUB_CARROT_PLUS: Final[str] = "The quick brown tiger\njumped over the lazy dog\n"
QUICK_FOX_SUB_CARROT_MINUS: Final[str] = "The quick brown tiger\njumped over the lazy dog"


def load_file(file: Path | str) -> str:
"""
Expand Down Expand Up @@ -120,74 +135,146 @@ def test_round_trip(file: str) -> None:
assert parser.render() == expected


def test_render_to_object() -> None:
@pytest.mark.parametrize(
"file,substitute,expected",
[
(
"simple-recipe.yaml",
False,
{
"about": {
"description": SIMPLE_DESCRIPTION,
"license": "Apache-2.0 AND MIT",
"summary": "This is a small recipe for testing",
},
"test_var_usage": {
"foo": "{{ version }}",
"bar": [
"baz",
"{{ zz_non_alpha_first }}",
"blah",
"This {{ name }} is silly",
"last",
],
},
"build": {"is_true": True, "skip": True, "number": 0},
"package": {"name": "{{ name|lower }}"},
"requirements": {
"empty_field1": None,
"host": ["setuptools", "fakereq"],
"empty_field2": None,
"run": ["python"],
"empty_field3": None,
},
"multi_level": {
"list_3": ["ls", "sl", "cowsay"],
"list_2": ["cat", "bat", "mat"],
"list_1": ["foo", "bar"],
},
},
),
(
"simple-recipe.yaml",
True,
{
"about": {
"description": SIMPLE_DESCRIPTION,
"license": "Apache-2.0 AND MIT",
"summary": "This is a small recipe for testing",
},
"test_var_usage": {
"foo": "0.10.8.6",
"bar": [
"baz",
42,
"blah",
"This types-toml is silly",
"last",
],
},
"build": {"is_true": True, "skip": True, "number": 0},
"package": {"name": "types-toml"},
"requirements": {
"empty_field1": None,
"host": ["setuptools", "fakereq"],
"empty_field2": None,
"run": ["python"],
"empty_field3": None,
},
"multi_level": {
"list_3": ["ls", "sl", "cowsay"],
"list_2": ["cat", "bat", "mat"],
"list_1": ["foo", "bar"],
},
},
),
(
"simple-recipe_multiline_strings.yaml",
False,
{
"about": {
"description0": QUICK_FOX_PIPE,
"description1": QUICK_FOX_PIPE_PLUS,
"description2": QUICK_FOX_PIPE_MINUS,
"description3": QUICK_FOX_CARROT,
"description4": QUICK_FOX_CARROT_PLUS,
"description5": QUICK_FOX_CARROT_MINUS,
"license": "Apache-2.0 AND MIT",
"summary": "This is a small recipe for testing",
},
"test_var_usage": {
"foo": "{{ version }}",
"bar": [
"baz",
"{{ zz_non_alpha_first }}",
"blah",
"This {{ name }} is silly",
"last",
],
},
"build": {"is_true": True, "skip": True, "number": 0},
"package": {"name": "{{ name|lower }}"},
},
),
(
"simple-recipe_multiline_strings.yaml",
True,
{
"about": {
"description0": QUICK_FOX_SUB_PIPE,
"description1": QUICK_FOX_SUB_PIPE_PLUS,
"description2": QUICK_FOX_SUB_PIPE_MINUS,
"description3": QUICK_FOX_SUB_CARROT,
"description4": QUICK_FOX_SUB_CARROT_PLUS,
"description5": QUICK_FOX_SUB_CARROT_MINUS,
"license": "Apache-2.0 AND MIT",
"summary": "This is a small recipe for testing",
},
"test_var_usage": {
"foo": "0.10.8.6",
"bar": [
"baz",
42,
"blah",
"This types-toml is silly",
"last",
],
},
"build": {"is_true": True, "skip": True, "number": 0},
"package": {"name": "types-toml"},
},
),
],
)
def test_render_to_object(file: str, substitute: bool, expected: JsonType) -> None:
"""
Tests rendering a recipe to an object format.
:param file: File to load and test against
:param substitute: True to run the function with JINJA substitutions on, False for off
:param expected: Expected value to return
"""
parser = load_recipe("simple-recipe.yaml")
assert parser.render_to_object() == {
"about": {
"description": SIMPLE_DESCRIPTION,
"license": "Apache-2.0 AND MIT",
"summary": "This is a small recipe for testing",
},
"test_var_usage": {
"foo": "{{ version }}",
"bar": [
"baz",
"{{ zz_non_alpha_first }}",
"blah",
"This {{ name }} is silly",
"last",
],
},
"build": {"is_true": True, "skip": True, "number": 0},
"package": {"name": "{{ name|lower }}"},
"requirements": {
"empty_field1": None,
"host": ["setuptools", "fakereq"],
"empty_field2": None,
"run": ["python"],
"empty_field3": None,
},
"multi_level": {
"list_3": ["ls", "sl", "cowsay"],
"list_2": ["cat", "bat", "mat"],
"list_1": ["foo", "bar"],
},
}
# Tests variable substitution mode
assert parser.render_to_object(True) == {
"about": {
"description": SIMPLE_DESCRIPTION,
"license": "Apache-2.0 AND MIT",
"summary": "This is a small recipe for testing",
},
"test_var_usage": {
"foo": "0.10.8.6",
"bar": [
"baz",
42,
"blah",
"This types-toml is silly",
"last",
],
},
"build": {"is_true": True, "skip": True, "number": 0},
"package": {"name": "types-toml"},
"requirements": {
"empty_field1": None,
"host": ["setuptools", "fakereq"],
"empty_field2": None,
"run": ["python"],
"empty_field3": None,
},
"multi_level": {
"list_3": ["ls", "sl", "cowsay"],
"list_2": ["cat", "bat", "mat"],
"list_1": ["foo", "bar"],
},
}
parser = load_recipe(file)
assert parser.render_to_object(substitute) == expected


def test_render_to_object_multi_output() -> None:
Expand Down Expand Up @@ -358,6 +445,20 @@ def test_contains_value(file: str, path: str, expected: bool) -> None:
# Return a multiline string
("simple-recipe.yaml", "/about/description", False, SIMPLE_DESCRIPTION),
("simple-recipe.yaml", "/about/description/", False, SIMPLE_DESCRIPTION),
# Return multiline string variants
("simple-recipe_multiline_strings.yaml", "/about/description0", False, QUICK_FOX_PIPE),
("simple-recipe_multiline_strings.yaml", "/about/description1", False, QUICK_FOX_PIPE_PLUS),
("simple-recipe_multiline_strings.yaml", "/about/description2", False, QUICK_FOX_PIPE_MINUS),
("simple-recipe_multiline_strings.yaml", "/about/description3", False, QUICK_FOX_CARROT),
("simple-recipe_multiline_strings.yaml", "/about/description4", False, QUICK_FOX_CARROT_PLUS),
("simple-recipe_multiline_strings.yaml", "/about/description5", False, QUICK_FOX_CARROT_MINUS),
# Return multiline string variants, with substitution
("simple-recipe_multiline_strings.yaml", "/about/description0", True, QUICK_FOX_SUB_PIPE),
("simple-recipe_multiline_strings.yaml", "/about/description1", True, QUICK_FOX_SUB_PIPE_PLUS),
("simple-recipe_multiline_strings.yaml", "/about/description2", True, QUICK_FOX_SUB_PIPE_MINUS),
("simple-recipe_multiline_strings.yaml", "/about/description3", True, QUICK_FOX_SUB_CARROT),
("simple-recipe_multiline_strings.yaml", "/about/description4", True, QUICK_FOX_SUB_CARROT_PLUS),
("simple-recipe_multiline_strings.yaml", "/about/description5", True, QUICK_FOX_SUB_CARROT_MINUS),
# Comments in lists could throw-off array indexing
("simple-recipe.yaml", "/multi_level/list_1/1", False, "bar"),
# Render a recursive, complex type.
Expand Down
Loading

0 comments on commit 53fe417

Please sign in to comment.