Merge pull request #418 from twisted/git-coverage

src/towncrier/_git.py

@@ -3,7 +3,7 @@
 
 import os
 
-from subprocess import call
+from subprocess import STDOUT, call, check_output
 
 import click
 
@@ -27,3 +27,22 @@ def remove_files(fragment_filenames, answer_yes):
 def stage_newsfile(directory, filename):
 
     call(["git", "add", os.path.join(directory, filename)])
+
+
+def get_remote_branches(base_directory):
+    output = check_output(
+        ["git", "branch", "-r"], cwd=base_directory, encoding="utf-8", stderr=STDOUT
+    )
+
+    return [branch.strip() for branch in output.strip().splitlines()]
+
+
+def list_changed_files_compared_to_branch(base_directory, compare_with):
+    output = check_output(
+        ["git", "diff", "--name-only", compare_with + "..."],
+        cwd=base_directory,
+        encoding="utf-8",
+        stderr=STDOUT,
+    )
+
+    return output.strip().splitlines()

src/towncrier/check.py

@@ -5,26 +5,16 @@
 import os
 import sys
 
-from subprocess import STDOUT, CalledProcessError, check_output
+from subprocess import CalledProcessError
 from warnings import warn
 
 import click
 
 from ._builder import find_fragments
+from ._git import get_remote_branches, list_changed_files_compared_to_branch
 from ._settings import config_option_help, load_config_from_options
 
 
-def _run(args, **kwargs):
-    kwargs["stderr"] = STDOUT
-    return check_output(args, **kwargs)
-
-
-def _get_remote_branches(base_directory, encoding):
-    output = _run(["git", "branch", "-r"], cwd=base_directory).decode(encoding)
-
-    return [branch.strip() for branch in output.splitlines()]
-
-
 def _get_default_compare_branch(branches):
     if "origin/main" in branches:
         return "origin/main"
@@ -75,26 +65,18 @@ def __main(comparewith, directory, config):
 
     base_directory, config = load_config_from_options(directory, config)
 
-    # Use UTF-8 both when sys.stdout does not have .encoding (Python 2.7) and
-    # when the attribute is present but set to None (explicitly piped output
-    # and also some CI such as GitHub Actions).
-    encoding = getattr(sys.stdout, "encoding", "utf8")
     if comparewith is None:
         comparewith = _get_default_compare_branch(
-            _get_remote_branches(base_directory=base_directory, encoding=encoding)
+            get_remote_branches(base_directory=base_directory)
         )
 
     if comparewith is None:
         click.echo("Could not detect default branch. Aborting.")
         sys.exit(1)
 
     try:
-        files_changed = (
-            _run(
-                ["git", "diff", "--name-only", comparewith + "..."], cwd=base_directory
-            )
-            .decode(encoding)
-            .strip()
+        files_changed = list_changed_files_compared_to_branch(
+            base_directory, comparewith
         )
     except CalledProcessError as e:
         click.echo("git produced output while failing:")
@@ -108,8 +90,7 @@ def __main(comparewith, directory, config):
         sys.exit(0)
 
     files = {
-        os.path.normpath(os.path.join(base_directory, path))
-        for path in files_changed.strip().splitlines()
+        os.path.normpath(os.path.join(base_directory, path)) for path in files_changed
     }
 
     click.echo("Looking at these files:")

src/towncrier/create.py

@@ -116,8 +116,7 @@ def _get_news_content_from_user(message):
         "# In order to abort, exit without saving.\n"
         '# Lines starting with "#" are ignored.\n'
     )
-    if message is not None:
-        initial_content += "\n" "{message}\n".format(message=message)
+    initial_content += f"\n{message}\n"
     content = click.edit(initial_content)
     if content is None:
         return None

src/towncrier/test/test_build.py

@@ -7,6 +7,7 @@
 from pathlib import Path
 from subprocess import call
 from textwrap import dedent
+from unittest.mock import patch
 
 from click.testing import CliRunner
 from twisted.trial.unittest import TestCase
@@ -24,6 +25,11 @@ def setup_simple_project():
     os.mkdir("foo/newsfragments")
 
 
+def read_all(filename):
+    with open(filename) as f:
+        return f.read()
+
+
 class TestCli(TestCase):
     maxDiff = None
 
@@ -100,7 +106,7 @@ def test_no_newsfragment_directory(self):
         self.assertEqual(1, result.exit_code, result.output)
         self.assertIn("Failed to list the news fragment files.\n", result.output)
 
-    def test_no_newsfragments(self):
+    def test_no_newsfragments_draft(self):
         """
         An empty newsfragment directory acts as if there are no changes.
         """
@@ -114,6 +120,23 @@ def test_no_newsfragments(self):
         self.assertEqual(0, result.exit_code)
         self.assertIn("No significant changes.\n", result.output)
 
+    def test_no_newsfragments(self):
+        """
+        An empty newsfragment directory acts as if there are no changes and
+        removing files handles it gracefully.
+        """
+        runner = CliRunner()
+
+        with runner.isolated_filesystem():
+            setup_simple_project()
+
+            result = runner.invoke(_main, ["--date", "01-01-2001"])
+
+            news = read_all("NEWS.rst")
+
+        self.assertEqual(0, result.exit_code)
+        self.assertIn("No significant changes.\n", news)
+
     def test_collision(self):
         runner = CliRunner()
 
@@ -334,6 +357,38 @@ def test_no_confirmation(self):
             self.assertFalse(os.path.isfile(fragment_path1))
             self.assertFalse(os.path.isfile(fragment_path2))
 
+    def test_confirmation_says_no(self):
+        """
+        If the user says "no" to removing the newsfragements, we end up with
+        a NEWS.rst AND the newsfragments.
+        """
+        runner = CliRunner()
+
+        with runner.isolated_filesystem():
+            setup_simple_project()
+            fragment_path1 = "foo/newsfragments/123.feature"
+            fragment_path2 = "foo/newsfragments/124.feature.rst"
+            with open(fragment_path1, "w") as f:
+                f.write("Adds levitation")
+            with open(fragment_path2, "w") as f:
+                f.write("Extends levitation")
+
+            call(["git", "init"])
+            call(["git", "config", "user.name", "user"])
+            call(["git", "config", "user.email", "user@example.com"])
+            call(["git", "add", "."])
+            call(["git", "commit", "-m", "Initial Commit"])
+
+            with patch("towncrier._git.click.confirm") as m:
+                m.return_value = False
+                result = runner.invoke(_main, [])
+
+            self.assertEqual(0, result.exit_code)
+            path = "NEWS.rst"
+            self.assertTrue(os.path.isfile(path))
+            self.assertTrue(os.path.isfile(fragment_path1))
+            self.assertTrue(os.path.isfile(fragment_path2))
+
     def test_needs_config(self):
         """
         Towncrier needs a configuration file.
@@ -586,10 +641,8 @@ def do_build_once_with(version, fragment_file, fragment):
             self.assertTrue(os.path.exists("7.9.0-notes.rst"), os.listdir("."))
 
             outputs = []
-            with open("7.8.9-notes.rst") as f:
-                outputs.append(f.read())
-            with open("7.9.0-notes.rst") as f:
-                outputs.append(f.read())
+            outputs.append(read_all("7.8.9-notes.rst"))
+            outputs.append(read_all("7.9.0-notes.rst"))
 
             self.assertEqual(
                 outputs[0],
@@ -697,8 +750,7 @@ def do_build_once_with(version, fragment_file, fragment):
             )
             self.assertTrue(os.path.exists("{version}-notes.rst"), os.listdir("."))
 
-            with open("{version}-notes.rst") as f:
-                output = f.read()
+            output = read_all("{version}-notes.rst")
 
             self.assertEqual(
                 output,
@@ -764,8 +816,7 @@ def test_bullet_points_false(self):
             )
 
             self.assertEqual(0, result.exit_code, result.output)
-            with open("NEWS.rst") as f:
-                output = f.read()
+            output = read_all("NEWS.rst")
 
         self.assertEqual(
             output,
@@ -976,8 +1027,7 @@ def test_start_string(self):
 
             self.assertEqual(0, result.exit_code, result.output)
             self.assertTrue(os.path.exists("NEWS.rst"), os.listdir("."))
-            with open("NEWS.rst") as f:
-                output = f.read()
+            output = read_all("NEWS.rst")
 
         expected_output = dedent(
             """\