Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show diff report in accessible colors and reveal hidden line characters #126

Merged
merged 17 commits into from
Mar 7, 2020
Merged
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ From v1.0.0 onwards, this project adheres to [Semantic Versioning](https://semve

## Master (Unreleased)

- Up to date with releases
- Fix bug where missing carriage return not shown in diff reports (#126)
- Account for accessibility and readability in snapshot outputs (#126)

## [v0.3.3](https://github.com/tophat/syrupy/compare/v0.3.2...v0.3.3)

Expand Down
12 changes: 8 additions & 4 deletions src/syrupy/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
from .location import TestLocation
from .session import SnapshotSession
from .terminal import (
green,
red,
received_style,
reset,
snapshot_style,
)
from .utils import import_module_member

Expand Down Expand Up @@ -64,10 +64,14 @@ def pytest_assertrepr_compare(op: str, left: Any, right: Any) -> Optional[List[s
https://docs.pytest.org/en/latest/reference.html#_pytest.hookspec.pytest_assertrepr_compare
"""
if isinstance(left, SnapshotAssertion):
assert_msg = reset(f"{green(left.name)} {op} {red('received')}")
assert_msg = reset(
f"{snapshot_style(left.name)} {op} {received_style('received')}"
)
return [assert_msg] + left.get_assert_diff(right)
elif isinstance(right, SnapshotAssertion):
assert_msg = reset(f"{red('received')} {op} {green(right.name)}")
assert_msg = reset(
f"{received_style('received')} {op} {snapshot_style(right.name)}"
)
return [assert_msg] + right.get_assert_diff(left)
return None

Expand Down
4 changes: 4 additions & 0 deletions src/syrupy/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,7 @@
SNAPSHOT_UNKNOWN_FOSSIL_KEY = "unknown snapshot fossil"

EXIT_STATUS_FAIL_UNUSED = 1

SYMBOL_ELLIPSIS = "..." # U+2026
SYMBOL_NEW_LINE = "␤" # U+2424
SYMBOL_CARRIAGE = "␍" # U+240D
26 changes: 26 additions & 0 deletions src/syrupy/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
TYPE_CHECKING,
Dict,
Iterator,
List,
Optional,
)

Expand Down Expand Up @@ -117,3 +118,28 @@ def __iter__(self) -> Iterator["SnapshotFossil"]:

def __contains__(self, key: str) -> bool:
return key in self._snapshot_fossils


@attr.s
class DiffedLine:
a: str = attr.ib(default=None)
b: str = attr.ib(default=None)
c: List[str] = attr.ib(factory=list)
diff_a: str = attr.ib(default="")
diff_b: str = attr.ib(default="")

@property
def has_snapshot(self) -> bool:
return self.a is not None

@property
def has_received(self) -> bool:
return self.b is not None

@property
def is_complete(self) -> bool:
return self.has_snapshot and self.has_received

@property
def is_context(self) -> bool:
return bool(self.c)
3 changes: 2 additions & 1 deletion src/syrupy/extensions/amber.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
Set,
)

from syrupy.constants import SYMBOL_ELLIPSIS
from syrupy.data import (
Snapshot,
SnapshotFossil,
Expand All @@ -28,7 +29,7 @@ class DataSerializer:

class MarkerDepthMax:
def __repr__(self) -> str:
return "..."
return SYMBOL_ELLIPSIS
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though we use the same symbol for context & max depth, semantically they're different and could theoretically change.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but then we could split the symbols apart at that point


@classmethod
def write_file(cls, snapshot_fossil: "SnapshotFossil") -> None:
Expand Down
173 changes: 143 additions & 30 deletions src/syrupy/extensions/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,36 @@
from typing import (
TYPE_CHECKING,
Callable,
Generator,
Dict,
Iterator,
List,
Optional,
Set,
Union,
)

from typing_extensions import final

from syrupy.constants import SNAPSHOT_DIRNAME
from syrupy.constants import (
SNAPSHOT_DIRNAME,
SYMBOL_CARRIAGE,
SYMBOL_ELLIPSIS,
SYMBOL_NEW_LINE,
)
from syrupy.data import (
DiffedLine,
Snapshot,
SnapshotEmptyFossil,
SnapshotFossil,
SnapshotFossils,
)
from syrupy.exceptions import SnapshotDoesNotExist
from syrupy.terminal import (
emphasize,
green,
mute,
red,
context_style,
received_diff_style,
received_style,
reset,
snapshot_diff_style,
snapshot_style,
)
from syrupy.utils import walk_snapshot_dir

Expand Down Expand Up @@ -214,36 +222,141 @@ def __ensure_snapshot_dir(self, *, index: int) -> None:
class SnapshotReporter(ABC):
def diff_lines(
self, serialized_data: "SerializedData", snapshot_data: "SerializedData"
) -> Generator[str, None, None]:
) -> Iterator[str]:
for line in self.__diff_lines(str(snapshot_data), str(serialized_data)):
yield reset(line)

def __diff_lines(self, a: str, b: str) -> Generator[str, None, None]:
line_styler = {"-": green, "+": red}
staged_line, skip = "", False
for line in ndiff(a.splitlines(), b.splitlines()):
if staged_line and line[:1] != "?":
yield line_styler[staged_line[:1]](staged_line)
staged_line, skip = "", False
if line[:1] in "-+":
staged_line = line
elif line[:1] == "?":
yield self.__diff_line(line, staged_line, line_styler[staged_line[:1]])
staged_line, skip = "", False
elif not skip:
yield mute(" ...")
skip = True
if staged_line:
yield line_styler[staged_line[:1]](staged_line)

def __diff_line(
self, marker_line: str, line: str, line_style: Callable[[Union[str, int]], str]
@property
def _ends(self) -> Dict[str, str]:
return {"\n": self._marker_new_line, "\r": self._marker_carriage}

@property
def _context_line_count(self) -> int:
return 1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous implementation is equivalent to setting this to 0


@property
def _context_line_max(self) -> int:
return self._context_line_count * 2

@property
def _marker_context_max(self) -> str:
return SYMBOL_ELLIPSIS

@property
def _marker_new_line(self) -> str:
return SYMBOL_NEW_LINE

@property
def _marker_carriage(self) -> str:
return SYMBOL_CARRIAGE

def __diff_lines(self, a: str, b: str) -> Iterator[str]:
for line in self.__diff_data(a, b):
show_ends = (
self.__strip_ends(line.a[1:]) == self.__strip_ends(line.b[1:])
if line.is_complete
else False
)
if line.has_snapshot:
yield self.__format_line(
line.a, line.diff_a, snapshot_style, snapshot_diff_style, show_ends
)
if line.has_received:
yield self.__format_line(
line.b, line.diff_b, received_style, received_diff_style, show_ends
)
yield from map(context_style, self.__limit_context(line.c))

def __diff_data(self, a: str, b: str) -> Iterator["DiffedLine"]:
staged_diffed_line: Optional["DiffedLine"] = None
for line in ndiff(a.splitlines(keepends=True), b.splitlines(keepends=True)):
is_context_line = line[0] == " "
is_snapshot_line = line[0] == "-"
is_received_line = line[0] == "+"
is_diff_line = line[0] == "?"

if is_context_line or is_diff_line:
line = self.__strip_ends(line)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was originally thinking of only showing the new line endings (i.e. the symbols) if necessary. So if the line endings are the only change.


if staged_diffed_line:
if is_diff_line:
if staged_diffed_line.has_received:
staged_diffed_line.diff_b = line
elif staged_diffed_line.has_snapshot:
staged_diffed_line.diff_a = line
# else: should never happen because then it would have
# encounted a diff line without any previously staged line
else:
should_unstage = (
staged_diffed_line.is_complete
or (staged_diffed_line.has_snapshot and is_snapshot_line)
or (staged_diffed_line.has_received and is_received_line)
or (staged_diffed_line.is_context and not is_context_line)
)
if should_unstage:
yield staged_diffed_line
staged_diffed_line = None
elif is_snapshot_line:
staged_diffed_line.a = line
elif is_received_line:
staged_diffed_line.b = line
elif is_context_line:
staged_diffed_line.c.append(line)

if not staged_diffed_line:
if is_snapshot_line:
staged_diffed_line = DiffedLine(a=line)
elif is_received_line:
staged_diffed_line = DiffedLine(b=line)
elif is_context_line:
staged_diffed_line = DiffedLine(c=[line])
# else: should never happen because then it would have
# encounted a diff line without any previously staged line

if staged_diffed_line:
yield staged_diffed_line

def __format_line(
self,
line: str,
diff_markers: str,
line_style: Callable[[str], str],
diff_style: Callable[[str], str],
show_ends: bool,
) -> str:
if show_ends:
for old, new in self._ends.items():
line = line.replace(old, new)
else:
line = self.__strip_ends(line)
return "".join(
emphasize(line_style(char)) if str(marker) in "-+^" else line_style(char)
for marker, char in zip_longest(marker_line.strip(), line)
diff_style(char) if str(marker) in "-+^" else line_style(char)
for marker, char in zip_longest(diff_markers.rstrip(), line)
if char is not None
)

def __limit_context(self, lines: List[str]) -> Iterator[str]:
yield from lines[: self._context_line_count]
num_lines = len(lines)
if num_lines:
if num_lines > self._context_line_max:
count_leading_whitespace: Callable[[str], int] = (
lambda s: len(s) - len(s.lstrip()) # noqa: E731
)
if self._context_line_count:
num_space = (
count_leading_whitespace(lines[self._context_line_count - 1])
+ count_leading_whitespace(lines[-self._context_line_count])
) // 2
else:
num_space = count_leading_whitespace(lines[num_lines // 2])
Comment on lines +343 to +352
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simpler to reason about but leaves room for ... having weird indentation

Suggested change
count_leading_whitespace: Callable[[str], int] = (
lambda s: len(s) - len(s.lstrip()) # noqa: E731
)
if self._context_line_count:
num_space = (
count_leading_whitespace(lines[self._context_line_count - 1])
+ count_leading_whitespace(lines[-self._context_line_count])
) // 2
else:
num_space = count_leading_whitespace(lines[num_lines // 2])
mid_line = lines[num_lines // 2]
num_space = len(mid_line) - len(mid_line.lstrip())

yield " " * num_space + self._marker_context_max
if self._context_line_count and num_lines > 1:
yield from lines[-self._context_line_count :] # noqa: E203

def __strip_ends(self, line: str) -> str:
return line.rstrip("".join(self._ends.keys()))


class AbstractSyrupyExtension(SnapshotSerializer, SnapshotFossilizer, SnapshotReporter):
def __init__(self, test_location: "TestLocation"):
Expand Down
4 changes: 2 additions & 2 deletions src/syrupy/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from typing import (
TYPE_CHECKING,
Any,
Generator,
Iterator,
List,
Set,
)
Expand Down Expand Up @@ -131,7 +131,7 @@ def unused(self) -> "SnapshotFossils":
return unused_fossils

@property
def lines(self) -> Generator[str, None, None]:
def lines(self) -> Iterator[str]:
summary_lines: List[str] = []
if self.num_failed:
summary_lines.append(
Expand Down
28 changes: 20 additions & 8 deletions src/syrupy/terminal.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,6 @@ def bold(text: Union[str, int]) -> str:
return colored.stylize(text, colored.attr("bold"))


def mute(text: Union[str, int]) -> str:
return colored.stylize(text, colored.attr("dim"))


def emphasize(text: Union[str, int]) -> str:
return colored.stylize(bold(text), colored.attr("underlined"))


def error_style(text: Union[str, int]) -> str:
return bold(red(text))

Expand All @@ -41,3 +33,23 @@ def warning_style(text: Union[str, int]) -> str:

def success_style(text: Union[str, int]) -> str:
return bold(green(text))


def snapshot_style(text: Union[str, int]) -> str:
return colored.stylize(text, colored.bg(225) + colored.fg(90))


def snapshot_diff_style(text: Union[str, int]) -> str:
return colored.stylize(text, colored.bg(90) + colored.fg(225))


def received_style(text: Union[str, int]) -> str:
return colored.stylize(text, colored.bg(195) + colored.fg(23))


def received_diff_style(text: Union[str, int]) -> str:
return colored.stylize(text, colored.bg(23) + colored.fg(195))


def context_style(text: Union[str, int]) -> str:
return colored.stylize(text, colored.attr("dim"))
4 changes: 2 additions & 2 deletions src/syrupy/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from pathlib import Path
from typing import (
Any,
Generator,
Iterator,
)

from .constants import SNAPSHOT_DIRNAME
Expand All @@ -13,7 +13,7 @@ def in_snapshot_dir(path: Path) -> bool:
return SNAPSHOT_DIRNAME in path.parts


def walk_snapshot_dir(root: str) -> Generator[str, None, None]:
def walk_snapshot_dir(root: str) -> Iterator[str]:
for filepath in Path(root).rglob("*"):
if not in_snapshot_dir(filepath):
continue
Expand Down
Loading