feat(amber): normalise line endings between operating systems (#377)

* ci: run tests on windows

* ci: specify encoding in build steps

* ci: only use pty on enables os

* wip: strip newline from end of read amber snapshot data

* refactor: remove manual path separators from source

* refactor: reuse serialize plain

* refactor: reuse indent method

* wip: normalise new line control characters

* test: show control character diff functionality

* ci: remove unneeded matrix

* ci: call out release python version

* cr: replace recieved with received

* fix: ensure translation of carriage returns

* test: carriage return and line feed difference

Co-authored-by: Noah <noah.negin-ulster@tophatmonocle.com>

.github/workflows/pythonapp.yml

@@ -8,33 +8,34 @@ env:
 jobs:
   analysis:
     name: Code Analysis
-    runs-on: ubuntu-18.04
+    runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v1
       - name: Set up Python 3.8
         uses: actions/setup-python@v1
         with:
           python-version: 3.8
-      - name: Install lint & packaging dependencies.
+      - name: Install project dependencies
         run: |
           python -m pip install -U pip --no-cache-dir
           python -m pip install -e . -r dev_requirements.txt
       - name: Lint
         run: invoke lint
   tests:
     name: Tests
-    runs-on: ubuntu-18.04
+    runs-on: ${{ matrix.os }}
     strategy:
       matrix:
+        os: [ubuntu-latest, windows-latest]
         python-version: [3.6, 3.7, 3.8]
-      fail-fast: true
+      fail-fast: false
     steps:
       - uses: actions/checkout@v1
       - name: Set up Python ${{ matrix.python-version }}
         uses: actions/setup-python@v1
         with:
           python-version: ${{ matrix.python-version }}
-      - name: Install test dependencies.
+      - name: Install project dependencies
         run: |
           python -m pip install -U pip --no-cache-dir
           python -m pip install -e . -r dev_requirements.txt
@@ -44,23 +45,19 @@ jobs:
         run: invoke test --coverage
   build:
     name: Build
-    runs-on: ubuntu-18.04
-    strategy:
-      matrix:
-        python-version: [3.8]
-      fail-fast: true
+    runs-on: ubuntu-latest
     needs: [analysis, tests]
     steps:
       - uses: actions/checkout@v1
-      - name: Set up Python ${{ matrix.python-version }}
+      - name: Set up Python 3.8
         uses: actions/setup-python@v1
         with:
-          python-version: ${{ matrix.python-version }}
-      - name: Install build dependencies.
+          python-version: 3.8
+      - name: Install project dependencies
         run: |
           python -m pip install -U pip --no-cache-dir
           python -m pip install -e . -r dev_requirements.txt
-      - name: Release Python ${{ matrix.python-version }}
+      - name: Release Python 3.8
         env:
           TWINE_USERNAME: ${{ secrets.TWINE_USERNAME }}
           TWINE_PASSWORD: ${{ secrets.TWINE_PASSWORD }}

benchmarks/__init__.py

@@ -103,7 +103,7 @@ def measure_perf(
         return
     repo = github.get_repo(GH_REPO)
     commit_sha = get_req_env("GITHUB_SHA")
-    with open(BENCH_PERF_FILE, "r") as bench_file:
+    with open(BENCH_PERF_FILE, "r", encoding="utf-8") as bench_file:
         try:
             repo.create_file(
                 path=get_commit_bench_path(commit_sha),
@@ -144,7 +144,7 @@ def fetch_ref_bench_json(github: "Github", ref_branch: str = GH_BRANCH_REF) -> b
     if not ref_json:
         return False
 
-    with open(BENCH_REF_FILE, "w") as ref_json_file:
+    with open(BENCH_REF_FILE, "w", encoding="utf-8") as ref_json_file:
         ref_json_file.write(ref_json)
     return True
 

setup.py

@@ -43,7 +43,7 @@
 
 
 def readme() -> str:
-    with open("README.md") as f:
+    with open("README.md", encoding="utf-8") as f:
         return f.read()
 
 

src/syrupy/__init__.py

@@ -87,15 +87,16 @@ def pytest_assertrepr_compare(
     https://docs.pytest.org/en/latest/reference.html#_pytest.hookspec.pytest_assertrepr_compare
     """
     with __terminal_color(config):
+        received_name = received_style("[+ received]")
+
+        def snapshot_name(name: str) -> str:
+            return snapshot_style(f"[- {name}]")
+
         if isinstance(left, SnapshotAssertion):
-            assert_msg = reset(
-                f"{snapshot_style(left.name)} {op} {received_style('received')}"
-            )
+            assert_msg = reset(f"{snapshot_name(left.name)} {op} {received_name}")
             return [assert_msg] + left.get_assert_diff()
         elif isinstance(right, SnapshotAssertion):
-            assert_msg = reset(
-                f"{received_style('received')} {op} {snapshot_style(right.name)}"
-            )
+            assert_msg = reset(f"{received_name} {op} {snapshot_name(right.name)}")
             return [assert_msg] + right.get_assert_diff()
     return None
 

src/syrupy/assertion.py

@@ -106,7 +106,11 @@ def get_assert_diff(self) -> List[str]:
         serialized_data = assertion_result.asserted_data or ""
         diff: List[str] = []
         if snapshot_data is None:
-            diff.append(gettext("Snapshot does not exist!"))
+            diff.append(
+                gettext("Snapshot '{}' does not exist!").format(
+                    assertion_result.snapshot_name
+                )
+            )
         if not assertion_result.success:
             diff.extend(self.extension.diff_lines(serialized_data, snapshot_data or ""))
         return diff

src/syrupy/extensions/amber/serializer.py

@@ -1,3 +1,4 @@
+import os
 from types import GeneratorType
 from typing import (
     TYPE_CHECKING,
@@ -46,20 +47,21 @@ class DataSerializer:
     _max_depth: int = 99
     _marker_divider: str = "---"
     _marker_name: str = "# name:"
+    _marker_crn: str = "\r\n"
 
     @classmethod
     def write_file(cls, snapshot_fossil: "SnapshotFossil") -> None:
         """
         Writes the snapshot data into the snapshot file that be read later.
         """
         filepath = snapshot_fossil.location
-        with open(filepath, "w", encoding="utf-8", newline="") as f:
+        with open(filepath, "w", encoding="utf-8", newline=None) as f:
             for snapshot in sorted(snapshot_fossil, key=lambda s: s.name):
                 snapshot_data = str(snapshot.data)
                 if snapshot_data is not None:
                     f.write(f"{cls._marker_name} {snapshot.name}\n")
                     for data_line in snapshot_data.splitlines(keepends=True):
-                        f.write(f"{cls._indent}{data_line}")
+                        f.write(cls.with_indent(data_line, 1))
                     f.write(f"\n{cls._marker_divider}\n")
 
     @classmethod
@@ -73,20 +75,23 @@ def read_file(cls, filepath: str) -> "SnapshotFossil":
         indent_len = len(cls._indent)
         snapshot_fossil = SnapshotFossil(location=filepath)
         try:
-            with open(filepath, "r", encoding="utf-8", newline="") as f:
+            with open(filepath, "r", encoding="utf-8", newline=None) as f:
                 test_name = None
                 snapshot_data = ""
                 for line in f:
                     if line.startswith(cls._marker_name):
-                        test_name = line[name_marker_len:-1].strip(" \r\n")
+                        test_name = line[name_marker_len:].strip(f" {cls._marker_crn}")
                         snapshot_data = ""
                         continue
                     elif test_name is not None:
                         if line.startswith(cls._indent):
                             snapshot_data += line[indent_len:]
                         elif line.startswith(cls._marker_divider) and snapshot_data:
                             snapshot_fossil.add(
-                                Snapshot(name=test_name, data=snapshot_data[:-1])
+                                Snapshot(
+                                    name=test_name,
+                                    data=snapshot_data.rstrip(os.linesep),
+                                )
                             )
         except FileNotFoundError:
             pass
@@ -95,6 +100,23 @@ def read_file(cls, filepath: str) -> "SnapshotFossil":
 
     @classmethod
     def serialize(
+        cls,
+        data: "SerializableData",
+        *,
+        exclude: Optional["PropertyFilter"] = None,
+        matcher: Optional["PropertyMatcher"] = None,
+    ) -> str:
+        """
+        After serializing, new line control characters are normalised. This is needed
+        for interoperablity of snapshot matching between systems that do not use the
+        same new line control characters. Example snapshots generated on windows os
+        should not break when running the tests on a unix based system and vice versa.
+        """
+        serialized = cls._serialize(data, exclude=exclude, matcher=matcher)
+        return serialized.replace(cls._marker_crn, "\n").replace("\r", "\n")
+
+    @classmethod
+    def _serialize(
         cls,
         data: "SerializableData",
         *,
@@ -137,26 +159,27 @@ def serialize(
     def serialize_number(
         cls, data: "SerializableData", *, depth: int = 0, **kwargs: Any
     ) -> str:
-        return cls.with_indent(repr(data), depth)
+        return cls.__serialize_plain(data=data, depth=depth)
 
     @classmethod
     def serialize_string(
         cls, data: "SerializableData", *, depth: int = 0, **kwargs: Any
     ) -> str:
-        if "\n" in data:
-            return cls.__serialize_lines(
-                data=data,
-                lines=(
-                    cls.with_indent(line, depth + 1 if depth else depth)
-                    for line in str(data).splitlines(keepends=True)
-                ),
-                depth=depth,
-                open_tag="'",
-                close_tag="'",
-                include_type=False,
-                ends="",
-            )
-        return cls.with_indent(repr(data), depth)
+        if all(c not in data for c in cls._marker_crn):
+            return cls.__serialize_plain(data=data, depth=depth)
+
+        return cls.__serialize_lines(
+            data=data,
+            lines=(
+                cls.with_indent(line, depth + 1 if depth else depth)
+                for line in str(data).splitlines(keepends=True)
+            ),
+            depth=depth,
+            open_tag="'",
+            close_tag="'",
+            include_type=False,
+            ends="",
+        )
 
     @classmethod
     def serialize_iterable(cls, data: "SerializableData", **kwargs: Any) -> str:
@@ -214,7 +237,7 @@ def serialize_dict(cls, data: "SerializableData", **kwargs: Any) -> str:
     @classmethod
     def serialize_unknown(cls, data: Any, *, depth: int = 0, **kwargs: Any) -> str:
         if data.__class__.__repr__ != object.__repr__:
-            return cls.with_indent(repr(data), depth)
+            return cls.__serialize_plain(data=data, depth=depth)
 
         return cls.__serialize_iterable(
             data=data,
@@ -239,7 +262,7 @@ def sort(cls, iterable: Iterable[Any]) -> Iterable[Any]:
         try:
             return sorted(iterable)
         except TypeError:
-            return sorted(iterable, key=cls.serialize)
+            return sorted(iterable, key=cls._serialize)
 
     @classmethod
     def object_type(cls, data: "SerializableData") -> str:
@@ -251,6 +274,15 @@ def __is_namedtuple(cls, obj: Any) -> bool:
             type(n) == str for n in getattr(obj, "_fields", [None])
         )
 
+    @classmethod
+    def __serialize_plain(
+        cls,
+        *,
+        data: "SerializableData",
+        depth: int = 0,
+    ) -> str:
+        return cls.with_indent(repr(data), depth)
+
     @classmethod
     def __serialize_iterable(
         cls,
@@ -284,13 +316,13 @@ def key_str(key: "PropertyName") -> str:
             if separator is None:
                 return ""
             return (
-                cls.serialize(data=key, **kwargs)
+                cls._serialize(data=key, **kwargs)
                 if serialize_key
                 else cls.with_indent(str(key), depth=depth + 1)
             ) + separator
 
         def value_str(key: "PropertyName", value: "SerializableData") -> str:
-            serialized = cls.serialize(
+            serialized = cls._serialize(
                 data=value, exclude=exclude, path=(*path, (key, type(value))), **kwargs
             )
             return serialized if separator is None else serialized.lstrip(cls._indent)
@@ -317,10 +349,7 @@ def __serialize_lines(
     ) -> str:
         lines = ends.join(lines)
         lines_end = "\n" if lines else ""
-        formatted_open_tag = (
-            open_tag if include_type else cls.with_indent(open_tag, depth)
-        )
+        maybe_obj_type = f"{cls.object_type(data)} " if include_type else ""
+        formatted_open_tag = cls.with_indent(f"{maybe_obj_type}{open_tag}", depth)
         formatted_close_tag = cls.with_indent(close_tag, depth)
-        return (
-            f"{cls.with_indent(cls.object_type(data), depth)} " if include_type else ""
-        ) + f"{formatted_open_tag}\n{lines}{lines_end}{formatted_close_tag}"
+        return f"{formatted_open_tag}\n{lines}{lines_end}{formatted_close_tag}"

src/syrupy/extensions/base.py

@@ -140,16 +140,24 @@ def write_snapshot(self, *, data: "SerializedData", index: int) -> None:
         snapshot_location = self.get_location(index=index)
         if not self.test_location.matches_snapshot_location(snapshot_location):
             warning_msg = gettext(
-                "\nCan not relate snapshot location '{}' to the test location."
-                "\nConsider adding '{}' to the generated location."
-            ).format(snapshot_location, self.test_location.filename)
+                "{line_end}Can not relate snapshot location '{}' to the test location."
+                "{line_end}Consider adding '{}' to the generated location."
+            ).format(
+                snapshot_location,
+                self.test_location.filename,
+                line_end="\n",
+            )
             warnings.warn(warning_msg)
         snapshot_name = self.get_snapshot_name(index=index)
         if not self.test_location.matches_snapshot_name(snapshot_name):
             warning_msg = gettext(
-                "\nCan not relate snapshot name '{}' to the test location."
-                "\nConsider adding '{}' to the generated name."
-            ).format(snapshot_name, self.test_location.testname)
+                "{line_end}Can not relate snapshot name '{}' to the test location."
+                "{line_end}Consider adding '{}' to the generated name."
+            ).format(
+                snapshot_name,
+                self.test_location.testname,
+                line_end="\n",
+            )
             warnings.warn(warning_msg)
         snapshot_fossil = SnapshotFossil(location=snapshot_location)
         snapshot_fossil.add(Snapshot(name=snapshot_name, data=data))