Merge pull request #419 from twisted/writer

Refactor _writer & bring to 100% coverage

README.rst

@@ -158,7 +158,7 @@ as the option 'title_format', already exists in newsfile, ``ValueError`` will be
 you "already produced newsfiles for this version".
 
 If ``single_file`` is set to ``false`` instead, each versioned ``towncrier build`` will generate a
-separate newsfile, whose name is formatted as the patten given by option ``filename``.
+separate newsfile, whose name is formatted as the pattern given by option ``filename``.
 For example, if ``filename="{version}-notes.rst"``, then the release note with version "7.8.9" will
 be written to the file "7.8.9-notes.rst". If the newsfile already exists, its content
 will be overwriten with new release note, without throwing a ``ValueError`` warning.

src/towncrier/_writer.py

@@ -6,38 +6,64 @@
 affecting existing content.
 """
 
-
-import os
+from pathlib import Path
 
 
 def append_to_newsfile(
-    directory, filename, start_string, top_line, content, single_file=True
+    directory, filename, start_string, top_line, content, single_file
 ):
+    """
+    Write *content* to *directory*/*filename* behind *start_string*.
+
+    Double-check *top_line* (i.e. the release header) is not already in the
+    file.
 
-    news_file = os.path.join(directory, filename)
+    if *single_file* is True, add it to an existing file, otherwise create a
+    fresh one.
+    """
+    news_file = Path(directory) / filename
 
-    if single_file:
-        if not os.path.exists(news_file):
-            existing_content = ""
-        else:
-            with open(news_file, encoding="utf8") as f:
-                existing_content = f.read()
-        existing_content = existing_content.split(start_string, 1)
-    else:
-        existing_content = [""]
+    header, prev_body = _figure_out_existing_content(
+        news_file, start_string, single_file
+    )
 
-    if top_line and top_line in existing_content[-1]:
+    if top_line and top_line in prev_body:
         raise ValueError("It seems you've already produced newsfiles for this version?")
 
-    with open(os.path.join(directory, filename), "wb") as f:
+    # Leave newlines alone. This probably leads to inconsistent newlines,
+    # because we've loaded existing content with universal newlines, but that's
+    # the original behavior.
+    with news_file.open("w", encoding="utf8", newline="") as f:
+        if header:
+            f.write(header)
+
+        f.write(content)
+
+        if prev_body:
+            f.write(f"\n\n{prev_body}")
+
+
+def _figure_out_existing_content(news_file, start_string, single_file):
+    """
+    Try to read *news_file* and split it into header (everything before
+    *start_string*) and the old body (everything after *start_string*).
+
+    If there's no *start_string*, return empty header.
+
+    Empty file and per-release files have neither.
+    """
+    if not single_file or not news_file.exists():
+        # Per-release news files always start empty.
+        # Non-existent files have no existing content.
+        return "", ""
+
+    # If we didn't use universal newlines here, we wouldn't find *start_string*
+    # which usually contains a `\n`.
+    with news_file.open(encoding="utf8") as f:
+        content = f.read()
 
-        if len(existing_content) > 1:
-            f.write(existing_content.pop(0).rstrip().encode("utf8"))
-            if start_string:
-                f.write(("\n\n" + start_string + "\n").encode("utf8"))
+    t = content.split(start_string, 1)
+    if len(t) == 2:
+        return f"{t[0].rstrip()}\n\n{start_string}\n", t[1].lstrip()
 
-        f.write(content.encode("utf8"))
-        if existing_content:
-            if existing_content[0]:
-                f.write(b"\n\n")
-            f.write(existing_content[0].lstrip().encode("utf8"))
+    return "", content.lstrip()

src/towncrier/test/test_build.py

@@ -717,6 +717,7 @@ def do_build_once_with(version, fragment_file, fragment):
                     "01-01-2001",
                     "--yes",
                 ],
+                catch_exceptions=False,
             )
             # not git repository, manually remove fragment file
             Path(f"newsfragments/{fragment_file}").unlink()
@@ -780,7 +781,7 @@ def test_bullet_points_false(self):
         """
         When all_bullets is false, subsequent lines are not indented.
 
-        The automatic ticket number inserted by towcier will allign with the
+        The automatic ticket number inserted by towncrier will align with the
         manual bullet.
         """
         runner = CliRunner()
@@ -794,7 +795,7 @@ def test_bullet_points_false(self):
                 )
             os.mkdir("newsfragments")
             with open("newsfragments/123.feature", "w") as f:
-                f.write("wow!\n" "~~~~\n" "\n" "No indentation at all.")
+                f.write("wow!\n~~~~\n\nNo indentation at all.")
             with open("newsfragments/124.bugfix", "w") as f:
                 f.write("#. Numbered bullet list.")
             with open("newsfragments/125.removal", "w") as f:
@@ -1010,7 +1011,7 @@ def test_start_string(self):
             with open("newsfragments/123.feature", "w") as f:
                 f.write("Adds levitation")
             with open("NEWS.rst", "w") as f:
-                f.write("a line\n\nanother\n\nRelease notes start marker\n")
+                f.write("a line\n\nanother\n\nRelease notes start marker\na footer!\n")
 
             result = runner.invoke(
                 _main,
@@ -1045,6 +1046,7 @@ def test_start_string(self):
             - Adds levitation (#123)
 
 
+            a footer!
         """
         )
 

src/towncrier/test/test_write.py

@@ -18,6 +18,8 @@
 
 
 class WritingTests(TestCase):
+    maxDiff = None
+
     def test_append_at_top(self):
 
         fragments = OrderedDict(
@@ -109,6 +111,7 @@ def test_append_at_top(self):
                 wrap=True,
                 versiondata={"name": "MyProject", "version": "1.0", "date": "never"},
             ),
+            single_file=True,
         )
 
         with open(os.path.join(tempdir, "NEWS.rst")) as f:
@@ -221,6 +224,7 @@ def test_append_at_top_with_hint(self):
                 wrap=True,
                 versiondata={"name": "MyProject", "version": "1.0", "date": "never"},
             ),
+            single_file=True,
         )
 
         with open(os.path.join(tempdir, "NEWS.rst")) as f:
@@ -259,6 +263,7 @@ def test_multiple_file_no_start_string(self):
             start_string=None,
             top_line="",
             content=content,
+            single_file=True,
         )
 
         with open(os.path.join(tempdir, "NEWS.rst")) as f: